[clang] aea82d4 - [Clang] Fix how we set the NumPositiveBits on an EnumDecl to cover the case of single enumerator with value zero or an empty enum
Shafik Yaghmour via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 25 16:01:09 PDT 2022
Author: Shafik Yaghmour
Date: 2022-07-25T16:01:01-07:00
New Revision: aea82d4551139ded0290afab739f0b367d055628
URL: https://github.com/llvm/llvm-project/commit/aea82d4551139ded0290afab739f0b367d055628
DIFF: https://github.com/llvm/llvm-project/commit/aea82d4551139ded0290afab739f0b367d055628.diff
LOG: [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
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaDecl.cpp
clang/test/CodeGenCXX/pr12251.cpp
compiler-rt/test/ubsan/TestCases/Misc/enum.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ab2f6387492b7..1dc66ac0364e4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -198,6 +198,10 @@ Bug Fixes
constant folded. Fixes `Issue 55638 <https://github.com/llvm/llvm-project/issues/55638>`_.
- Fixed incompatibility of Clang's ``<stdatomic.h>`` with MSVC ``<atomic>``.
Fixes `MSVC STL Issue 2862 <https://github.com/microsoft/STL/issues/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 8d2fc5331a0df..1c793eb3320e1 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 88f3cbea9202a..2615bc7260e95 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 8e95f8b403a93..569fb638526da 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<E>(0xFFFFFFFF);
+ EClass e2 = static_cast<EClass>(0xFFFFFFFF);
+ EEmpty e4 = static_cast<EEmpty>(1);
+ EEmpty e5 = static_cast<EEmpty>(2);
+ EMinus e6 = static_cast<EMinus>(1);
+ EMinus e7 = static_cast<EMinus>(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 <unknown>, 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 <unknown>, 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'
}
More information about the cfe-commits
mailing list