Skip to content

[clang] Emit bad shift warnings #70307

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,8 @@ Improvements to Clang's diagnostics

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

- Clang now diagnoses integer constant expressions that are folded to a constant value as an extension in more circumstances. Fixes #GH59863

Improvements to Clang's time-trace
----------------------------------

Expand Down
7 changes: 7 additions & 0 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2859,6 +2859,9 @@ static bool handleIntIntBinOp(EvalInfo &Info, const BinaryOperator *E,
else if (LHS.countl_zero() < SA)
Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
}
if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
Info.getLangOpts().CPlusPlus11)
return false;
Result = LHS << SA;
return true;
}
Expand All @@ -2882,6 +2885,10 @@ static bool handleIntIntBinOp(EvalInfo &Info, const BinaryOperator *E,
if (SA != RHS)
Info.CCEDiag(E, diag::note_constexpr_large_shift)
<< RHS << E->getType() << LHS.getBitWidth();

if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
Info.getLangOpts().CPlusPlus11)
return false;
Result = LHS >> SA;
return true;
}
Expand Down
35 changes: 31 additions & 4 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11078,7 +11078,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
if (Right.isNegative()) {
S.DiagRuntimeBehavior(Loc, RHS.get(),
S.PDiag(diag::warn_shift_negative)
<< RHS.get()->getSourceRange());
<< RHS.get()->getSourceRange());
return;
}

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

Expand Down Expand Up @@ -11126,7 +11126,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
if (Left.isNegative()) {
S.DiagRuntimeBehavior(Loc, LHS.get(),
S.PDiag(diag::warn_shift_lhs_negative)
<< LHS.get()->getSourceRange());
<< LHS.get()->getSourceRange());
return;
}

Expand Down Expand Up @@ -16964,11 +16964,38 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
// Circumvent ICE checking in C++11 to avoid evaluating the expression twice
// in the non-ICE case.
if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
SmallVector<PartialDiagnosticAt, 8> Notes;
if (Result)
*Result = E->EvaluateKnownConstIntCheckOverflow(Context);
*Result = E->EvaluateKnownConstIntCheckOverflow(Context, &Notes);
if (!isa<ConstantExpr>(E))
E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
: ConstantExpr::Create(Context, E);

if (Notes.empty())
return E;

// If our only note is the usual "invalid subexpression" note, just point
// the caret at its location rather than producing an essentially
// redundant note.
if (Notes.size() == 1 && Notes[0].second.getDiagID() ==
diag::note_invalid_subexpr_in_const_expr) {
DiagLoc = Notes[0].first;
Notes.clear();
}

if (getLangOpts().CPlusPlus) {
if (!Diagnoser.Suppress) {
Diagnoser.diagnoseNotICE(*this, DiagLoc) << E->getSourceRange();
for (const PartialDiagnosticAt &Note : Notes)
Diag(Note.first, Note.second);
}
return ExprError();
}

Diagnoser.diagnoseFold(*this, DiagLoc) << E->getSourceRange();
for (const PartialDiagnosticAt &Note : Notes)
Diag(Note.first, Note.second);

return E;
}

Expand Down
3 changes: 2 additions & 1 deletion clang/test/C/drs/dr0xx.c
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,8 @@ 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 {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
expected-note {{left shift of negative value -1}} */
_Static_assert(1 << 3 == 1u << 3u, "fail"); /* Shift of a positive signed value does sensible things. */
}

Expand Down
7 changes: 5 additions & 2 deletions clang/test/C/drs/dr2xx.c
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,12 @@ void dr258(void) {
* Constant expressions
*/
void dr261(void) {
/* This is still an integer constant expression despite the overflow. */
/* This is not an integer constant expression because of the overflow,
* but we fold it as a constant expression anyway as a GNU extension. */
enum e1 {
ex1 = __INT_MAX__ + 1 /* expected-warning {{overflow in expression; result is -2'147'483'648 with type 'int'}} */
ex1 = __INT_MAX__ + 1 /* expected-warning {{overflow in expression; result is -2'147'483'648 with type 'int'}}
expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
expected-note {{value 2147483648 is outside the range of representable values of type 'int'}} */
};

/* This is not an integer constant expression, because of the comma operator,
Expand Down
4 changes: 2 additions & 2 deletions clang/test/Sema/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ void test17(void) {
#define OPT(...) (__builtin_constant_p(__VA_ARGS__) && strlen(__VA_ARGS__) < 4)
// FIXME: These are incorrectly treated as ICEs because strlen is treated as
// a builtin.
ASSERT(OPT("abc"));
ASSERT(!OPT("abcd"));
ASSERT(OPT("abc")); // expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
ASSERT(!OPT("abcd")); // expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
// In these cases, the strlen is non-constant, but the __builtin_constant_p
// is 0: the array size is not an ICE but is foldable.
ASSERT(!OPT(test17_c));
Expand Down
12 changes: 8 additions & 4 deletions clang/test/Sema/constant-builtins-2.c
Original file line number Diff line number Diff line change
Expand Up @@ -265,17 +265,21 @@ char clz52[__builtin_clzg((unsigned __int128)0x1) == BITSIZE(__int128) - 1 ? 1 :
char clz53[__builtin_clzg((unsigned __int128)0x1, 42) == BITSIZE(__int128) - 1 ? 1 : -1];
char clz54[__builtin_clzg((unsigned __int128)0xf) == BITSIZE(__int128) - 4 ? 1 : -1];
char clz55[__builtin_clzg((unsigned __int128)0xf, 42) == BITSIZE(__int128) - 4 ? 1 : -1];
char clz56[__builtin_clzg((unsigned __int128)(1 << (BITSIZE(__int128) - 1))) == 0 ? 1 : -1];
char clz57[__builtin_clzg((unsigned __int128)(1 << (BITSIZE(__int128) - 1)), 42) == 0 ? 1 : -1];
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}}
// expected-note@-1 {{shift count 127 >= width of type 'int' (32 bits)}}
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}}
// expected-note@-1 {{shift count 127 >= width of type 'int' (32 bits)}}
#endif
int clz58 = __builtin_clzg((unsigned _BitInt(128))0); // expected-error {{not a compile-time constant}}
char clz59[__builtin_clzg((unsigned _BitInt(128))0, 42) == 42 ? 1 : -1];
char clz60[__builtin_clzg((unsigned _BitInt(128))0x1) == BITSIZE(_BitInt(128)) - 1 ? 1 : -1];
char clz61[__builtin_clzg((unsigned _BitInt(128))0x1, 42) == BITSIZE(_BitInt(128)) - 1 ? 1 : -1];
char clz62[__builtin_clzg((unsigned _BitInt(128))0xf) == BITSIZE(_BitInt(128)) - 4 ? 1 : -1];
char clz63[__builtin_clzg((unsigned _BitInt(128))0xf, 42) == BITSIZE(_BitInt(128)) - 4 ? 1 : -1];
char clz64[__builtin_clzg((unsigned _BitInt(128))(1 << (BITSIZE(_BitInt(128)) - 1))) == 0 ? 1 : -1];
char clz65[__builtin_clzg((unsigned _BitInt(128))(1 << (BITSIZE(_BitInt(128)) - 1)), 42) == 0 ? 1 : -1];
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}}
// expected-note@-1 {{shift count 127 >= width of type 'int' (32 bits)}}
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}}
// expected-note@-1 {{shift count 127 >= width of type 'int' (32 bits)}}

char ctz1[__builtin_ctz(1) == 0 ? 1 : -1];
char ctz2[__builtin_ctz(8) == 3 ? 1 : -1];
Expand Down
2 changes: 2 additions & 0 deletions clang/test/Sema/integer-overflow.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ void check_integer_overflows_in_function_calls(void) {
}
void check_integer_overflows_in_array_size(void) {
int arr[4608 * 1024 * 1024]; // expected-warning {{overflow in expression; result is 536'870'912 with type 'int'}}
// expected-warning@-1 {{variable length array folded to constant array as an extension}}
// expected-note@-2 {{value 4831838208 is outside the range of representable values of type 'int'}}
}

struct s {
Expand Down
8 changes: 8 additions & 0 deletions clang/test/Sema/shift-count-negative.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// RUN: %clang_cc1 -x c -fsyntax-only -verify=expected,c -pedantic %s
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=expected,cpp %s

enum shiftof {
X = (1<<-29) // c-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
// cpp-error@-1 {{expression is not an integral constant expression}}
// expected-note@-2 {{negative shift count -29}}
};
9 changes: 9 additions & 0 deletions clang/test/Sema/shift-count-overflow.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// RUN: %clang_cc1 -fsyntax-only -verify=expected,c -pedantic %s
// RUN: %clang_cc1 -x c++ -std=c++98 -fsyntax-only -verify=expected,cpp %s
// RUN: %clang_cc1 -x c++ -std=c++11 -fsyntax-only -verify=expected,cpp %s

enum shiftof {
X = (1<<32) // c-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
// cpp-error@-1 {{expression is not an integral constant expression}}
// expected-note@-2 {{shift count 32 >= width of type 'int'}}
};
13 changes: 13 additions & 0 deletions clang/test/Sema/shift-negative-value.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// RUN: %clang_cc1 -x c -fsyntax-only -verify=expected,c -pedantic %s
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=expected,cpp -Wshift-negative-value %s
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=expected,cpp -Wall %s
// RUN: %clang_cc1 -x c++ -std=c++98 -fsyntax-only -verify=expected,cpp -Wshift-negative-value %s
// RUN: %clang_cc1 -x c++ -std=c++98 -fsyntax-only -verify=expected,cpp -Wall %s
// RUN: %clang_cc1 -x c++ -std=c++11 -fsyntax-only -verify=expected,cpp -Wshift-negative-value %s
// RUN: %clang_cc1 -x c++ -std=c++11 -fsyntax-only -verify=expected,cpp -Wall %s

enum shiftof {
X = (-1<<29) // c-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
// cpp-error@-1 {{expression is not an integral constant expression}}
// expected-note@-2 {{left shift of negative value -1}}
};
6 changes: 4 additions & 2 deletions clang/test/Sema/vla-2.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@
// 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;
}

Expand Down
16 changes: 10 additions & 6 deletions clang/test/SemaCXX/enum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,12 @@ void PR8089() {
// expressions with UB to be non-constant.
enum { overflow = 123456 * 234567 };
#if __cplusplus >= 201103L
// expected-warning@-2 {{not an integral constant expression}}
// expected-note@-3 {{value 28958703552 is outside the range of representable values}}
#else
// expected-warning@-5 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
// expected-warning@-2 {{expression is not an integral constant expression; folding it to a constant is a GNU extension}}
// expected-note@-3 {{value 28958703552 is outside the range of representable values of type 'int'}}
#else
// expected-error@-5 {{expression is not an integral constant expression}}
// expected-note@-6 {{value 28958703552 is outside the range of representable values of type 'int'}}
// expected-warning@-7 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
#endif

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

// PR28903
Expand Down
2 changes: 1 addition & 1 deletion clang/test/SemaCXX/shift.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ 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@-1 {{implicit conversion}}
// expected-warning@-1 {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}}
c = 1 >> (unsigned)-1; // expected-warning {{shift count >= width of type}}
c = 1 << c;
c <<= 0;
Expand Down
Loading