Skip to content

Commit e4163c0

Browse files
budimirarandjelovichtecbudimirarandjelovichtec
authored
[clang] Emit bad shift warnings (#70307)
Diagnose bad shifts and emit warnings
1 parent d69e949 commit e4163c0

14 files changed

+104
-22
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,8 @@ Improvements to Clang's diagnostics
705705

706706
- For the ARM target, calling an interrupt handler from another function is now an error. #GH95359.
707707

708+
- Clang now diagnoses integer constant expressions that are folded to a constant value as an extension in more circumstances. Fixes #GH59863
709+
708710
Improvements to Clang's time-trace
709711
----------------------------------
710712

‎clang/lib/AST/ExprConstant.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2859,6 +2859,9 @@ static bool handleIntIntBinOp(EvalInfo &Info, const BinaryOperator *E,
28592859
else if (LHS.countl_zero() < SA)
28602860
Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
28612861
}
2862+
if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
2863+
Info.getLangOpts().CPlusPlus11)
2864+
return false;
28622865
Result = LHS << SA;
28632866
return true;
28642867
}
@@ -2882,6 +2885,10 @@ static bool handleIntIntBinOp(EvalInfo &Info, const BinaryOperator *E,
28822885
if (SA != RHS)
28832886
Info.CCEDiag(E, diag::note_constexpr_large_shift)
28842887
<< RHS << E->getType() << LHS.getBitWidth();
2888+
2889+
if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
2890+
Info.getLangOpts().CPlusPlus11)
2891+
return false;
28852892
Result = LHS >> SA;
28862893
return true;
28872894
}

‎clang/lib/Sema/SemaExpr.cpp

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11078,7 +11078,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
1107811078
if (Right.isNegative()) {
1107911079
S.DiagRuntimeBehavior(Loc, RHS.get(),
1108011080
S.PDiag(diag::warn_shift_negative)
11081-
<< RHS.get()->getSourceRange());
11081+
<< RHS.get()->getSourceRange());
1108211082
return;
1108311083
}
1108411084

@@ -11093,7 +11093,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
1109311093
if (Right.uge(LeftSize)) {
1109411094
S.DiagRuntimeBehavior(Loc, RHS.get(),
1109511095
S.PDiag(diag::warn_shift_gt_typewidth)
11096-
<< RHS.get()->getSourceRange());
11096+
<< RHS.get()->getSourceRange());
1109711097
return;
1109811098
}
1109911099

@@ -11126,7 +11126,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
1112611126
if (Left.isNegative()) {
1112711127
S.DiagRuntimeBehavior(Loc, LHS.get(),
1112811128
S.PDiag(diag::warn_shift_lhs_negative)
11129-
<< LHS.get()->getSourceRange());
11129+
<< LHS.get()->getSourceRange());
1113011130
return;
1113111131
}
1113211132

