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

Budimir Aranđelović via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 26 03:42:28 PDT 2024


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

>From 17fb77d094018db20de0ac2a65861338006f9d68 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/lib/Sema/SemaExpr.cpp                | 29 +++++++++++++++-------
 clang/test/AST/Interp/shifts.cpp           |  7 ++----
 clang/test/C/drs/dr0xx.c                   |  2 +-
 clang/test/Sema/shift-count-negative.c     |  8 ++++++
 clang/test/Sema/shift-count-overflow.c     |  6 +++++
 clang/test/Sema/shift-negative-value.c     |  8 ++++++
 clang/test/Sema/vla-2.c                    |  9 ++++---
 clang/test/SemaCXX/cxx2a-explicit-bool.cpp |  2 +-
 clang/test/SemaCXX/shift.cpp               |  1 -
 9 files changed, 52 insertions(+), 20 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/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 94f52004cf6c27..4fef5163c657f8 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11444,9 +11444,12 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
   llvm::APSInt Right = RHSResult.Val.getInt();
 
   if (Right.isNegative()) {
-    S.DiagRuntimeBehavior(Loc, RHS.get(),
-                          S.PDiag(diag::warn_shift_negative)
-                            << RHS.get()->getSourceRange());
+    if (S.ExprEvalContexts.back().isConstantEvaluated())
+      S.Diag(Loc, diag::warn_shift_negative) << RHS.get()->getSourceRange();
+    else
+      S.DiagRuntimeBehavior(Loc, RHS.get(),
+                            S.PDiag(diag::warn_shift_negative)
+                              << RHS.get()->getSourceRange());
     return;
   }
 
@@ -11460,9 +11463,14 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
   }
   llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
   if (Right.uge(LeftBits)) {
-    S.DiagRuntimeBehavior(Loc, RHS.get(),
-                          S.PDiag(diag::warn_shift_gt_typewidth)
-                            << RHS.get()->getSourceRange());
+    if (S.ExprEvalContexts.back().isConstantEvaluated()
+        && !S.getLangOpts().CPlusPlus11)
+      S.Diag(Loc, diag::warn_shift_gt_typewidth)
+        << RHS.get()->getSourceRange();
+    else
+      S.DiagRuntimeBehavior(Loc, RHS.get(),
+                            S.PDiag(diag::warn_shift_gt_typewidth)
+                              << RHS.get()->getSourceRange());
     return;
   }
 
@@ -11493,9 +11501,12 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
   // If LHS does not have a non-negative value then, the
   // behavior is undefined before C++2a. Warn about it.
   if (Left.isNegative()) {
-    S.DiagRuntimeBehavior(Loc, LHS.get(),
-                          S.PDiag(diag::warn_shift_lhs_negative)
-                            << LHS.get()->getSourceRange());
+    if (S.ExprEvalContexts.back().isConstantEvaluated())
+      S.Diag(Loc, diag::warn_shift_lhs_negative) << LHS.get()->getSourceRange();
+    else
+      S.DiagRuntimeBehavior(Loc, LHS.get(),
+                            S.PDiag(diag::warn_shift_lhs_negative)
+                              << LHS.get()->getSourceRange());
     return;
   }
 
diff --git a/clang/test/AST/Interp/shifts.cpp b/clang/test/AST/Interp/shifts.cpp
index b1df7b85cc9f2b..c11ba0f6f3a19e 100644
--- a/clang/test/AST/Interp/shifts.cpp
+++ b/clang/test/AST/Interp/shifts.cpp
@@ -33,13 +33,10 @@ namespace shifts {
                            // FIXME: 'implicit conversion' warning missing in the new interpreter. \
                            // cxx17-warning {{shift count >= width of type}} \
                            // ref-warning {{shift count >= width of type}} \
-                           // ref-warning {{implicit conversion}} \
-                           // ref-cxx17-warning {{shift count >= width of type}} \
-                           // ref-cxx17-warning {{implicit conversion}}
+                           // ref-cxx17-warning {{shift count >= width of type}}
     c = 1 >> (unsigned)-1; // expected-warning {{shift count >= width of type}} \
                            // cxx17-warning {{shift count >= width of type}} \
-                           // ref-warning {{shift count >= width of type}} \
-                           // ref-cxx17-warning {{shift count >= width of type}}
+                           // ref-warning {{shift count >= width of type}}
     c = 1 << c;
     c <<= 0;
     c >>= 0;
diff --git a/clang/test/C/drs/dr0xx.c b/clang/test/C/drs/dr0xx.c
index 6a3717f0729b60..6611b2b65264fa 100644
--- a/clang/test/C/drs/dr0xx.c
+++ b/clang/test/C/drs/dr0xx.c
@@ -426,7 +426,7 @@ 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"); /* Undefined behavior since C99 */
  _Static_assert(1 << 3 == 1u << 3u, "fail"); /* Shift of a positive signed value does sensible things. */
 }
 
