From: Shafik Yaghmour Date: Mon, 25 Jul 2022 22:56:44 +0000 (-0700) Subject: [Clang] Fix how we set the NumPositiveBits on an EnumDecl to cover the case of single... X-Git-Tag: upstream/15.0.7~501 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=aea82d4551139ded0290afab739f0b367d055628;p=platform%2Fupstream%2Fllvm.git [Clang] Fix how we set the NumPositiveBits on an EnumDecl to cover the case of single enumerator with value zero or an empty enum Currently in Sema::ActOnEnumBody(...) when calculating NumPositiveBits we miss the case where there is only a single enumerator with value zero and the case of an empty enum. In both cases we end up with zero positive bits when in fact we need one bit to store the value zero. This PR updates the calculation to account for these cases. Differential Revision: https://reviews.llvm.org/D130301 --- diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index ab2f638..1dc66ac 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -198,6 +198,10 @@ Bug Fixes constant folded. Fixes `Issue 55638 `_. - Fixed incompatibility of Clang's ```` with MSVC ````. Fixes `MSVC STL Issue 2862 `_. +- Empty enums and enums with a single enumerator with value zero will be + considered to have one positive bit in order to represent the underlying + value. This effects whether we consider the store of the value one to be well + defined. Improvements to Clang's diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 8d2fc53..1c793eb 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -18899,14 +18899,24 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange, const llvm::APSInt &InitVal = ECD->getInitVal(); // Keep track of the size of positive and negative values. - if (InitVal.isUnsigned() || InitVal.isNonNegative()) - NumPositiveBits = std::max(NumPositiveBits, - (unsigned)InitVal.getActiveBits()); - else + if (InitVal.isUnsigned() || InitVal.isNonNegative()) { + // If the enumerator is zero that should still be counted as a positive + // bit since we need a bit to store the value zero. + unsigned ActiveBits = InitVal.getActiveBits(); + NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u}); + } else { NumNegativeBits = std::max(NumNegativeBits, (unsigned)InitVal.getMinSignedBits()); + } } + // If we have have an empty set of enumerators we still need one bit. + // From [dcl.enum]p8 + // If the enumerator-list is empty, the values of the enumeration are as if + // the enumeration had a single enumerator with value 0 + if (!NumPositiveBits && !NumNegativeBits) + NumPositiveBits = 1; + // Figure out the type that should be used for this enum. QualType BestType; unsigned BestWidth; diff --git a/clang/test/CodeGenCXX/pr12251.cpp b/clang/test/CodeGenCXX/pr12251.cpp index 88f3cbe..2615bc7 100644 --- a/clang/test/CodeGenCXX/pr12251.cpp +++ b/clang/test/CodeGenCXX/pr12251.cpp @@ -18,14 +18,14 @@ e1 g1(e1 *x) { return *x; } // CHECK-LABEL: define{{.*}} i32 @_Z2g1P2e1 -// CHECK: ret i32 0 +// CHECK: ret i32 %0 enum e2 { e2_a = 0 }; e2 g2(e2 *x) { return *x; } // CHECK-LABEL: define{{.*}} i32 @_Z2g2P2e2 -// CHECK: ret i32 0 +// CHECK: ret i32 %0 enum e3 { e3_a = 16 }; e3 g3(e3 *x) { diff --git a/compiler-rt/test/ubsan/TestCases/Misc/enum.cpp b/compiler-rt/test/ubsan/TestCases/Misc/enum.cpp index 8e95f8b..569fb63 100644 --- a/compiler-rt/test/ubsan/TestCases/Misc/enum.cpp +++ b/compiler-rt/test/ubsan/TestCases/Misc/enum.cpp @@ -1,21 +1,33 @@ -// RUN: %clangxx -fsanitize=enum %s -O3 -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-PLAIN -// RUN: %clangxx -fsanitize=enum -std=c++11 -DE="class E" %s -O3 -o %t && %run %t -// RUN: %clangxx -fsanitize=enum -std=c++11 -DE="class E : bool" %s -O3 -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-BOOL +// RUN: %clangxx -fsanitize=enum %s -O3 -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=CHECK // FIXME: UBSan fails to add the correct instrumentation code for some reason on // Windows. // XFAIL: windows-msvc -enum E { a = 1 } e; -#undef E +enum E { a = 1 }; +enum class EClass { a = 1 }; +enum class EBool : bool { a = 1 } e3; +enum EEmpty {}; +enum EMinus { em = -1 }; int main(int argc, char **argv) { - // memset(&e, 0xff, sizeof(e)); - for (unsigned char *p = (unsigned char*)&e; p != (unsigned char*)(&e + 1); ++p) + E e1 = static_cast(0xFFFFFFFF); + EClass e2 = static_cast(0xFFFFFFFF); + EEmpty e4 = static_cast(1); + EEmpty e5 = static_cast(2); + EMinus e6 = static_cast(1); + EMinus e7 = static_cast(2); + + for (unsigned char *p = (unsigned char *)&e3; p != (unsigned char *)(&e3 + 1); + ++p) *p = 0xff; - // CHECK-PLAIN: error: load of value 4294967295, which is not a valid value for type 'enum E' - // FIXME: Support marshalling and display of enum class values. - // CHECK-BOOL: error: load of value , which is not a valid value for type 'enum E' - return (int)e != -1; + return ((int)e1 != -1) & ((int)e2 != -1) & + // CHECK: error: load of value 4294967295, which is not a valid value for type 'E' + ((int)e3 != -1) & ((int)e4 == 1) & + // CHECK: error: load of value , which is not a valid value for type 'enum EBool' + ((int)e5 == 2) & ((int)e6 == 1) & + // CHECK: error: load of value 2, which is not a valid value for type 'EEmpty' + ((int)e7 == 2); + // CHECK: error: load of value 2, which is not a valid value for type 'EMinus' }