Skip to content

Commit 894d650

Browse files
budimirarandjelovichtecbudimirarandjelovichtec
committed
[clang] Emit bad shift warnings
1 parent 6daa983 commit 894d650

File tree

10 files changed

+84
-29
lines changed

10 files changed

+84
-29
lines changed

‎clang/lib/AST/ExprConstant.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2803,9 +2803,10 @@ static bool handleIntIntBinOp(EvalInfo &Info, const Expr *E, const APSInt &LHS,
28032803
// C++11 [expr.shift]p1: Shift width must be less than the bit width of
28042804
// the shifted type.
28052805
unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
2806-
if (SA != RHS) {
2806+
if (SA != RHS && Info.getLangOpts().CPlusPlus11) {
28072807
Info.CCEDiag(E, diag::note_constexpr_large_shift)
2808-
<< RHS << E->getType() << LHS.getBitWidth();
2808+
<< RHS << E->getType() << LHS.getBitWidth();
2809+
return false;
28092810
} else if (LHS.isSigned() && !Info.getLangOpts().CPlusPlus20) {
28102811
// C++11 [expr.shift]p2: A signed left shift must have a non-negative
28112812
// operand, and must not overflow the corresponding unsigned type.
@@ -2836,9 +2837,11 @@ static bool handleIntIntBinOp(EvalInfo &Info, const Expr *E, const APSInt &LHS,
28362837
// C++11 [expr.shift]p1: Shift width must be less than the bit width of the
28372838
// shifted type.
28382839
unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
2839-
if (SA != RHS)
2840+
if (SA != RHS && Info.getLangOpts().CPlusPlus11) {
28402841
Info.CCEDiag(E, diag::note_constexpr_large_shift)
2841-
<< RHS << E->getType() << LHS.getBitWidth();
2842+
<< RHS << E->getType() << LHS.getBitWidth();
2843+
return false;
2844+
}
28422845
Result = LHS >> SA;
28432846
return true;
28442847
}

‎clang/lib/Sema/SemaExpr.cpp

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11428,26 +11428,35 @@ static bool isScopedEnumerationType(QualType T) {
1142811428
return false;
1142911429
}
1143011430

11431-
static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
11432-
SourceLocation Loc, BinaryOperatorKind Opc,
11433-
QualType LHSType) {
11431+
enum BadShiftValueKind {
11432+
BSV_Ok,
11433+
BSV_ShiftIsNegative,
11434+
BSV_ShiftLHSIsNegative,
11435+
BSV_ShiftSizeGTTypeWidth,
11436+
};
11437+
11438+
static BadShiftValueKind DiagnoseBadShiftValues(Sema &S, ExprResult &LHS,
11439+
ExprResult &RHS,
11440+
SourceLocation Loc,
11441+
BinaryOperatorKind Opc,
11442+
QualType LHSType) {
1143411443
// OpenCL 6.3j: shift values are effectively % word size of LHS (more defined),
1143511444
// so skip remaining warnings as we don't want to modify values within Sema.
1143611445
if (S.getLangOpts().OpenCL)
11437-
return;
11446+
return BSV_Ok;
1143811447

1143911448
// Check right/shifter operand
1144011449
Expr::EvalResult RHSResult;
1144111450
if (RHS.get()->isValueDependent() ||
1144211451
!RHS.get()->EvaluateAsInt(RHSResult, S.Context))
11443-
return;
11452+
return BSV_Ok;
1144411453
llvm::APSInt Right = RHSResult.Val.getInt();
1144511454

1144611455
if (Right.isNegative()) {
1144711456
S.DiagRuntimeBehavior(Loc, RHS.get(),
1144811457
S.PDiag(diag::warn_shift_negative)
1144911458
<< RHS.get()->getSourceRange());
11450-
return;
11459+
return BSV_ShiftIsNegative;
1145111460
}
1145211461

1145311462
QualType LHSExprType = LHS.get()->getType();
@@ -11463,12 +11472,12 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
1146311472
S.DiagRuntimeBehavior(Loc, RHS.get(),
1146411473
S.PDiag(diag::warn_shift_gt_typewidth)
1146511474
<< RHS.get()->getSourceRange());
11466-
return;
11475+
return BSV_ShiftSizeGTTypeWidth;
1146711476
}
1146811477

1146911478
// FIXME: We probably need to handle fixed point types specially here.
1147011479
if (Opc != BO_Shl || LHSExprType->isFixedPointType())
11471-
return;
11480+
return BSV_Ok;
1147211481

1147311482
// When left shifting an ICE which is signed, we can check for overflow which
1147411483
// according to C++ standards prior to C++2a has undefined behavior
@@ -11480,29 +11489,29 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
1148011489
if (LHS.get()->isValueDependent() ||
1148111490
LHSType->hasUnsignedIntegerRepresentation() ||
1148211491
!LHS.get()->EvaluateAsInt(LHSResult, S.Context))
11483-
return;
11492+
return BSV_Ok;
1148411493
llvm::APSInt Left = LHSResult.Val.getInt();
1148511494

1148611495
// Don't warn if signed overflow is defined, then all the rest of the
1148711496
// diagnostics will not be triggered because the behavior is defined.
1148811497
// Also don't warn in C++20 mode (and newer), as signed left shifts
1148911498
// always wrap and never overflow.
1149011499
if (S.getLangOpts().isSignedOverflowDefined() || S.getLangOpts().CPlusPlus20)
11491-
return;
11500+
return BSV_Ok;
1149211501

1149311502
// If LHS does not have a non-negative value then, the
1149411503
// behavior is undefined before C++2a. Warn about it.
1149511504
if (Left.isNegative()) {
1149611505
S.DiagRuntimeBehavior(Loc, LHS.get(),
1149711506
S.PDiag(diag::warn_shift_lhs_negative)
1149811507
<< LHS.get()->getSourceRange());
11499-
return;
11508+
return BSV_ShiftLHSIsNegative;
1150011509
}
1150111510

1150211511
llvm::APInt ResultBits =
1150311512
static_cast<llvm::APInt&>(Right) + Left.getMinSignedBits();
1150411513
if (LeftBits.uge(ResultBits))
11505-
return;
11514+
return BSV_Ok;
1150611515
llvm::APSInt Result = Left.extend(ResultBits.getLimitedValue());
1150711516
Result = Result.shl(Right);
1150811517

@@ -11519,13 +11528,17 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
1151911528
S.Diag(Loc, diag::warn_shift_result_sets_sign_bit)
1152011529
<< HexResult << LHSType
1152111530
<< LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
11522-
return;
11531+
11532+
// We return BSV_Ok because we already emitted the diagnostic.
11533+
return BSV_Ok;
1152311534
}
1152411535

1152511536
S.Diag(Loc, diag::warn_shift_result_gt_typewidth)
1152611537
<< HexResult.str() << Result.getMinSignedBits() << LHSType
1152711538
<< Left.getBitWidth() << LHS.get()->getSourceRange()
1152811539
<< RHS.get()->getSourceRange();
11540+
11541+
return BSV_Ok;
1152911542
}
1153011543

1153111544
/// Return the resulting type when a vector is shifted
@@ -11773,7 +11786,25 @@ QualType Sema::CheckShiftOperands(ExprResult &LHS, ExprResult &RHS,
1177311786
isScopedEnumerationType(RHSType)) {
1177411787
return InvalidOperands(Loc, LHS, RHS);
1177511788
}
11776-
DiagnoseBadShiftValues(*this, LHS, RHS, Loc, Opc, LHSType);
11789+
11790+
BadShiftValueKind BSVKind =
11791+
DiagnoseBadShiftValues(*this, LHS, RHS, Loc, Opc, LHSType);
11792+
if (ExprEvalContexts.back().isConstantEvaluated()) {
11793+
switch (BSVKind) {
11794+
case BSV_ShiftSizeGTTypeWidth:
11795+
if (!getLangOpts().CPlusPlus11)
11796+
Diag(Loc, diag::warn_shift_gt_typewidth) << RHS.get()->getSourceRange();
11797+
break;
11798+
case BSV_ShiftIsNegative:
11799+
Diag(Loc, diag::warn_shift_negative) << RHS.get()->getSourceRange();
11800+
break;
11801+
case BSV_ShiftLHSIsNegative:
11802+
Diag(Loc, diag::warn_shift_lhs_negative) << RHS.get()->getSourceRange();
11803+
break;
11804+
default:
11805+
break;
11806+
}
11807+
}
1177711808

1177811809
// "The type of the result is that of the promoted left operand."
1177911810
return LHSType;

‎clang/test/AST/Interp/shifts.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,10 @@ namespace shifts {
3333
// FIXME: 'implicit conversion' warning missing in the new interpreter. \
3434
// cxx17-warning {{shift count >= width of type}} \
3535
// ref-warning {{shift count >= width of type}} \
36-
// ref-warning {{implicit conversion}} \
37-
// ref-cxx17-warning {{shift count >= width of type}} \
38-
// ref-cxx17-warning {{implicit conversion}}
36+
// ref-cxx17-warning {{shift count >= width of type}}
3937
c = 1 >> (unsigned)-1; // expected-warning {{shift count >= width of type}} \
4038
// cxx17-warning {{shift count >= width of type}} \
41-
// ref-warning {{shift count >= width of type}} \
42-
// ref-cxx17-warning {{shift count >= width of type}}
39+
// ref-warning {{shift count >= width of type}}
4340
c = 1 << c;
4441
c <<= 0;
4542
c >>= 0;

‎clang/test/C/drs/dr0xx.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ void dr081(void) {
426426
/* Demonstrate that we don't crash when left shifting a signed value; that's
427427
* implementation defined behavior.
428428
*/
429-
_Static_assert(-1 << 1 == -2, "fail"); /* Didn't shift a zero into the "sign bit". */
429+
_Static_assert(-1 << 1 == -2, "fail"); /* expected-warning {{shifting a negative signed value is undefined}} */
430430
_Static_assert(1 << 3 == 1u << 3u, "fail"); /* Shift of a positive signed value does sensible things. */
431431
}
432432

‎clang/test/Sema/shift-count-negative.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wshift-count-negative %s
2+
// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
3+
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wshift-count-negative %s
4+
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
5+
6+
enum shiftof {
7+
X = (1<<-29) //expected-warning {{shift count is negative}}
8+
};

‎clang/test/Sema/shift-count-overflow.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// RUN: %clang_cc1 -fsyntax-only -verify -Wshift-count-overflow %s
2+
// RUN: %clang_cc1 -fsyntax-only -verify -Wall %s
3+
4+
enum shiftof {
5+
X = (1<<32) // expected-warning {{shift count >= width of type}}
6+
};

‎clang/test/Sema/shift-negative-value.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wshift-negative-value %s
2+
// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
3+
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wshift-negative-value %s
4+
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
5+
6+
enum shiftof {
7+
X = (-1<<29) //expected-warning {{shifting a negative signed value is undefined}}
8+
};

‎clang/test/Sema/vla-2.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,17 @@
44
// a different codepath when we have already emitted an error.)
55

66
int PotentiallyEvaluatedSizeofWarn(int n) {
7-
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}}
7+
return (int)sizeof *(0 << 32,(int(*)[n])0); /* expected-warning {{shift count >= width of type}}
8+
expected-warning {{left operand of comma operator has no effect}} */
89
}
910

