[clang] [clang][Sema] Fix for enums overflowing (#24667) (PR #78742)

via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 20 21:00:53 PDT 2024


https://github.com/wheatman updated https://github.com/llvm/llvm-project/pull/78742

>From a1275f9711d9e0c485c14e605c677446c4366393 Mon Sep 17 00:00:00 2001
From: Brian Wheatman <bwheatman at gmail.com>
Date: Fri, 19 Jan 2024 11:13:33 -0500
Subject: [PATCH] [clang][Sema] Fix for overflow in enumerators(#24667)

Enums which do not have a specified type can only grow to bigger types
which contain more bits than the prior types.  This means that the
largest signed integer type cannot grow to the largest unsigned integer types.

In the process also implements N3029 Improved Normal Enumerations and
patially implements N3030 Enhancements to Enumerations which brings C enums
more inline with C++ enums.

Fixes #24667
---
 clang/docs/ReleaseNotes.rst |   8 +++
 clang/lib/Sema/SemaDecl.cpp |  90 +++++++++++++++++++------
 clang/test/C/C2x/n3029.c    |  81 ++++++++++++++++++++++
 clang/test/C/C2x/n3030.c    | 130 ++++++++++++++++++++++++++++++++++++
 clang/test/Sema/enum.c      |  43 ++++++++++--
 clang/test/SemaCXX/enum.cpp |   2 +
 6 files changed, 329 insertions(+), 25 deletions(-)
 create mode 100644 clang/test/C/C2x/n3029.c
 create mode 100644 clang/test/C/C2x/n3030.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3a74c070ff9ffe..9ae53d3a400ad5 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -159,6 +159,12 @@ C23 Feature Support
   ``__TYPE_FMTb__`` (e.g., ``__UINT_FAST64_FMTB__``) in C23 mode for use with
   macros typically exposed from ``<inttypes.h>``, such as ``PRIb8``.
   (`#81896: <https://github.com/llvm/llvm-project/issues/81896>`_).
+- Enumerations should allow values greater than ``INT_MAX`` and smaller than
+  ``INT_MIN``, in order to provide a value-preserved set of integer constants. `N3029 Improved Normal Enumerations <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3029.htm>`_
+
+- Enumerations should have the ability to specify the underlying type to aid
+  in portability and usability across platforms, across ABIs, and across
+  languages (for serialization and similar purposes). `N3030 Enhancements to Enumerations <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3030.htm>`_
 
 - Clang now supports `N3018 The constexpr specifier for object definitions`
   <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3018.htm>`_.
@@ -300,6 +306,8 @@ Bug Fixes in This Version
 
 - Fixes an assertion failure on invalid code when trying to define member
   functions in lambdas.
+- Fixes a miscompilation when an enum has a specified value such that the automatic
+  increment overflows a ``signed long``. Fixes #GH24667.
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 5850cd0ab6b9aa..de672237e676e0 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19881,6 +19881,18 @@ EnumConstantDecl *Sema::CheckEnumConstant(EnumDecl *Enum,
           //     - If an initializer is specified for an enumerator, the
           //       initializing value has the same type as the expression.
           EltTy = Val->getType();
+        } else if (getLangOpts().C23) {
+          // C23 6.7.2.2p11 b4
+          // int, if given explicitly with = and the value of the
+          // integer constant expression is representable by an int
+          //
+          // C23 6.7.2.2p11 b5
+          // the type of the integer constant expression, if given
+          // explicitly with = and if the value of the integer
+          // constant expression is not representable by int;
+          if (isRepresentableIntegerValue(Context, EnumVal, Context.IntTy))
+            Val = ImpCastExprToType(Val, Context.IntTy, CK_IntegralCast).get();
+          EltTy = Val->getType();
         } else {
           // C99 6.7.2.2p2:
           //   The expression that defines the value of an enumeration constant
@@ -19890,8 +19902,8 @@ EnumConstantDecl *Sema::CheckEnumConstant(EnumDecl *Enum,
           // Complain if the value is not representable in an int.
           if (!isRepresentableIntegerValue(Context, EnumVal, Context.IntTy))
             Diag(IdLoc, diag::ext_enum_value_not_int)
-              << toString(EnumVal, 10) << Val->getSourceRange()
-              << (EnumVal.isUnsigned() || EnumVal.isNonNegative());
+                << toString(EnumVal, 10) << Val->getSourceRange()
+                << (EnumVal.isUnsigned() || EnumVal.isNonNegative());
           else if (!Context.hasSameType(Val->getType(), Context.IntTy)) {
             // Force the type of the expression to 'int'.
             Val = ImpCastExprToType(Val, Context.IntTy, CK_IntegralCast).get();
@@ -19939,20 +19951,28 @@ EnumConstantDecl *Sema::CheckEnumConstant(EnumDecl *Enum,
         //       sufficient to contain the incremented value. If no such type
         //       exists, the program is ill-formed.
         QualType T = getNextLargerIntegralType(Context, EltTy);
-        if (T.isNull() || Enum->isFixed()) {
+        if (Enum->isFixed()) {
           // There is no integral type larger enough to represent this
           // value. Complain, then allow the value to wrap around.
           EnumVal = LastEnumConst->getInitVal();
           EnumVal = EnumVal.zext(EnumVal.getBitWidth() * 2);
           ++EnumVal;
-          if (Enum->isFixed())
-            // When the underlying type is fixed, this is ill-formed.
-            Diag(IdLoc, diag::err_enumerator_wrapped)
-              << toString(EnumVal, 10)
-              << EltTy;
-          else
+          // When the underlying type is fixed, this is ill-formed.
+          Diag(IdLoc, diag::err_enumerator_wrapped)
+              << toString(EnumVal, 10) << EltTy;
+
+        } else if (T.isNull()) {
+          if (EltTy->isSignedIntegerType() && getLangOpts().CPlusPlus) {
+            EltTy = Context.getCorrespondingUnsignedType(EltTy);
+          } else {
+            // There is no integral type larger enough to represent this
+            // value. Complain, then allow the value to wrap around.
+            EnumVal = LastEnumConst->getInitVal();
+            EnumVal = EnumVal.zext(EnumVal.getBitWidth() * 2);
+            ++EnumVal;
             Diag(IdLoc, diag::ext_enumerator_increment_too_large)
-              << toString(EnumVal, 10);
+                << toString(EnumVal, 10);
+          }
         } else {
           EltTy = T;
         }
@@ -19970,14 +19990,26 @@ EnumConstantDecl *Sema::CheckEnumConstant(EnumDecl *Enum,
         // an int (C99 6.7.2.2p2). However, we support GCC's extension that
         // permits enumerator values that are representable in some larger
         // integral type.
-        if (!getLangOpts().CPlusPlus && !T.isNull())
+        // starting in C23 standard C allows larger enum values
+        // C23 6.7.2.2p11
+        // - the type of the value from the previous enumeration constant with
+        //   one added to it. If such an integer constant expression would
+        //   overflow or wraparound the value of the previous enumeration
+        //   constant from the addition of one, the type takes on either:
+        // - - a suitably sized signed integer type, excluding the bit-precise
+        //     signed integer types, capable of representing the value of the
+        //     previous enumeration constant plus one; or,
+        // - - a suitably sized unsigned integer type, excluding the bit-precise
+        //     unsigned integer types, capable of representing the value of the
+        //     previous enumeration constant plus one.
+
+        if (!getLangOpts().CPlusPlus && !T.isNull() && !getLangOpts().C23)
           Diag(IdLoc, diag::warn_enum_value_overflow);
-      } else if (!getLangOpts().CPlusPlus &&
-                 !EltTy->isDependentType() &&
-                 !isRepresentableIntegerValue(Context, EnumVal, EltTy)) {
+      } else if (!getLangOpts().CPlusPlus && !EltTy->isDependentType() &&
+                 !isRepresentableIntegerValue(Context, EnumVal, EltTy) &&
+                 !getLangOpts().C23) {
         // Enforce C99 6.7.2.2p2 even when we compute the next value.
-        Diag(IdLoc, diag::ext_enum_value_not_int)
-          << toString(EnumVal, 10) << 1;
+        Diag(IdLoc, diag::ext_enum_value_not_int) << toString(EnumVal, 10) << 1;
       }
     }
   }
@@ -20298,6 +20330,8 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
   // TODO: If the result value doesn't fit in an int, it must be a long or long
   // long value.  ISO C does not support this, but GCC does as an extension,
   // emit a warning.
+  // starting in C23 the type can be any integral type,
+  // but will be an int if it fits in an int
   unsigned IntWidth = Context.getTargetInfo().getIntWidth();
   unsigned CharWidth = Context.getTargetInfo().getCharWidth();
   unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
@@ -20364,8 +20398,25 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
       BestPromotionType = BestType;
 
     BestWidth = Context.getIntWidth(BestType);
-  }
-  else if (NumNegativeBits) {
+  } else if (getLangOpts().C23) {
+    if (Elements.empty()) {
+      if (Packed) {
+        BestType = Context.UnsignedCharTy;
+        BestPromotionType = Context.IntTy;
+        BestWidth = CharWidth;
+      } else {
+        BestType = Context.IntTy;
+        BestWidth = IntWidth;
+        BestPromotionType = BestType;
+      }
+    } else {
+      EnumConstantDecl *ECD = cast_or_null<EnumConstantDecl>(Elements.back());
+      if (ECD) {
+        BestType = ECD->getType();
+        BestWidth = Context.getIntWidth(BestType);
+      }
+    }
+  } else if (NumNegativeBits) {
     // If there is a negative value, figure out the smallest integer type (of
     // int/long/longlong) that fits.
     // If it's packed, check also if it fits a char or a short.
@@ -20453,8 +20504,7 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
     QualType NewTy;
     unsigned NewWidth;
     bool NewSign;
-    if (!getLangOpts().CPlusPlus &&
-        !Enum->isFixed() &&
+    if (!getLangOpts().CPlusPlus && !getLangOpts().C23 && !Enum->isFixed() &&
         isRepresentableIntegerValue(Context, InitVal, Context.IntTy)) {
       NewTy = Context.IntTy;
       NewWidth = IntWidth;
diff --git a/clang/test/C/C2x/n3029.c b/clang/test/C/C2x/n3029.c
new file mode 100644
index 00000000000000..7eaa561520848b
--- /dev/null
+++ b/clang/test/C/C2x/n3029.c
@@ -0,0 +1,81 @@
+// RUN: %clang_cc1 -std=c23 -verify -pedantic %s
+
+/* WG14 N3029: Yes
+ * Improved Normal Enumerations
+ */
+
+ // expected-no-diagnostics
+
+
+enum a {
+	a0 = 0xFFFFFFFFFFFFFFFFULL
+};
+
+
+static_assert(_Generic(a0, unsigned long long: 0, int: 1, default: 2) == 0);
+
+
+// 6.7.2.2.5
+
+// During the processing of each enumeration constant in the enumerator list, the type of the enumeration constant shall be:
+
+// int, if there are no previous enumeration constants in the enumerator list and no explicit = with a defining integer constant expression; or,
+
+
+enum b {
+	b0
+};
+
+
+static_assert(_Generic(b0, int: 1, default: 2) == 1);
+
+// int, if given explicitly with = and the value of the integer constant expression is representable by an int; or,
+
+enum c {
+	c0 = 1
+};
+
+static_assert(_Generic(c0, int: 1, default: 2) == 1);
+
+// the type of the integer constant expression, if given explicitly with = and if the value of the integer constant expression is not representable by int; or,
+
+
+enum d {
+	d0 = 1U
+};
+
+static_assert(_Generic(d0, int: 1, default: 2) == 1);
+
+enum e {
+	e0 = 0XFFFFFFFFU
+};
+
+static_assert(_Generic(e0, int: 1, unsigned int: 2, default: 3) == 2);
+
+enum f {
+	f0 = 0xFFFFFFFFFFFFFFFLL
+};
+
+static_assert(_Generic(f0, int: 1, long long: 2, default: 3) == 2);
+
+// the type of the value from last enumeration constant with 1 added to it. If such an integer constant expression would overflow or wraparound the value of the previous enumeration constant from the addition of 1, the type takes on either:
+
+// a suitably sized signed integer type (excluding the bit-precise signed integer types) capable of representing the value of the previous enumeration constant plus 1; or,
+
+// a suitably sized unsigned integer type (excluding the bit-precise unsigned integer types) capable of representing the value of the previous enumeration constant plus 1.
+
+// A signed integer type is chosen if the previous enumeration constant being added is of signed integer type. An unsigned integer type is chosen if the previous enumeration constant is of unsigned integer type. If there is no suitably sized integer type described previous which can represent the new value, then the enumeration has no type which is capable of representing all of its values).
+
+enum g {
+     g0 = 2147483647, g1 
+};
+
+static_assert(_Generic(g0, int: 1, long: 2, default: 3) == 2);
+static_assert(_Generic(g1, int: 1, long: 2, default: 3) == 2);
+
+enum h {
+     h0 = 4294967295U, h1 
+};
+
+static_assert(_Generic(h0, int: 1, unsigned long: 2, default: 3) == 2);
+static_assert(_Generic(h1, int: 1, unsigned long: 2, default: 3) == 2);
diff --git a/clang/test/C/C2x/n3030.c b/clang/test/C/C2x/n3030.c
new file mode 100644
index 00000000000000..0eea3546494e3e
--- /dev/null
+++ b/clang/test/C/C2x/n3030.c
@@ -0,0 +1,130 @@
+// RUN: %clang_cc1 -std=c23 -verify -pedantic %s
+
+/* WG14 N3030: Partial
+ * Enhancements to Enumerations
+ */
+
+#include <limits.h>
+
+enum a : unsigned long long {
+	a0 = 0xFFFFFFFFFFFFFFFFULL
+	// ^ not a constraint violation with a 64-bit unsigned long long
+};
+
+
+static_assert(_Generic(a0, unsigned long long: 0, default: 1) == 0);
+
+enum e : unsigned short {
+    x
+};
+
+static_assert(_Generic(x, enum e: 0, default: 1) == 0);
+
+static_assert(_Generic(x, unsigned short: 0, default: 1) == 0);
+
+
+static_assert(_Generic(x, enum e: 0, unsigned short: 2, default: 1) == 0); // expected-error {{type 'unsigned short' in generic association compatible with previously specified type 'enum e'}} 
+// expected-note at -1 {{compatible type 'enum e' specified here}}
+
+
+
+enum us : unsigned short {
+	us_max = USHRT_MAX,
+	us_violation, // expected-error {{enumerator value 65536 is not representable in the underlying type 'unsigned short'}} 
+	us_violation_2 = us_max + 1, // expected-error {{enumerator value is not representable in the underlying type 'unsigned short'}}
+	us_wrap_around_to_zero = (unsigned short)(USHRT_MAX + 1) /* Okay: conversion
+	                          done in constant expression before conversion to
+	                          underlying type: unsigned semantics okay. */
+};
+
+
+enum ui : unsigned int {
+	ui_max = UINT_MAX,
+	ui_violation, // expected-error {{enumerator value 4294967296 is not representable in the underlying type 'unsigned int'}} 
+	ui_no_violation = ui_max + 1, /* Okay: Arithmetic performed as typical
+	                                  unsigned integer arithmetic: conversion
+	                                  from a value that is already 0 to 0. */
+	ui_wrap_around_to_zero = (unsigned int)(UINT_MAX + 1) /* Okay: conversion
+	                          done in constant expression before conversion to
+	                          underlying type: unsigned semantics okay. */
+};
+
+static_assert(ui_wrap_around_to_zero + us_wrap_around_to_zero == 0);
+
+enum E1: short;
+enum E2: short; // expected-note {{previous declaration is here}}
+enum E3; // expected-warning {{ISO C forbids forward references to 'enum' types}}
+enum E4 : unsigned long long;
+
+enum E1 : short { m11, m12 };
+enum E1 x1 = m11;
+
+enum E2 : long { m21, m22 }; // expected-error {{enumeration redeclared with different underlying type 'long' (was 'short')}} 
+
+enum E3 { // expected-note {{definition of 'enum E3' is not complete until the closing '}'}}
+// expected-note at -1 {{previous declaration is here}}
+	m31,
+	m32,
+	m33 = sizeof(enum E3) // expected-error {{invalid application of 'sizeof' to an incomplete type 'enum E3'}} 
+};
+enum E3 : int; // expected-error {{enumeration previously declared with nonfixed underlying type}} 
+
+enum E4 : unsigned long long {
+	m40 = sizeof(enum E4),
+	m41 = ULLONG_MAX,
+	m42 // expected-error {{enumerator value 18446744073709551616 is not representable in the underlying type 'unsigned long long'}} 
+};
+
+enum E5 y; // expected-error {{tentative definition has type 'enum E5' that is never completed}}
+// expected-warning at -1 {{ISO C forbids forward references to 'enum' types}}
+// expected-note at -2 {{forward declaration of 'enum E5'}}
+enum E6 : long int z; // expected-error {{non-defining declaration of enumeration with a fixed underlying type is only permitted as a standalone declaration; missing list of enumerators?}}
+enum E7 : long int = 0; // expected-error {{non-defining declaration of enumeration with a fixed underlying type is only permitted as a standalone declaration; missing list of enumerators?}}
+// expected-error at -1 {{expected identifier or '('}}
+
+
+enum underlying : unsigned char {
+	b0
+};
+
+static_assert(_Generic(b0, int: 2, unsigned char: 1, default: 0 ) == 1);
+
+static_assert(_Generic((enum underlying)b0, int: 2, unsigned char: 1, default: 0 ) == 1);
+
+
+void f1 (enum a2 : long b2); // expected-error {{non-defining declaration of enumeration with a fixed underlying type is only permitted as a standalone declaration; missing list of enumerators?}}
+// expected-warning at -1 {{declaration of 'enum a2' will not be visible outside of this function}}
+void f2 (enum c2 : long { x2 } d2);
+// expected-warning at -1 {{declaration of 'enum c2' will not be visible outside of this function}}
+enum e2 : int f3(); // expected-error {{non-defining declaration of enumeration with a fixed underlying type is only permitted as a standalone declaration; missing list of enumerators?}}
+
+typedef enum t u; // expected-warning {{ISO C forbids forward references to 'enum' types}}
+typedef enum v : short W; // expected-error {{non-defining declaration of enumeration with a fixed underlying type is only permitted as a standalone declaration; missing list of enumerators?}}
+typedef enum q : short { s } R;
+
+struct s1 {
+	int x2;
+	enum e2 : int : 1; // expected-error {{non-defining declaration of enumeration with a fixed underlying type is only permitted as a standalone declaration; missing list of enumerators?}}
+	int y2;
+};
+
+enum forward; // expected-warning {{ISO C forbids forward references to 'enum' types}}
+//TODO
+extern enum forward fwd_val0; /* Constraint violation: incomplete type */
+extern enum forward* fwd_ptr0; /* Constraint violation: enums cannot be
+                                  used like other incomplete types */
+// expected-note at -2 {{previous declaration is here}}
+extern int* fwd_ptr0; // expected-error {{redeclaration of 'fwd_ptr0' with a different type: 'int *' vs 'enum forward *'}}
+
+enum forward1 : int;
+extern enum forward1 fwd_val1;
+extern int fwd_val1;
+extern enum forward1* fwd_ptr1;
+extern int* fwd_ptr1;
+
+int foo () {
+	enum e : short;
+	enum e : short f = 0; // expected-error {{non-defining declaration of enumeration with a fixed underlying type is only permitted as a standalone declaration; missing list of enumerators?}}
+	enum g : short { y } h = y;
+	return 0;
+}
diff --git a/clang/test/Sema/enum.c b/clang/test/Sema/enum.c
index b0707914c0d852..2c1cafc74d9cbb 100644
--- a/clang/test/Sema/enum.c
+++ b/clang/test/Sema/enum.c
@@ -1,11 +1,11 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify -pedantic
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -fsyntax-only -std=c23 -verify -pedantic
+
+#if __STDC_VERSION__  < 202311L
 enum e {A,
         B = 42LL << 32,        // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
       C = -4, D = 12456 };
 
-enum f { a = -2147483648, b = 2147483647 }; // ok.
-
 enum g {  // too negative
    c = -2147483649,         // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
    d = 2147483647 };
@@ -19,6 +19,41 @@ enum x                      // expected-warning {{enumeration values exceed rang
 { y = -9223372036854775807LL-1,  // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
 z = 9223372036854775808ULL };    // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
 
+
+enum GH24667 {   GH24667_x = 9223372036854775807UL, };
+// expected-warning at -1 {{ISO C restricts enumerator values to range of 'int' (9223372036854775807 is too large)}}
+
+#else
+enum e {A,
+        B = 42LL << 32,
+      C = -4, D = 12456 };
+
+// these should not raise warnings with C23 
+enum g {
+   c = -2147483649,         
+   d = 2147483647 };
+enum h { e = -2147483648,
+   f = 2147483648,           
+  i = 0xFFFF0000 
+};
+
+// minll maxull
+enum x                      // expected-warning {{enumeration values exceed range of largest integer}}
+{ y = -9223372036854775807LL-1,  
+z = 9223372036854775808ULL }; 
+
+enum GH24667 {   GH24667_x = 9223372036854775807UL, };
+enum N3029 { 	N3029_0 = 0xFFFFFFFFFFFFFFFFULL };
+enum N3030 : unsigned long long {
+	N3030_0 = 0xFFFFFFFFFFFFFFFFULL
+	// ^ not a constraint violation with a 64-bit unsigned long long
+};
+#endif
+
+enum f { a = -2147483648, b = 2147483647 }; // ok.
+
+
+
 int test(void) {
   return sizeof(enum e) ;
 }
@@ -172,10 +207,8 @@ enum class GH42372_2 {
 #if __STDC_VERSION__ >= 202311L
 // FIXME: GCC picks __uint128_t as the underlying type for the enumeration
 // value and Clang picks unsigned long long.
-// FIXME: Clang does not yet implement WG14 N3029, so the warning about
-// restricting enumerator values to 'int' is not correct.
 enum GH59352 { // expected-warning {{enumeration values exceed range of largest integer}}
- BigVal = 66666666666666666666wb // expected-warning {{ISO C restricts enumerator values to range of 'int' (66666666666666666666 is too large)}}
+ BigVal = 66666666666666666666wb
 };
 _Static_assert(BigVal == 66666666666666666666wb); /* expected-error {{static assertion failed due to requirement 'BigVal == 66666666666666666666wb'}}
                                                      expected-note {{expression evaluates to '11326434445538011818 == 66666666666666666666'}}
diff --git a/clang/test/SemaCXX/enum.cpp b/clang/test/SemaCXX/enum.cpp
index c482b3c571ab4a..4d3fc0ca62a7b2 100644
--- a/clang/test/SemaCXX/enum.cpp
+++ b/clang/test/SemaCXX/enum.cpp
@@ -125,3 +125,5 @@ struct PR28903 {
     })
   };
 };
+
+enum GH24667 { GH24667_x = 9223372036854775807 , GH24667_y };



More information about the cfe-commits mailing list