@@ -16964,11 +16964,38 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
1696416964
// Circumvent ICE checking in C++11 to avoid evaluating the expression twice
1696516965
// in the non-ICE case.
1696616966
if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
16967+
SmallVector<PartialDiagnosticAt, 8> Notes;
1696716968
if (Result)
16968-
*Result = E->EvaluateKnownConstIntCheckOverflow(Context);
16969+
*Result = E->EvaluateKnownConstIntCheckOverflow(Context, &Notes);
1696916970
if (!isa<ConstantExpr>(E))
1697016971
E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
1697116972
: ConstantExpr::Create(Context, E);
16973+
16974+
if (Notes.empty())
16975+
return E;
16976+
16977+
// If our only note is the usual "invalid subexpression" note, just point
16978+
// the caret at its location rather than producing an essentially
16979+
// redundant note.
16980+
if (Notes.size() == 1 && Notes[0].second.getDiagID() ==
16981+
diag::note_invalid_subexpr_in_const_expr) {
16982+
DiagLoc = Notes[0].first;
16983+
Notes.clear();
16984+
}
16985+
16986+
if (getLangOpts().CPlusPlus) {
16987+
if (!Diagnoser.Suppress) {
16988+
Diagnoser.diagnoseNotICE(*this, DiagLoc) << E->getSourceRange();
16989+
for (const PartialDiagnosticAt &Note : Notes)
16990+
Diag(Note.first, Note.second);
16991+
}
16992+
return ExprError();
16993+
}
16994+
16995+
Diagnoser.diagnoseFold(*this, DiagLoc) << E->getSourceRange();
16996+
for (const PartialDiagnosticAt &Note : Notes)
16997+
Diag(Note.first, Note.second);
16998+
1697216999
return E;
1697317000
}
1697417001

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,8 @@ void dr081(void) {
430430
/* Demonstrate that we don't crash when left shifting a signed value; that's
431431
* implementation defined behavior.
432432
*/
433-
_Static_assert(-1 << 1 == -2, "fail"); /* Didn't shift a zero into the "sign bit". */
433+
_Static_assert(-1 << 1 == -2, "fail"); /* expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
434+
expected-note {{left shift of negative value -1}} */
434435
_Static_assert(1 << 3 == 1u << 3u, "fail"); /* Shift of a positive signed value does sensible things. */
435436
}
436437

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,12 @@ void dr258(void) {
275275
* Constant expressions
276276
*/
277277
void dr261(void) {
278-
/* This is still an integer constant expression despite the overflow. */
278+
/* This is not an integer constant expression because of the overflow,
279+
* but we fold it as a constant expression anyway as a GNU extension. */
279280
enum e1 {
280-
ex1 = __INT_MAX__ + 1 /* expected-warning {{overflow in expression; result is -2'147'483'648 with type 'int'}} */
281+
ex1 = __INT_MAX__ + 1 /* expected-warning {{overflow in expression; result is -2'147'483'648 with type 'int'}}
282+
expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
283+
expected-note {{value 2147483648 is outside the range of representable values of type 'int'}} */
281284
};
282285

283286
/* This is not an integer constant expression, because of the comma operator,

‎clang/test/Sema/builtins.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,8 @@ void test17(void) {
171171
#define OPT(...) (__builtin_constant_p(__VA_ARGS__) && strlen(__VA_ARGS__) < 4)
172172
// FIXME: These are incorrectly treated as ICEs because strlen is treated as
173173
// a builtin.
174-
ASSERT(OPT("abc"));
175-
ASSERT(!OPT("abcd"));
174+
ASSERT(OPT("abc")); // expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
175+
ASSERT(!OPT("abcd")); // expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
176176
// In these cases, the strlen is non-constant, but the __builtin_constant_p
177177
// is 0: the array size is not an ICE but is foldable.
178178
ASSERT(!OPT(test17_c));

‎clang/test/Sema/constant-builtins-2.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -265,17 +265,21 @@ char clz52[__builtin_clzg((unsigned __int128)0x1) == BITSIZE(__int128) - 1 ? 1 :
265265
char clz53[__builtin_clzg((unsigned __int128)0x1, 42) == BITSIZE(__int128) - 1 ? 1 : -1];
266266
char clz54[__builtin_clzg((unsigned __int128)0xf) == BITSIZE(__int128) - 4 ? 1 : -1];
267267
char clz55[__builtin_clzg((unsigned __int128)0xf, 42) == BITSIZE(__int128) - 4 ? 1 : -1];
268-
char clz56[__builtin_clzg((unsigned __int128)(1 << (BITSIZE(__int128) - 1))) == 0 ? 1 : -1];
269-
char clz57[__builtin_clzg((unsigned __int128)(1 << (BITSIZE(__int128) - 1)), 42) == 0 ? 1 : -1];
268+
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}}
269+
// expected-note@-1 {{shift count 127 >= width of type 'int' (32 bits)}}
270+
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}}
271+
// expected-note@-1 {{shift count 127 >= width of type 'int' (32 bits)}}
270272
#endif
271273
int clz58 = __builtin_clzg((unsigned _BitInt(128))0); // expected-error {{not a compile-time constant}}
272274
char clz59[__builtin_clzg((unsigned _BitInt(128))0, 42) == 42 ? 1 : -1];
273275
char clz60[__builtin_clzg((unsigned _BitInt(128))0x1) == BITSIZE(_BitInt(128)) - 1 ? 1 : -1];
274276
char clz61[__builtin_clzg((unsigned _BitInt(128))0x1, 42) == BITSIZE(_BitInt(128)) - 1 ? 1 : -1];
275277
char clz62[__builtin_clzg((unsigned _BitInt(128))0xf) == BITSIZE(_BitInt(128)) - 4 ? 1 : -1];
276278
char clz63[__builtin_clzg((unsigned _BitInt(128))0xf, 42) == BITSIZE(_BitInt(128)) - 4 ? 1 : -1];
277-
char clz64[__builtin_clzg((unsigned _BitInt(128))(1 << (BITSIZE(_BitInt(128)) - 1))) == 0 ? 1 : -1];
278-
char clz65[__builtin_clzg((unsigned _BitInt(128))(1 << (BITSIZE(_BitInt(128)) - 1)), 42) == 0 ? 1 : -1];
279+
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}}
280+
// expected-note@-1 {{shift count 127 >= width of type 'int' (32 bits)}}
281+
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}}
282+
// expected-note@-1 {{shift count 127 >= width of type 'int' (32 bits)}}
279283

280284
char ctz1[__builtin_ctz(1) == 0 ? 1 : -1];
281285
char ctz2[__builtin_ctz(8) == 3 ? 1 : -1];

‎clang/test/Sema/integer-overflow.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,8 @@ void check_integer_overflows_in_function_calls(void) {
174174
}
175175
void check_integer_overflows_in_array_size(void) {
176176
int arr[4608 * 1024 * 1024]; // expected-warning {{overflow in expression; result is 536'870'912 with type 'int'}}
177+
// expected-warning@-1 {{variable length array folded to constant array as an extension}}
178+
// expected-note@-2 {{value 4831838208 is outside the range of representable values of type 'int'}}
177179
}
178180

179181
struct s {

‎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=expected,c -pedantic %s
2+
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=expected,cpp %s
3+
4+
enum shiftof {
5+
X = (1<<-29) // c-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
6+
// cpp-error@-1 {{expression is not an integral constant expression}}
7+
// expected-note@-2 {{negative shift count -29}}
8+
};

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// RUN: %clang_cc1 -fsyntax-only -verify=expected,c -pedantic %s
2+
// RUN: %clang_cc1 -x c++ -std=c++98 -fsyntax-only -verify=expected,cpp %s
3+
// RUN: %clang_cc1 -x c++ -std=c++11 -fsyntax-only -verify=expected,cpp %s
4+
5+
enum shiftof {
6+
X = (1<<32) // c-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
7+
// cpp-error@-1 {{expression is not an integral constant expression}}
8+
// expected-note@-2 {{shift count 32 >= width of type 'int'}}
9+
};

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// RUN: %clang_cc1 -x c -fsyntax-only -verify=expected,c -pedantic %s
2+
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=expected,cpp -Wshift-negative-value %s
3+
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=expected,cpp -Wall %s
4+
// RUN: %clang_cc1 -x c++ -std=c++98 -fsyntax-only -verify=expected,cpp -Wshift-negative-value %s
5+
// RUN: %clang_cc1 -x c++ -std=c++98 -fsyntax-only -verify=expected,cpp -Wall %s
6+
// RUN: %clang_cc1 -x c++ -std=c++11 -fsyntax-only -verify=expected,cpp -Wshift-negative-value %s
7+
// RUN: %clang_cc1 -x c++ -std=c++11 -fsyntax-only -verify=expected,cpp -Wall %s
8+
9+
enum shiftof {
10+
X = (-1<<29) // c-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
11+
// cpp-error@-1 {{expression is not an integral constant expression}}
12+
// expected-note@-2 {{left shift of negative value -1}}
13+
};

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@
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

‎clang/test/SemaCXX/enum.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,12 @@ void PR8089() {
100100
// expressions with UB to be non-constant.
101101
enum { overflow = 123456 * 234567 };
102102
#if __cplusplus >= 201103L
103-
// expected-warning@-2 {{not an integral constant expression}}
104-
// expected-note@-3 {{value 28958703552 is outside the range of representable values}}
105-
#else
106-
// expected-warning@-5 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
103+
// expected-warning@-2 {{expression is not an integral constant expression; folding it to a constant is a GNU extension}}
104+
// expected-note@-3 {{value 28958703552 is outside the range of representable values of type 'int'}}
105+
#else
106+
// expected-error@-5 {{expression is not an integral constant expression}}
107+
// expected-note@-6 {{value 28958703552 is outside the range of representable values of type 'int'}}
108+
// expected-warning@-7 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
107109
#endif
108110

109111
// FIXME: This is not consistent with the above case.
@@ -112,8 +114,10 @@ enum NoFold : int { overflow2 = 123456 * 234567 };
112114
// expected-error@-2 {{enumerator value is not a constant expression}}
113115
// expected-note@-3 {{value 28958703552 is outside the range of representable values}}
114116
#else
115-
// expected-warning@-5 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
116-
// expected-warning@-6 {{extension}}
117+
// expected-warning@-5 {{enumeration types with a fixed underlying type are a C++11 extension}}
118+
// expected-warning@-6 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
119+
// expected-error@-7 {{expression is not an integral constant expression}}
120+
// expected-note@-8 {{value 28958703552 is outside the range of representable values of type 'int'}}
117121
#endif
118122

119123
// PR28903

‎clang/test/SemaCXX/shift.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ 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}}
25+
// expected-warning@-1 {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}}
2626
c = 1 >> (unsigned)-1; // expected-warning {{shift count >= width of type}}
2727
c = 1 << c;
2828
c <<= 0;

0 commit comments

Comments
 (0)