1011
void PotentiallyEvaluatedTypeofWarn(int n) {
11-
__typeof(*(0 << 32,(int(*)[n])0)) x; // expected-warning {{left operand of comma operator has no effect}} expected-warning {{shift count >= width of type}}
12+
__typeof(*(0 << 32,(int(*)[n])0)) x; /* expected-warning {{shift count >= width of type}}
13+
expected-warning {{left operand of comma operator has no effect}} */
1214
(void)x;
1315
}
1416

1517
void PotentiallyEvaluatedArrayBoundWarn(int n) {
16-
(void)*(int(*)[(0 << 32,n)])0; // expected-warning {{left operand of comma operator has no effect}}
18+
(void)*(int(*)[(0 << 32,n)])0; /* expected-warning {{shift count >= width of type}}
19+
expected-warning {{left operand of comma operator has no effect}} */
1720
}

‎clang/test/SemaCXX/cxx2a-explicit-bool.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ namespace special_cases
2121
template<int a>
2222
struct A {
2323
// expected-note@-1+ {{candidate constructor}}
24-
explicit(1 << a)
24+
explicit(1 << a) // expected-warning {{shift count is negative}}
2525
// expected-note@-1 {{negative shift count -1}}
2626
// expected-error@-2 {{explicit specifier argument is not a constant expression}}
2727
A(int);

‎clang/test/SemaCXX/shift.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ void test() {
2222
c = 1 << -1; // expected-warning {{shift count is negative}}
2323
c = 1 >> -1; // expected-warning {{shift count is negative}}
2424
c = 1 << (unsigned)-1; // expected-warning {{shift count >= width of type}}
25-
// expected-warning@-1 {{implicit conversion}}
2625
c = 1 >> (unsigned)-1; // expected-warning {{shift count >= width of type}}
2726
c = 1 << c;
2827
c <<= 0;

0 commit comments

Comments
 (0)