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

Budimir Aranđelović via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 26 06:30:42 PDT 2024


https://github.com/budimirarandjelovicsyrmia updated https://github.com/llvm/llvm-project/pull/70307

>From 9e4309a805f31096d72cb21cd266175cac5b07c1 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia <budimir.arandjelovic at syrmia.com>
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/docs/ReleaseNotes.rst            |  2 ++
 clang/lib/AST/ExprConstant.cpp         |  7 ++++++
 clang/lib/Sema/SemaExpr.cpp            | 35 +++++++++++++++++++++++---
 clang/test/C/drs/dr0xx.c               |  3 ++-
 clang/test/C/drs/dr2xx.c               |  4 ++-
 clang/test/Sema/builtins.c             |  6 +++--
 clang/test/Sema/constant-builtins-2.c  | 12 ++++++---
 clang/test/Sema/integer-overflow.c     |  2 ++
 clang/test/Sema/shift-count-negative.c |  8 ++++++
 clang/test/Sema/shift-count-overflow.c |  9 +++++++
 clang/test/Sema/shift-negative-value.c | 13 ++++++++++
 clang/test/Sema/vla-2.c                |  6 +++--
 clang/test/SemaCXX/enum.cpp            | 16 +++++++-----
 clang/test/SemaCXX/shift.cpp           |  2 +-
 14 files changed, 104 insertions(+), 21 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9ed3ff4507671c..d09cec913fd162 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -626,6 +626,8 @@ Improvements to Clang's diagnostics
   used rather than when they are needed for constant evaluation or when code is generated for them.
   The check is now stricter to prevent crashes for some unsupported declarations (Fixes #GH95495).
 
+- Clang now diagnoses non-C++11 integer constant expressions. Fixes #GH59863
+
 Improvements to Clang's time-trace
 ----------------------------------
 
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index fe4b9a569ab874..5196d85d2a9852 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2858,6 +2858,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;
   }
@@ -2881,6 +2884,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
index 4a2f3a65eac96f..0c7d0fc6173e1a 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11256,7 +11256,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;
   }
 
@@ -11271,7 +11271,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;
   }
 
@@ -11304,7 +11304,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;
   }
 
@@ -17138,11 +17138,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
index 36de32a93da95d..252dc9329c4cad 100644
--- 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
index 1b68b65acca6af..3d53dacc044d02 100644
--- a/clang/test/C/drs/dr2xx.c
+++ b/clang/test/C/drs/dr2xx.c
@@ -277,7 +277,9 @@ void dr258(void) {
 void dr261(void) {
   /* This is still an integer constant expression despite the overflow. */
   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
index 4f843aeec24e69..3afab07db6196d 100644
--- a/clang/test/Sema/builtins.c
+++ b/clang/test/Sema/builtins.c
@@ -171,8 +171,10 @@ 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}}
+                         expected-note {{subexpression not valid in a constant expression}} */
+  ASSERT(!OPT("abcd")); /* expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
+                           expected-note {{subexpression not valid in a constant expression}} */
   // 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
index a60a1f16a45874..00767267cd6c29 100644
--- a/clang/test/Sema/constant-builtins-2.c
+++ b/clang/test/Sema/constant-builtins-2.c
@@ -265,8 +265,10 @@ 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 at -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 at -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];
@@ -274,8 +276,10 @@ char clz60[__builtin_clzg((unsigned _BitInt(128))0x1) == BITSIZE(_BitInt(128)) -
 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 at -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 at -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
index 220fc1bed515a1..3141443c733053 100644
--- 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 at -1 {{variable length array folded to constant array as an extension}}
+                               // expected-note at -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
new file mode 100644
index 00000000000000..97f85feed52c01
--- /dev/null
+++ 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 at -1 {{expression is not an integral constant expression}}
+                 // expected-note at -2 {{negative shift count -29}}
+};
diff --git a/clang/test/Sema/shift-count-overflow.c b/clang/test/Sema/shift-count-overflow.c
new file mode 100644
index 00000000000000..b5186586c2272f
--- /dev/null
+++ 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 at -1 {{expression is not an integral constant expression}}
+                // expected-note at -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
new file mode 100644
index 00000000000000..e7b749c77d88a2
--- /dev/null
+++ 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 at -1 {{expression is not an integral constant expression}}
+                 // expected-note at -2 {{left shift of negative value -1}}
+};
diff --git a/clang/test/Sema/vla-2.c b/clang/test/Sema/vla-2.c
index 316931f2706077..577407e15b4792 100644
--- 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
index c482b3c571ab4a..7d4a05083b9cdb 100644
--- 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 at -2 {{not an integral constant expression}}
-// expected-note at -3 {{value 28958703552 is outside the range of representable values}}
-#else 
-// expected-warning at -5 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
+// expected-warning at -2 {{expression is not an integral constant expression; folding it to a constant is a GNU extension}}
+// expected-note at -3 {{value 28958703552 is outside the range of representable values of type 'int'}}
+#else
+// expected-error at -5 {{expression is not an integral constant expression}}
+// expected-note at -6 {{value 28958703552 is outside the range of representable values of type 'int'}}
+// expected-warning at -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 at -2 {{enumerator value is not a constant expression}}
 // expected-note at -3 {{value 28958703552 is outside the range of representable values}}
 #else
-// expected-warning at -5 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
-// expected-warning at -6 {{extension}}
+// expected-warning at -5 {{enumeration types with a fixed underlying type are a C++11 extension}}
+// expected-warning at -6 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
+// expected-error at -7 {{expression is not an integral constant expression}}
+// expected-note at -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
index 89a98791d3eba7..6367e6d2f250bb 100644
--- 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 at -1 {{implicit conversion}}
+                         // expected-warning at -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;



More information about the cfe-commits mailing list