[clang] Emit bad shift warnings (llvm#70307)

Diagnose bad shifts and emit warnings
budimirarandjelovicsyrmia · Jul 11, 2024 · e4163c0 · e4163c0
1 parent d69e949
commit e4163c0
Show file tree

Hide file tree

Showing 14 changed files with 104 additions and 22 deletions.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
@@ -705,6 +705,8 @@ Improvements to Clang's diagnostics
 
 - For the ARM target, calling an interrupt handler from another function is now an error. #GH95359.
 
+- Clang now diagnoses integer constant expressions that are folded to a constant value as an extension in more circumstances. Fixes #GH59863
+
 Improvements to Clang's time-trace
 ----------------------------------
 

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
@@ -2859,6 +2859,9 @@ static bool handleIntIntBinOp(EvalInfo &Info, const BinaryOperator *E,
       else if (LHS.countl_zero() < SA)
         Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
     }
+    if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+        Info.getLangOpts().CPlusPlus11)
+      return false;
     Result = LHS << SA;
     return true;
   }
@@ -2882,6 +2885,10 @@ static bool handleIntIntBinOp(EvalInfo &Info, const BinaryOperator *E,
     if (SA != RHS)
       Info.CCEDiag(E, diag::note_constexpr_large_shift)
         << RHS << E->getType() << LHS.getBitWidth();
+
+    if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+        Info.getLangOpts().CPlusPlus11)
+      return false;
     Result = LHS >> SA;
     return true;
   }

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
@@ -11078,7 +11078,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
   if (Right.isNegative()) {
     S.DiagRuntimeBehavior(Loc, RHS.get(),
                           S.PDiag(diag::warn_shift_negative)
-                            << RHS.get()->getSourceRange());
+                              << RHS.get()->getSourceRange());
     return;
   }
 
@@ -11093,7 +11093,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
   if (Right.uge(LeftSize)) {
     S.DiagRuntimeBehavior(Loc, RHS.get(),
                           S.PDiag(diag::warn_shift_gt_typewidth)
-                            << RHS.get()->getSourceRange());
+                              << RHS.get()->getSourceRange());
     return;
   }
 
@@ -11126,7 +11126,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
   if (Left.isNegative()) {
     S.DiagRuntimeBehavior(Loc, LHS.get(),
                           S.PDiag(diag::warn_shift_lhs_negative)
-                            << LHS.get()->getSourceRange());
+                              << LHS.get()->getSourceRange());
     return;
   }
 
@@ -16964,11 +16964,38 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+    SmallVector<PartialDiagnosticAt, 8> Notes;
     if (Result)
-      *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+      *Result = E->EvaluateKnownConstIntCheckOverflow(Context, &Notes);
     if (!isa<ConstantExpr>(E))
       E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
                  : ConstantExpr::Create(Context, E);
+
+    if (Notes.empty())
+      return E;
+
+    // If our only note is the usual "invalid subexpression" note, just point
+    // the caret at its location rather than producing an essentially
+    // redundant note.
+    if (Notes.size() == 1 && Notes[0].second.getDiagID() ==
+          diag::note_invalid_subexpr_in_const_expr) {
+      DiagLoc = Notes[0].first;
+      Notes.clear();
+    }
+
+    if (getLangOpts().CPlusPlus) {
+      if (!Diagnoser.Suppress) {
+        Diagnoser.diagnoseNotICE(*this, DiagLoc) << E->getSourceRange();
+        for (const PartialDiagnosticAt &Note : Notes)
+          Diag(Note.first, Note.second);
+      }
+      return ExprError();
+    }
+
+    Diagnoser.diagnoseFold(*this, DiagLoc) << E->getSourceRange();
+    for (const PartialDiagnosticAt &Note : Notes)
+      Diag(Note.first, Note.second);
+
     return E;
   }
 

