[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

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
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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;

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) {

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'
 }