https://github.com/budimirarandjelovicsyrmia updated https://github.com/llvm/llvm-project/pull/70307
From a66ca21ad79df2385fbaec12344f9c16cc3c5b83 Mon Sep 17 00:00:00 2001 From: budimirarandjelovicsyrmia <budimir.arandjelo...@syrmia.com> Date: Thu, 26 Oct 2023 10:39:52 +0200 Subject: [PATCH] [clang] Emit bad shift warnings --- clang/lib/AST/ExprConstant.cpp | 7 +++++++ clang/lib/Sema/SemaExpr.cpp | 17 +++++++++++++---- clang/test/C/drs/dr0xx.c | 3 ++- clang/test/C/drs/dr2xx.c | 4 +++- clang/test/Sema/builtins.c | 6 ++++-- clang/test/Sema/code_align.c | 7 ++++--- clang/test/Sema/constant-builtins-2.c | 8 ++++---- clang/test/Sema/shift-count-negative.c | 10 ++++++++++ clang/test/Sema/shift-count-overflow.c | 7 +++++++ clang/test/Sema/shift-negative-value.c | 10 ++++++++++ clang/test/Sema/vla-2.c | 6 ++++-- clang/test/SemaCXX/enum.cpp | 6 ++++-- clang/test/SemaCXX/shift.cpp | 2 +- 13 files changed, 73 insertions(+), 20 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 fe4b9a569ab87..80c4886e5d94a 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -2858,6 +2858,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().CPlusPlus) + return false; Result = LHS << SA; return true; } @@ -2881,6 +2884,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().CPlusPlus) + return false; Result = LHS >> SA; return true; } diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 4a2f3a65eac96..9f5ec9f432df9 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -11256,7 +11256,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; } @@ -11271,7 +11271,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; } @@ -11304,7 +11304,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; } @@ -17138,11 +17138,20 @@ 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.size() && !Diagnoser.Suppress) { + Diagnoser.diagnoseNotICE(*this, DiagLoc) << E->getSourceRange(); + for (const PartialDiagnosticAt &Note : Notes) + Diag(Note.first, Note.second); + return ExprError(); + } + return E; } diff --git a/clang/test/C/drs/dr0xx.c b/clang/test/C/drs/dr0xx.c index 36de32a93da95..530c216722cc3 100644 --- a/clang/test/C/drs/dr0xx.c +++ b/clang/test/C/drs/dr0xx.c @@ -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"); /* c89only-error {{static assertion expression is not an integral constant expression}} + c89only-note {{left shift of negative value -1}} */ _Static_assert(1 << 3 == 1u << 3u, "fail"); /* Shift of a positive signed value does sensible things. */ } diff --git a/clang/test/C/drs/dr2xx.c b/clang/test/C/drs/dr2xx.c index 1b68b65acca6a..689beb67d4ea6 100644 --- a/clang/test/C/drs/dr2xx.c +++ b/clang/test/C/drs/dr2xx.c @@ -277,7 +277,9 @@ void dr258(void) { void dr261(void) { /* This is still an integer constant expression despite the overflow. */ 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'}} + c89only-error {{expression is not an integer constant expression}} + c89only-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, diff --git a/clang/test/Sema/builtins.c b/clang/test/Sema/builtins.c index 4f843aeec24e6..42741b46dcb5c 100644 --- a/clang/test/Sema/builtins.c +++ b/clang/test/Sema/builtins.c @@ -171,8 +171,10 @@ 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-error {{expression is not an integer constant expression}} + // expected-note@-1 {{subexpression not valid in a constant expression}} + ASSERT(!OPT("abcd")); // expected-error {{expression is not an integer constant expression}} + // expected-note@-1 {{subexpression not valid in a constant expression}} // 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)); diff --git a/clang/test/Sema/code_align.c b/clang/test/Sema/code_align.c index f01f51382112f..ec29708ff5512 100644 --- a/clang/test/Sema/code_align.c +++ b/clang/test/Sema/code_align.c @@ -97,9 +97,10 @@ void foo1(int A) for(int I=0; I<256; ++I) { bar(I); } #ifdef __SIZEOF_INT128__ - // cpp-local-error@+3{{expression is not an integral constant expression}} - // cpp-local-note@+2{{left shift of negative value -1311768467294899680}} - // c-local-error@+1{{'code_align' attribute requires an integer argument which is a constant power of two between 1 and 4096 inclusive; provided argument was -(__int128_t)1311768467294899680ULL << 64}} + // cpp-local-error@+4{{expression is not an integral constant expression}} + // cpp-local-note@+3{{left shift of negative value -1311768467294899680}} + // c-local-error+2{{expression is not an integer constant expression}} + // c-local-note+1{{left shift of negative value -1311768467294899680}} [[clang::code_align(-(__int128_t)0x1234567890abcde0ULL << 64)]] for(int I=0; I<256; ++I) { bar(I); } #endif diff --git a/clang/test/Sema/constant-builtins-2.c b/clang/test/Sema/constant-builtins-2.c index a60a1f16a4587..d0c7e5d0af239 100644 --- a/clang/test/Sema/constant-builtins-2.c +++ b/clang/test/Sema/constant-builtins-2.c @@ -265,8 +265,8 @@ 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}} +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}} #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]; @@ -274,8 +274,8 @@ char clz60[__builtin_clzg((unsigned _BitInt(128))0x1) == BITSIZE(_BitInt(128)) - 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}} +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}} char ctz1[__builtin_ctz(1) == 0 ? 1 : -1]; char ctz2[__builtin_ctz(8) == 3 ? 1 : -1]; diff --git a/clang/test/Sema/shift-count-negative.c b/clang/test/Sema/shift-count-negative.c new file mode 100644 index 0000000000000..34d51294b43d4 --- /dev/null +++ b/clang/test/Sema/shift-count-negative.c @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -x c -fsyntax-only -verify=expected,c -Wshift-count-negative %s +// RUN: %clang_cc1 -x c -fsyntax-only -verify=expected,c -Wall %s +// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=expected,cpp -Wshift-count-negative %s +// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=expected,cpp -Wall %s + +enum shiftof { + X = (1<<-29) // c-error {{expression is not an integer constant expression}} + // cpp-error@-1 {{expression is not an integral constant expression}} + // expected-note@-2 {{negative shift count -29}} +}; diff --git a/clang/test/Sema/shift-count-overflow.c b/clang/test/Sema/shift-count-overflow.c new file mode 100644 index 0000000000000..77dd4b3c6a401 --- /dev/null +++ b/clang/test/Sema/shift-count-overflow.c @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wshift-count-overflow %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wall %s + +enum shiftof { + X = (1<<32) // expected-error {{expression is not an integer constant expression}} + // expected-note@-1 {{shift count 32 >= width of type 'int'}} +}; diff --git a/clang/test/Sema/shift-negative-value.c b/clang/test/Sema/shift-negative-value.c new file mode 100644 index 0000000000000..0394c00e00d0f --- /dev/null +++ b/clang/test/Sema/shift-negative-value.c @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -x c -fsyntax-only -verify=expected,c -Wshift-negative-value %s +// RUN: %clang_cc1 -x c -fsyntax-only -verify=expected,c -Wall %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 + +enum shiftof { + X = (-1<<29) // c-error {{expression is not an integer constant expression}} + // cpp-error@-1 {{expression is not an integral constant expression}} + // expected-note@-2 {{left shift of negative value -1}} +}; diff --git a/clang/test/Sema/vla-2.c b/clang/test/Sema/vla-2.c index 316931f270607..577407e15b479 100644 --- a/clang/test/Sema/vla-2.c +++ b/clang/test/Sema/vla-2.c @@ -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; } diff --git a/clang/test/SemaCXX/enum.cpp b/clang/test/SemaCXX/enum.cpp index c482b3c571ab4..7b3741d710a52 100644 --- a/clang/test/SemaCXX/enum.cpp +++ b/clang/test/SemaCXX/enum.cpp @@ -98,7 +98,8 @@ void PR8089() { // This is accepted as a GNU extension. In C++98, there was no provision for // expressions with UB to be non-constant. -enum { overflow = 123456 * 234567 }; +enum { overflow = 123456 * 234567 }; /* expected-error {{expression is not an integral constant expression}} + expected-note {{value 28958703552 is outside the range of representable values of type 'int}} */ #if __cplusplus >= 201103L // expected-warning@-2 {{not an integral constant expression}} // expected-note@-3 {{value 28958703552 is outside the range of representable values}} @@ -107,7 +108,8 @@ enum { overflow = 123456 * 234567 }; #endif // FIXME: This is not consistent with the above case. -enum NoFold : int { overflow2 = 123456 * 234567 }; +enum NoFold : int { overflow2 = 123456 * 234567 }; /* expected-error {{expression is not an integral constant expression}} + expected-note {{value 28958703552 is outside the range of representable values of type 'int}} */ #if __cplusplus >= 201103L // expected-error@-2 {{enumerator value is not a constant expression}} // expected-note@-3 {{value 28958703552 is outside the range of representable values}} diff --git a/clang/test/SemaCXX/shift.cpp b/clang/test/SemaCXX/shift.cpp index 89a98791d3eba..b6f795725c0d7 100644 --- a/clang/test/SemaCXX/shift.cpp +++ b/clang/test/SemaCXX/shift.cpp @@ -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}} + // cxx17-warning {{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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits