Skip to content

Commit 9e4309a

Browse files
[clang] Emit bad shift warnings
1 parent a46a2c2 commit 9e4309a

File tree

14 files changed

+104
-21
lines changed

14 files changed

+104
-21
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,8 @@ Improvements to Clang's diagnostics
626626
used rather than when they are needed for constant evaluation or when code is generated for them.
627627
The check is now stricter to prevent crashes for some unsupported declarations (Fixes #GH95495).
628628

629+
- Clang now diagnoses non-C++11 integer constant expressions. Fixes #GH59863
630+
629631
Improvements to Clang's time-trace
630632
----------------------------------
631633

clang/lib/AST/ExprConstant.cpp

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

clang/lib/Sema/SemaExpr.cpp

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11256,7 +11256,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
1125611256
if (Right.isNegative()) {
1125711257
S.DiagRuntimeBehavior(Loc, RHS.get(),
1125811258
S.PDiag(diag::warn_shift_negative)
11259-
<< RHS.get()->getSourceRange());
11259+
<< RHS.get()->getSourceRange());
1126011260
return;
1126111261
}
1126211262

@@ -11271,7 +11271,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
1127111271
if (Right.uge(LeftSize)) {
1127211272
S.DiagRuntimeBehavior(Loc, RHS.get(),
1127311273
S.PDiag(diag::warn_shift_gt_typewidth)
11274-
<< RHS.get()->getSourceRange());
11274+
<< RHS.get()->getSourceRange());
1127511275
return;
1127611276
}
1127711277

@@ -11304,7 +11304,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
1130411304
if (Left.isNegative()) {
1130511305
S.DiagRuntimeBehavior(Loc, LHS.get(),
1130611306
S.PDiag(diag::warn_shift_lhs_negative)
11307-
<< LHS.get()->getSourceRange());
11307+
<< LHS.get()->getSourceRange());
1130811308
return;
1130911309
}
1131011310

@@ -17138,11 +17138,38 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
1713817138
// Circumvent ICE checking in C++11 to avoid evaluating the expression twice
1713917139
// in the non-ICE case.
1714017140
if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
17141+
SmallVector<PartialDiagnosticAt, 8> Notes;
1714117142
if (Result)
17142-
*Result = E->EvaluateKnownConstIntCheckOverflow(Context);
17143+
*Result = E->EvaluateKnownConstIntCheckOverflow(Context, &Notes);
1714317144
if (!isa<ConstantExpr>(E))
1714417145
E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
1714517146
: ConstantExpr::Create(Context, E);
17147+
17148+
if (Notes.empty())
17149+
return E;
17150+
17151+
// If our only note is the usual "invalid subexpression" note, just point
17152+
// the caret at its location rather than producing an essentially
17153+
// redundant note.
17154+
if (Notes.size() == 1 && Notes[0].second.getDiagID() ==
17155+
diag::note_invalid_subexpr_in_const_expr) {
17156+
DiagLoc = Notes[0].first;
17157+
Notes.clear();
17158+
}
17159+
17160+
if (getLangOpts().CPlusPlus) {
17161+
if (!Diagnoser.Suppress) {
17162+
Diagnoser.diagnoseNotICE(*this, DiagLoc) << E->getSourceRange();
17163+
for (const PartialDiagnosticAt &Note : Notes)
17164+
Diag(Note.first, Note.second);
17165+
}
17166+
return ExprError();
17167+
}
17168+
17169+
Diagnoser.diagnoseFold(*this, DiagLoc) << E->getSourceRange();
17170+
for (const PartialDiagnosticAt &Note : Notes)
17171+
Diag(Note.first, Note.second);
17172+
1714617173
return E;
1714717174
}
1714817175

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: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,9 @@ void dr258(void) {
277277
void dr261(void) {
278278
/* This is still an integer constant expression despite the overflow. */
279279
enum e1 {
280-
ex1 = __INT_MAX__ + 1 /* expected-warning {{overflow in expression; result is -2'147'483'648 with type 'int'}} */
280+
ex1 = __INT_MAX__ + 1 /* expected-warning {{overflow in expression; result is -2'147'483'648 with type 'int'}}
281+
expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
282+
expected-note {{value 2147483648 is outside the range of representable values of type 'int'}} */
281283
};
282284

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

clang/test/Sema/builtins.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,10 @@ 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+
expected-note {{subexpression not valid in a constant expression}} */
176+
ASSERT(!OPT("abcd")); /* expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
177+
expected-note {{subexpression not valid in a constant expression}} */
176178
// In these cases, the strlen is non-constant, but the __builtin_constant_p
177179
// is 0: the array size is not an ICE but is foldable.
178180
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 {
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+
};
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+
};
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)