[PATCH] D130301: [Clang] Fix how we set the NumPositiveBits on an E numDecl to cover the case of single enumerator with value zero or an empty enum

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 446702.
shafik added a comment.
Herald added a subscriber: Enna1.

-Modified UBSan test to cover the empty enum case. I also refactored the test which was pretty clever but hard to work with as is.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130301/new/

https://reviews.llvm.org/D130301

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGenCXX/pr12251.cpp
  compiler-rt/test/ubsan/TestCases/Misc/enum.cpp


Index: compiler-rt/test/ubsan/TestCases/Misc/enum.cpp
===================================================================
--- compiler-rt/test/ubsan/TestCases/Misc/enum.cpp
+++ compiler-rt/test/ubsan/TestCases/Misc/enum.cpp
@@ -1,21 +1,28 @@
-// 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 {};
 
 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);
+
+  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;
+  // CHECK: error: load of value 4294967295, which is not a valid value for type 'E'
+  return ((int)e1 != -1) & ((int)e2 != -1) &
+         // CHECK: error: load of value <unknown>, which is not a valid value for type 'enum EBool'
+         ((int)e3 != -1) & ((int)e4 == 1) &
+         // CHECK: error: load of value 2, which is not a valid value for type 'EEmpty'
+         ((int)e5 == 2);
 }
Index: clang/test/CodeGenCXX/pr12251.cpp
===================================================================
--- clang/test/CodeGenCXX/pr12251.cpp
+++ clang/test/CodeGenCXX/pr12251.cpp
@@ -18,14 +18,14 @@
   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) {
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -18878,14 +18878,24 @@
     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
+      const unsigned ActiveBits =
+          InitVal.getActiveBits() ? InitVal.getActiveBits() : 1;
+      NumPositiveBits = std::max(NumPositiveBits, ActiveBits);
+    } else
       NumNegativeBits = std::max(NumNegativeBits,
                                  (unsigned)InitVal.getMinSignedBits());
   }
 
+  // If we have have a 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;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130301.446702.patch
Type: text/x-patch
Size: 3959 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220722/71d579c8/attachment-0001.bin>