diff --git a/clang/test/Sema/shift-count-negative.c b/clang/test/Sema/shift-count-negative.c
new file mode 100644
index 00000000000000..af16332e0b0ee7
--- /dev/null
+++ b/clang/test/Sema/shift-count-negative.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wshift-count-negative %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wshift-count-negative %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
+
+enum shiftof {
+    X = (1<<-29) // expected-warning {{shift count is negative}}
+};
diff --git a/clang/test/Sema/shift-count-overflow.c b/clang/test/Sema/shift-count-overflow.c
new file mode 100644
index 00000000000000..99803cdd8e5252
--- /dev/null
+++ b/clang/test/Sema/shift-count-overflow.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wshift-count-overflow %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wall %s
+
+enum shiftof {
+    X = (1<<32) // expected-warning {{shift count >= width of type}}
+};
diff --git a/clang/test/Sema/shift-negative-value.c b/clang/test/Sema/shift-negative-value.c
new file mode 100644
index 00000000000000..ce6c5f2c5c419e
--- /dev/null
+++ b/clang/test/Sema/shift-negative-value.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wshift-negative-value %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wshift-negative-value %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
+
+enum shiftof {
+    X = (-1<<29) // expected-warning {{shifting a negative signed value is undefined}}
+};
diff --git a/clang/test/Sema/vla-2.c b/clang/test/Sema/vla-2.c
index 316931f2706077..140dd4facb7464 100644
--- a/clang/test/Sema/vla-2.c
+++ b/clang/test/Sema/vla-2.c
@@ -4,14 +4,17 @@
 // 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;
 }
 
 void PotentiallyEvaluatedArrayBoundWarn(int n) {
-  (void)*(int(*)[(0 << 32,n)])0; // expected-warning {{left operand of comma operator has no effect}}
+  (void)*(int(*)[(0 << 32,n)])0; /* expected-warning {{shift count >= width of type}}
+                                    expected-warning {{left operand of comma operator has no effect}} */
 }
diff --git a/clang/test/SemaCXX/cxx2a-explicit-bool.cpp b/clang/test/SemaCXX/cxx2a-explicit-bool.cpp
index ad2a3a1ea75e9f..16b23348197e37 100644
--- a/clang/test/SemaCXX/cxx2a-explicit-bool.cpp
+++ b/clang/test/SemaCXX/cxx2a-explicit-bool.cpp
@@ -21,7 +21,7 @@ namespace special_cases
 template<int a>
 struct A {
 // expected-note at -1+ {{candidate constructor}}
-  explicit(1 << a)
+  explicit(1 << a) // expected-warning {{shift count is negative}}
 // expected-note at -1 {{negative shift count -1}}
 // expected-error at -2 {{explicit specifier argument is not a constant expression}}
   A(int);
diff --git a/clang/test/SemaCXX/shift.cpp b/clang/test/SemaCXX/shift.cpp
index 89a98791d3eba7..c3249b124e926a 100644
--- a/clang/test/SemaCXX/shift.cpp
+++ b/clang/test/SemaCXX/shift.cpp
@@ -22,7 +22,6 @@ 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}}
   c = 1 >> (unsigned)-1; // expected-warning {{shift count >= width of type}}
   c = 1 << c;
   c <<= 0;



More information about the cfe-commits mailing list