Skip to content

Commit

Permalink
[Clang] Diagnose ill-formed constant expression when setting a non fi…
Browse files Browse the repository at this point in the history
…xed enum to a value outside the range of the enumeration values

DR2338 clarified that it was undefined behavior to set the value outside the
range of the enumerations values for an enum without a fixed underlying type.

We should diagnose this with a constant expression context.

Differential Revision: https://reviews.llvm.org/D130058
  • Loading branch information
shafik committed Jul 28, 2022
1 parent 58526b2 commit b364535
Show file tree
Hide file tree
Showing 13 changed files with 119 additions and 28 deletions.
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
Expand Up @@ -56,6 +56,10 @@ Bug Fixes

Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
- Clang will now correctly diagnose as ill-formed a constant expression where an
enum without a fixed underlying type is set to a value outside the range of
of the enumerations values. Fixes
`Issue 50055: <https://github.com/llvm/llvm-project/issues/50055>`_.

Non-comprehensive list of changes in this release
-------------------------------------------------
Expand Down
5 changes: 5 additions & 0 deletions clang/include/clang/AST/Decl.h
Expand Up @@ -3842,6 +3842,11 @@ class EnumDecl : public TagDecl {
/// -101 1001011 8
unsigned getNumNegativeBits() const { return EnumDeclBits.NumNegativeBits; }

/// Calculates the [Min,Max) values the enum can store based on the
/// NumPositiveBits and NumNegativeBits. This matters for enums that do not
/// have a fixed underlying type.
void getValueRange(llvm::APInt &Max, llvm::APInt &Min) const;

/// Returns true if this is a C++11 scoped enumeration.
bool isScoped() const { return EnumDeclBits.IsScoped; }

Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticASTKinds.td
Expand Up @@ -366,6 +366,9 @@ def note_constexpr_unsupported_layout : Note<
"type %0 has unexpected layout">;
def note_constexpr_unsupported_flexible_array : Note<
"flexible array initialization is not yet supported">;
def note_constexpr_unscoped_enum_out_of_range : Note<
"integer value %0 is outside the valid range of values [%1, %2) for this "
"enumeration type">;
def err_experimental_clang_interp_failed : Error<
"the experimental clang interpreter failed to evaluate an expression">;

Expand Down
15 changes: 15 additions & 0 deletions clang/lib/AST/Decl.cpp
Expand Up @@ -4621,6 +4621,21 @@ SourceRange EnumDecl::getSourceRange() const {
return Res;
}

void EnumDecl::getValueRange(llvm::APInt &Max, llvm::APInt &Min) const {
unsigned Bitwidth = getASTContext().getIntWidth(getIntegerType());
unsigned NumNegativeBits = getNumNegativeBits();
unsigned NumPositiveBits = getNumPositiveBits();

if (NumNegativeBits) {
unsigned NumBits = std::max(NumNegativeBits, NumPositiveBits + 1);
Max = llvm::APInt(Bitwidth, 1) << (NumBits - 1);
Min = -Max;
} else {
Max = llvm::APInt(Bitwidth, 1) << NumPositiveBits;
Min = llvm::APInt::getZero(Bitwidth);
}
}

//===----------------------------------------------------------------------===//
// RecordDecl Implementation
//===----------------------------------------------------------------------===//
Expand Down
31 changes: 31 additions & 0 deletions clang/lib/AST/ExprConstant.cpp
Expand Up @@ -13519,6 +13519,37 @@ bool IntExprEvaluator::VisitCastExpr(const CastExpr *E) {
return Info.Ctx.getTypeSize(DestType) == Info.Ctx.getTypeSize(SrcType);
}

if (DestType->isEnumeralType()) {
const EnumType *ET = dyn_cast<EnumType>(DestType.getCanonicalType());
const EnumDecl *ED = ET->getDecl();
// Check that the value is within the range of the enumeration values.
//
// This corressponds to [expr.static.cast]p10 which says:
// A value of integral or enumeration type can be explicitly converted
// to a complete enumeration type ... If the enumeration type does not
// have a fixed underlying type, the value is unchanged if the original
// value is within the range of the enumeration values ([dcl.enum]), and
// otherwise, the behavior is undefined.
//
// This was resolved as part of DR2338 which has CD5 status.
if (!ED->isFixed()) {
llvm::APInt Min;
llvm::APInt Max;

ED->getValueRange(Max, Min);

if (ED->getNumNegativeBits() &&
(Max.sle(Result.getInt().getSExtValue()) ||
Min.sgt(Result.getInt().getSExtValue())))
CCEDiag(E, diag::note_constexpr_unscoped_enum_out_of_range)
<< Result.getInt() << Min.getSExtValue() << Max.getSExtValue();
else if (!ED->getNumNegativeBits() &&
Max.ule(Result.getInt().getZExtValue()))
CCEDiag(E, diag::note_constexpr_unscoped_enum_out_of_range)
<< Result.getInt() << Min.getZExtValue() << Max.getZExtValue();
}
}

return Success(HandleIntToIntCast(Info, E, DestType, SrcType,
Result.getInt()), E);
}
Expand Down
16 changes: 1 addition & 15 deletions clang/lib/CodeGen/CGExpr.cpp
Expand Up @@ -1661,21 +1661,7 @@ static bool getRangeForType(CodeGenFunction &CGF, QualType Ty,
End = llvm::APInt(CGF.getContext().getTypeSize(Ty), 2);
} else {
const EnumDecl *ED = ET->getDecl();
llvm::Type *LTy = CGF.ConvertTypeForMem(ED->getIntegerType());
unsigned Bitwidth = LTy->getScalarSizeInBits();
unsigned NumNegativeBits = ED->getNumNegativeBits();
unsigned NumPositiveBits = ED->getNumPositiveBits();

if (NumNegativeBits) {
unsigned NumBits = std::max(NumNegativeBits, NumPositiveBits + 1);
assert(NumBits <= Bitwidth);
End = llvm::APInt(Bitwidth, 1) << (NumBits - 1);
Min = -End;
} else {
assert(NumPositiveBits <= Bitwidth);
End = llvm::APInt(Bitwidth, 1) << NumPositiveBits;
Min = llvm::APInt::getZero(Bitwidth);
}
ED->getValueRange(End, Min);
}
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Analysis/cfg.cpp
Expand Up @@ -223,7 +223,7 @@ namespace NoReturnSingleSuccessor {
// CHECK-NEXT: Succs (1): B1
// CHECK: [B0 (EXIT)]
// CHECK-NEXT: Preds (1): B1
enum MyEnum { A, B, C };
enum MyEnum : int { A, B, C };
static const enum MyEnum D = (enum MyEnum) 32;

int test_enum_with_extension(enum MyEnum value) {
Expand Down
8 changes: 4 additions & 4 deletions clang/test/Sema/aarch64-sve-intrinsics/acle_sve_imm.cpp
Expand Up @@ -206,19 +206,19 @@ void test_range_0_13(svbool_t pg, const void *const_void_ptr)
{
// expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 13]}}
svprfb(pg, const_void_ptr, svprfop(14));
// expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 13]}}
// expected-error-re@+1 {{must be a constant integer}}
svprfb_vnum(pg, const_void_ptr, 0, svprfop(-1));
// expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 13]}}
svprfd(pg, const_void_ptr, svprfop(14));
// expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 13]}}
// expected-error-re@+1 {{must be a constant integer}}
svprfd_vnum(pg, const_void_ptr, 0, svprfop(-1));
// expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 13]}}
svprfh(pg, const_void_ptr, svprfop(14));
// expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 13]}}
// expected-error-re@+1 {{must be a constant integer}}
svprfh_vnum(pg, const_void_ptr, 0, svprfop(-1));
// expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 13]}}
svprfw(pg, const_void_ptr, svprfop(14));
// expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 13]}}
// expected-error-re@+1 {{must be a constant integer}}
svprfw_vnum(pg, const_void_ptr, 0, svprfop(-1));
}

Expand Down
47 changes: 47 additions & 0 deletions clang/test/SemaCXX/constant-expression-cxx11.cpp
Expand Up @@ -2399,3 +2399,50 @@ void local_constexpr_var() {
constexpr int a = 0; // expected-note {{address of non-static constexpr variable 'a' may differ on each invocation of the enclosing function; add 'static' to give it a constant address}}
constexpr const int *p = &a; // expected-error {{constant expression}} expected-note {{pointer to 'a' is not a constant expression}}
}

namespace GH50055 {
// Enums without fixed underlying type
enum E1 {e11=-4, e12=4};
enum E2 {e21=0, e22=4};
enum E3 {e31=-4, e32=1024};
enum E4 {e41=0};
// Empty but as-if it had a single enumerator with value 0
enum EEmpty {};

// Enum with fixed underlying type because the underlying type is explicitly specified
enum EFixed : int {efixed1=-4, efixed2=4};
// Enum with fixed underlying type because it is scoped
enum class EScoped {escoped1=-4, escoped2=4};

void testValueInRangeOfEnumerationValues() {
constexpr E1 x1 = static_cast<E1>(-8);
constexpr E1 x2 = static_cast<E1>(8); // expected-error {{must be initialized by a constant expression}}
// expected-note@-1 {{integer value 8 is outside the valid range of values [-8, 8) for this enumeration type}}

constexpr E2 x3 = static_cast<E2>(-8); // expected-error {{must be initialized by a constant expression}}
// expected-note@-1 {{integer value -8 is outside the valid range of values [0, 8) for this enumeration type}}
constexpr E2 x4 = static_cast<E2>(0);
constexpr E2 x5 = static_cast<E2>(8); // expected-error {{must be initialized by a constant expression}}
// expected-note@-1 {{integer value 8 is outside the valid range of values [0, 8) for this enumeration type}}

constexpr E3 x6 = static_cast<E3>(-2048);
constexpr E3 x7 = static_cast<E3>(-8);
constexpr E3 x8 = static_cast<E3>(0);
constexpr E3 x9 = static_cast<E3>(8);
constexpr E3 x10 = static_cast<E3>(2048); // expected-error {{must be initialized by a constant expression}}
// expected-note@-1 {{integer value 2048 is outside the valid range of values [-2048, 2048) for this enumeration type}}

constexpr E4 x11 = static_cast<E4>(0);
constexpr E4 x12 = static_cast<E4>(1);
constexpr E4 x13 = static_cast<E4>(2); // expected-error {{must be initialized by a constant expression}}
// expected-note@-1 {{integer value 2 is outside the valid range of values [0, 2) for this enumeration type}}

constexpr EEmpty x14 = static_cast<EEmpty>(0);
constexpr EEmpty x15 = static_cast<EEmpty>(1);
constexpr EEmpty x16 = static_cast<EEmpty>(2); // expected-error {{must be initialized by a constant expression}}
// expected-note@-1 {{integer value 2 is outside the valid range of values [0, 2) for this enumeration type}}

constexpr EFixed x17 = static_cast<EFixed>(100);
constexpr EScoped x18 = static_cast<EScoped>(100);
}
}
2 changes: 1 addition & 1 deletion clang/test/SemaCXX/enum-scoped.cpp
Expand Up @@ -327,7 +327,7 @@ namespace test11 {
}

namespace PR35586 {
enum C { R, G, B };
enum C { R=-1, G, B };
enum B { F = (enum C) -1, T}; // this should compile cleanly, it used to assert.
};

Expand Down
8 changes: 4 additions & 4 deletions clang/test/SemaTemplate/temp_arg_enum_printing.cpp
Expand Up @@ -3,7 +3,7 @@
namespace NamedEnumNS
{

enum NamedEnum
enum class NamedEnum
{
Val0,
Val1
Expand All @@ -13,9 +13,9 @@ template <NamedEnum E>
void foo();

void test() {
// CHECK: template<> void foo<NamedEnumNS::Val0>()
NamedEnumNS::foo<Val0>();
// CHECK: template<> void foo<NamedEnumNS::Val1>()
// CHECK: template<> void foo<NamedEnumNS::NamedEnum::Val0>()
NamedEnumNS::foo<NamedEnum::Val0>();
// CHECK: template<> void foo<NamedEnumNS::NamedEnum::Val1>()
NamedEnumNS::foo<(NamedEnum)1>();
// CHECK: template<> void foo<(NamedEnumNS::NamedEnum)2>()
NamedEnumNS::foo<(NamedEnum)2>();
Expand Down
2 changes: 1 addition & 1 deletion clang/www/cxx_dr_status.html
Expand Up @@ -13842,7 +13842,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://wg21.link/cwg2338">2338</a></td>
<td>CD5</td>
<td>Undefined behavior converting to short enums with fixed underlying types</td>
<td class="full" align="center">Clang 12</td>
<td class="full" align="center">Clang 16</td>
</tr>
<tr id="2339">
<td><a href="https://wg21.link/cwg2339">2339</a></td>
Expand Down
Expand Up @@ -25,9 +25,9 @@ struct udt { };
}
template<template<typename> class T>
void ttp_user() { }
enum Enumeration { Enumerator1, Enumerator2, Enumerator3 = 1 };
enum Enumeration : int { Enumerator1, Enumerator2, Enumerator3 = 1 };
enum class EnumerationClass { Enumerator1, Enumerator2, Enumerator3 = 1 };
enum { AnonEnum1, AnonEnum2, AnonEnum3 = 1 };
enum : int { AnonEnum1, AnonEnum2, AnonEnum3 = 1 };
enum EnumerationSmall : unsigned char { kNeg = 0xff };
}
template <typename... Ts>
Expand Down

0 comments on commit b364535

Please sign in to comment.