diff --git a/clang/test/C/drs/dr0xx.c b/clang/test/C/drs/dr0xx.c
@@ -430,7 +430,8 @@ void dr081(void) {
   /* Demonstrate that we don't crash when left shifting a signed value; that's
    * implementation defined behavior.
    */
- _Static_assert(-1 << 1 == -2, "fail"); /* Didn't shift a zero into the "sign bit". */
+ _Static_assert(-1 << 1 == -2, "fail"); /* expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
+                                           expected-note {{left shift of negative value -1}} */
  _Static_assert(1 << 3 == 1u << 3u, "fail"); /* Shift of a positive signed value does sensible things. */
 }
 

diff --git a/clang/test/C/drs/dr2xx.c b/clang/test/C/drs/dr2xx.c
@@ -275,9 +275,12 @@ void dr258(void) {
  * Constant expressions
  */
 void dr261(void) {
-  /* This is still an integer constant expression despite the overflow. */
+  /* This is not an integer constant expression because of the overflow,
+   * but we fold it as a constant expression anyway as a GNU extension. */
   enum e1 {
-    ex1 = __INT_MAX__ + 1  /* expected-warning {{overflow in expression; result is -2'147'483'648 with type 'int'}} */
+    ex1 = __INT_MAX__ + 1  /* expected-warning {{overflow in expression; result is -2'147'483'648 with type 'int'}}
+                              expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
+                              expected-note {{value 2147483648 is outside the range of representable values of type 'int'}} */
   };
 
   /* This is not an integer constant expression, because of the comma operator,

diff --git a/clang/test/Sema/builtins.c b/clang/test/Sema/builtins.c
@@ -171,8 +171,8 @@ void test17(void) {
 #define OPT(...) (__builtin_constant_p(__VA_ARGS__) && strlen(__VA_ARGS__) < 4)
   // FIXME: These are incorrectly treated as ICEs because strlen is treated as
   // a builtin.
-  ASSERT(OPT("abc"));
-  ASSERT(!OPT("abcd"));
+  ASSERT(OPT("abc")); // expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
+  ASSERT(!OPT("abcd")); // expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
   // In these cases, the strlen is non-constant, but the __builtin_constant_p
   // is 0: the array size is not an ICE but is foldable.
   ASSERT(!OPT(test17_c));

diff --git a/clang/test/Sema/constant-builtins-2.c b/clang/test/Sema/constant-builtins-2.c
@@ -265,17 +265,21 @@ char clz52[__builtin_clzg((unsigned __int128)0x1) == BITSIZE(__int128) - 1 ? 1 :
 char clz53[__builtin_clzg((unsigned __int128)0x1, 42) == BITSIZE(__int128) - 1 ? 1 : -1];
 char clz54[__builtin_clzg((unsigned __int128)0xf) == BITSIZE(__int128) - 4 ? 1 : -1];
 char clz55[__builtin_clzg((unsigned __int128)0xf, 42) == BITSIZE(__int128) - 4 ? 1 : -1];
-char clz56[__builtin_clzg((unsigned __int128)(1 << (BITSIZE(__int128) - 1))) == 0 ? 1 : -1];
-char clz57[__builtin_clzg((unsigned __int128)(1 << (BITSIZE(__int128) - 1)), 42) == 0 ? 1 : -1];
+char clz56[__builtin_clzg((unsigned __int128)(1 << (BITSIZE(__int128) - 1))) == 0 ? 1 : -1]; // expected-warning {{variable length array folded to constant array as an extension}}
+                                                                                             // expected-note@-1 {{shift count 127 >= width of type 'int' (32 bits)}}
+char clz57[__builtin_clzg((unsigned __int128)(1 << (BITSIZE(__int128) - 1)), 42) == 0 ? 1 : -1]; // expected-warning {{variable length array folded to constant array as an extension}}
+                                                                                                 // expected-note@-1 {{shift count 127 >= width of type 'int' (32 bits)}}
 #endif
 int clz58 = __builtin_clzg((unsigned _BitInt(128))0); // expected-error {{not a compile-time constant}}
 char clz59[__builtin_clzg((unsigned _BitInt(128))0, 42) == 42 ? 1 : -1];
 char clz60[__builtin_clzg((unsigned _BitInt(128))0x1) == BITSIZE(_BitInt(128)) - 1 ? 1 : -1];
 char clz61[__builtin_clzg((unsigned _BitInt(128))0x1, 42) == BITSIZE(_BitInt(128)) - 1 ? 1 : -1];
 char clz62[__builtin_clzg((unsigned _BitInt(128))0xf) == BITSIZE(_BitInt(128)) - 4 ? 1 : -1];
 char clz63[__builtin_clzg((unsigned _BitInt(128))0xf, 42) == BITSIZE(_BitInt(128)) - 4 ? 1 : -1];
-char clz64[__builtin_clzg((unsigned _BitInt(128))(1 << (BITSIZE(_BitInt(128)) - 1))) == 0 ? 1 : -1];
-char clz65[__builtin_clzg((unsigned _BitInt(128))(1 << (BITSIZE(_BitInt(128)) - 1)), 42) == 0 ? 1 : -1];
+char clz64[__builtin_clzg((unsigned _BitInt(128))(1 << (BITSIZE(_BitInt(128)) - 1))) == 0 ? 1 : -1]; // expected-warning {{variable length array folded to constant array as an extension}}
+                                                                                                     // expected-note@-1 {{shift count 127 >= width of type 'int' (32 bits)}}
+char clz65[__builtin_clzg((unsigned _BitInt(128))(1 << (BITSIZE(_BitInt(128)) - 1)), 42) == 0 ? 1 : -1]; // expected-warning {{variable length array folded to constant array as an extension}}
+                                                                                                         // expected-note@-1 {{shift count 127 >= width of type 'int' (32 bits)}}
 
 char ctz1[__builtin_ctz(1) == 0 ? 1 : -1];
 char ctz2[__builtin_ctz(8) == 3 ? 1 : -1];

diff --git a/clang/test/Sema/integer-overflow.c b/clang/test/Sema/integer-overflow.c
@@ -174,6 +174,8 @@ void check_integer_overflows_in_function_calls(void) {
 }
 void check_integer_overflows_in_array_size(void) {
   int arr[4608 * 1024 * 1024]; // expected-warning {{overflow in expression; result is 536'870'912 with type 'int'}}
+                               // expected-warning@-1 {{variable length array folded to constant array as an extension}}
+                               // expected-note@-2 {{value 4831838208 is outside the range of representable values of type 'int'}}
 }
 
 struct s {

diff --git a/clang/test/Sema/shift-count-negative.c b/clang/test/Sema/shift-count-negative.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify=expected,c -pedantic %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=expected,cpp %s
+
+enum shiftof {
+    X = (1<<-29) // c-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
+                 // cpp-error@-1 {{expression is not an integral constant expression}}
+                 // expected-note@-2 {{negative shift count -29}}
+};
diff --git a/clang/test/Sema/shift-count-overflow.c b/clang/test/Sema/shift-count-overflow.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,c -pedantic %s
+// RUN: %clang_cc1 -x c++ -std=c++98 -fsyntax-only -verify=expected,cpp %s
+// RUN: %clang_cc1 -x c++ -std=c++11 -fsyntax-only -verify=expected,cpp %s
+
+enum shiftof {
+    X = (1<<32) // c-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
+                // cpp-error@-1 {{expression is not an integral constant expression}}
+                // expected-note@-2 {{shift count 32 >= width of type 'int'}}
+};
diff --git a/clang/test/Sema/shift-negative-value.c b/clang/test/Sema/shift-negative-value.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify=expected,c -pedantic %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=expected,cpp -Wshift-negative-value %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=expected,cpp -Wall %s
+// RUN: %clang_cc1 -x c++ -std=c++98 -fsyntax-only -verify=expected,cpp -Wshift-negative-value %s
+// RUN: %clang_cc1 -x c++ -std=c++98 -fsyntax-only -verify=expected,cpp -Wall %s
+// RUN: %clang_cc1 -x c++ -std=c++11 -fsyntax-only -verify=expected,cpp -Wshift-negative-value %s
+// RUN: %clang_cc1 -x c++ -std=c++11 -fsyntax-only -verify=expected,cpp -Wall %s
+
+enum shiftof {
+    X = (-1<<29) // c-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
+                 // cpp-error@-1 {{expression is not an integral constant expression}}
+                 // expected-note@-2 {{left shift of negative value -1}}
+};
diff --git a/clang/test/Sema/vla-2.c b/clang/test/Sema/vla-2.c
@@ -4,11 +4,13 @@
 // a different codepath when we have already emitted an error.)
 
 int PotentiallyEvaluatedSizeofWarn(int n) {
-  return (int)sizeof *(0 << 32,(int(*)[n])0); // expected-warning {{left operand of comma operator has no effect}} expected-warning {{shift count >= width of type}}
+  return (int)sizeof *(0 << 32,(int(*)[n])0); /* expected-warning {{shift count >= width of type}}
+                                                 expected-warning {{left operand of comma operator has no effect}} */
 }
 
 void PotentiallyEvaluatedTypeofWarn(int n) {
-  __typeof(*(0 << 32,(int(*)[n])0)) x; // expected-warning {{left operand of comma operator has no effect}} expected-warning {{shift count >= width of type}}
+  __typeof(*(0 << 32,(int(*)[n])0)) x; /* expected-warning {{shift count >= width of type}}
+                                          expected-warning {{left operand of comma operator has no effect}} */
   (void)x;
 }
 

diff --git a/clang/test/SemaCXX/enum.cpp b/clang/test/SemaCXX/enum.cpp
@@ -100,10 +100,12 @@ void PR8089() {
 // expressions with UB to be non-constant.
 enum { overflow = 123456 * 234567 };
 #if __cplusplus >= 201103L
-// expected-warning@-2 {{not an integral constant expression}}
-// expected-note@-3 {{value 28958703552 is outside the range of representable values}}
-#else 
-// expected-warning@-5 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
+// expected-warning@-2 {{expression is not an integral constant expression; folding it to a constant is a GNU extension}}
+// expected-note@-3 {{value 28958703552 is outside the range of representable values of type 'int'}}
+#else
+// expected-error@-5 {{expression is not an integral constant expression}}
+// expected-note@-6 {{value 28958703552 is outside the range of representable values of type 'int'}}
+// expected-warning@-7 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
 #endif
 
 // FIXME: This is not consistent with the above case.
@@ -112,8 +114,10 @@ enum NoFold : int { overflow2 = 123456 * 234567 };
 // expected-error@-2 {{enumerator value is not a constant expression}}
 // expected-note@-3 {{value 28958703552 is outside the range of representable values}}
 #else
-// expected-warning@-5 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
-// expected-warning@-6 {{extension}}
+// expected-warning@-5 {{enumeration types with a fixed underlying type are a C++11 extension}}
+// expected-warning@-6 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
+// expected-error@-7 {{expression is not an integral constant expression}}
+// expected-note@-8 {{value 28958703552 is outside the range of representable values of type 'int'}}
 #endif
 
 // PR28903

diff --git a/clang/test/SemaCXX/shift.cpp b/clang/test/SemaCXX/shift.cpp
@@ -22,7 +22,7 @@ void test() {
   c = 1 << -1; // expected-warning {{shift count is negative}}
   c = 1 >> -1; // expected-warning {{shift count is negative}}
   c = 1 << (unsigned)-1; // expected-warning {{shift count >= width of type}}
-                         // expected-warning@-1 {{implicit conversion}}
+                         // expected-warning@-1 {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}}
   c = 1 >> (unsigned)-1; // expected-warning {{shift count >= width of type}}
   c = 1 << c;
   c <<= 0;