[clang] [clang] Emit bad shift warnings (PR #70307)
Budimir Aranđelović via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 26 01:49:18 PDT 2023
https://github.com/budimirarandjelovicsyrmia created https://github.com/llvm/llvm-project/pull/70307
Diagnose bad shifts and emit warnings
>From 894d6503b0e1ced73bab65b0454abfc65d5b8a99 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/AST/ExprConstant.cpp | 11 ++--
clang/lib/Sema/SemaExpr.cpp | 59 +++++++++++++++++-----
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 -
10 files changed, 84 insertions(+), 29 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/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 78cfecbec9fd363..52ee2655c18f994 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2803,9 +2803,10 @@ static bool handleIntIntBinOp(EvalInfo &Info, const Expr *E, const APSInt &LHS,
// C++11 [expr.shift]p1: Shift width must be less than the bit width of
// the shifted type.
unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
- if (SA != RHS) {
+ if (SA != RHS && Info.getLangOpts().CPlusPlus11) {
Info.CCEDiag(E, diag::note_constexpr_large_shift)
- << RHS << E->getType() << LHS.getBitWidth();
+ << RHS << E->getType() << LHS.getBitWidth();
+ return false;
} else if (LHS.isSigned() && !Info.getLangOpts().CPlusPlus20) {
// C++11 [expr.shift]p2: A signed left shift must have a non-negative
// operand, and must not overflow the corresponding unsigned type.
@@ -2836,9 +2837,11 @@ static bool handleIntIntBinOp(EvalInfo &Info, const Expr *E, const APSInt &LHS,
// C++11 [expr.shift]p1: Shift width must be less than the bit width of the
// shifted type.
unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
- if (SA != RHS)
+ if (SA != RHS && Info.getLangOpts().CPlusPlus11) {
Info.CCEDiag(E, diag::note_constexpr_large_shift)
- << RHS << E->getType() << LHS.getBitWidth();
+ << RHS << E->getType() << LHS.getBitWidth();
+ return false;
+ }
Result = LHS >> SA;
return true;
}
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 94f52004cf6c27a..8d3768c06915ce9 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11428,26 +11428,35 @@ static bool isScopedEnumerationType(QualType T) {
return false;
}
-static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
- SourceLocation Loc, BinaryOperatorKind Opc,
- QualType LHSType) {
+enum BadShiftValueKind {
+ BSV_Ok,
+ BSV_ShiftIsNegative,
+ BSV_ShiftLHSIsNegative,
+ BSV_ShiftSizeGTTypeWidth,
+};
+
+static BadShiftValueKind DiagnoseBadShiftValues(Sema &S, ExprResult &LHS,
+ ExprResult &RHS,
+ SourceLocation Loc,
+ BinaryOperatorKind Opc,
+ QualType LHSType) {
// OpenCL 6.3j: shift values are effectively % word size of LHS (more defined),
// so skip remaining warnings as we don't want to modify values within Sema.
if (S.getLangOpts().OpenCL)
- return;
+ return BSV_Ok;
// Check right/shifter operand
Expr::EvalResult RHSResult;
if (RHS.get()->isValueDependent() ||
!RHS.get()->EvaluateAsInt(RHSResult, S.Context))
- return;
+ return BSV_Ok;
llvm::APSInt Right = RHSResult.Val.getInt();
if (Right.isNegative()) {
S.DiagRuntimeBehavior(Loc, RHS.get(),
S.PDiag(diag::warn_shift_negative)
<< RHS.get()->getSourceRange());
- return;
+ return BSV_ShiftIsNegative;
}
QualType LHSExprType = LHS.get()->getType();
@@ -11463,12 +11472,12 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
S.DiagRuntimeBehavior(Loc, RHS.get(),
S.PDiag(diag::warn_shift_gt_typewidth)
<< RHS.get()->getSourceRange());
- return;
+ return BSV_ShiftSizeGTTypeWidth;
}
// FIXME: We probably need to handle fixed point types specially here.
if (Opc != BO_Shl || LHSExprType->isFixedPointType())
- return;
+ return BSV_Ok;
// When left shifting an ICE which is signed, we can check for overflow which
// according to C++ standards prior to C++2a has undefined behavior
@@ -11480,7 +11489,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
if (LHS.get()->isValueDependent() ||
LHSType->hasUnsignedIntegerRepresentation() ||
!LHS.get()->EvaluateAsInt(LHSResult, S.Context))
- return;
+ return BSV_Ok;
llvm::APSInt Left = LHSResult.Val.getInt();
// Don't warn if signed overflow is defined, then all the rest of the
@@ -11488,7 +11497,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
// Also don't warn in C++20 mode (and newer), as signed left shifts
// always wrap and never overflow.
if (S.getLangOpts().isSignedOverflowDefined() || S.getLangOpts().CPlusPlus20)
- return;
+ return BSV_Ok;
// If LHS does not have a non-negative value then, the
// behavior is undefined before C++2a. Warn about it.
@@ -11496,13 +11505,13 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
S.DiagRuntimeBehavior(Loc, LHS.get(),
S.PDiag(diag::warn_shift_lhs_negative)
<< LHS.get()->getSourceRange());
- return;
+ return BSV_ShiftLHSIsNegative;
}
llvm::APInt ResultBits =
static_cast<llvm::APInt&>(Right) + Left.getMinSignedBits();
if (LeftBits.uge(ResultBits))
- return;
+ return BSV_Ok;
llvm::APSInt Result = Left.extend(ResultBits.getLimitedValue());
Result = Result.shl(Right);
@@ -11519,13 +11528,17 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
S.Diag(Loc, diag::warn_shift_result_sets_sign_bit)
<< HexResult << LHSType
<< LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
- return;
+
+ // We return BSV_Ok because we already emitted the diagnostic.
+ return BSV_Ok;
}
S.Diag(Loc, diag::warn_shift_result_gt_typewidth)
<< HexResult.str() << Result.getMinSignedBits() << LHSType
<< Left.getBitWidth() << LHS.get()->getSourceRange()
<< RHS.get()->getSourceRange();
+
+ return BSV_Ok;
}
/// Return the resulting type when a vector is shifted
@@ -11773,7 +11786,25 @@ QualType Sema::CheckShiftOperands(ExprResult &LHS, ExprResult &RHS,
isScopedEnumerationType(RHSType)) {
return InvalidOperands(Loc, LHS, RHS);
}
- DiagnoseBadShiftValues(*this, LHS, RHS, Loc, Opc, LHSType);
+
+ BadShiftValueKind BSVKind =
+ DiagnoseBadShiftValues(*this, LHS, RHS, Loc, Opc, LHSType);
+ if (ExprEvalContexts.back().isConstantEvaluated()) {
+ switch (BSVKind) {
+ case BSV_ShiftSizeGTTypeWidth:
+ if (!getLangOpts().CPlusPlus11)
+ Diag(Loc, diag::warn_shift_gt_typewidth) << RHS.get()->getSourceRange();
+ break;
+ case BSV_ShiftIsNegative:
+ Diag(Loc, diag::warn_shift_negative) << RHS.get()->getSourceRange();
+ break;
+ case BSV_ShiftLHSIsNegative:
+ Diag(Loc, diag::warn_shift_lhs_negative) << RHS.get()->getSourceRange();
+ break;
+ default:
+ break;
+ }
+ }
// "The type of the result is that of the promoted left operand."
return LHSType;
diff --git a/clang/test/AST/Interp/shifts.cpp b/clang/test/AST/Interp/shifts.cpp
index b1df7b85cc9f2bd..c11ba0f6f3a19e9 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 6a3717f0729b607..1425ea30fc2ad09 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"); /* expected-warning {{shifting a negative signed value is undefined}} */
_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 000000000000000..ecda778f5dd8aad
--- /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 000000000000000..99803cdd8e52529
--- /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 000000000000000..82a1094f3c1414f
--- /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 316931f2706077d..140dd4facb74649 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 ad2a3a1ea75e9fb..16b23348197e37e 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 89a98791d3eba79..c3249b124e926ae 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