Skip to content

release/19.x: [ExprConstant] Handle shift overflow the same way as other kinds of overflow (#99579) #100452

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 26, 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 @@ -765,6 +765,8 @@ Improvements to Clang's diagnostics

UsingWithAttr<int> objUsingWA; // warning: 'UsingWithAttr' is deprecated

- Clang now diagnoses undefined behavior in constant expressions more consistently. This includes invalid shifts, and signed overflow in arithmetic.

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

Expand Down
26 changes: 17 additions & 9 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2839,6 +2839,8 @@ static bool handleIntIntBinOp(EvalInfo &Info, const BinaryOperator *E,
// During constant-folding, a negative shift is an opposite shift. Such
// a shift is not a constant expression.
Info.CCEDiag(E, diag::note_constexpr_negative_shift) << RHS;
if (!Info.noteUndefinedBehavior())
return false;
RHS = -RHS;
goto shift_right;
}
Expand All @@ -2849,19 +2851,23 @@ 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.noteUndefinedBehavior())
return false;
} else if (LHS.isSigned() && !Info.getLangOpts().CPlusPlus20) {
// C++11 [expr.shift]p2: A signed left shift must have a non-negative
// operand, and must not overflow the corresponding unsigned type.
// C++2a [expr.shift]p2: E1 << E2 is the unique value congruent to
// E1 x 2^E2 module 2^N.
if (LHS.isNegative())
if (LHS.isNegative()) {
Info.CCEDiag(E, diag::note_constexpr_lshift_of_negative) << LHS;
else if (LHS.countl_zero() < SA)
if (!Info.noteUndefinedBehavior())
return false;
} else if (LHS.countl_zero() < SA) {
Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
if (!Info.noteUndefinedBehavior())
return false;
}
}
if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
Info.getLangOpts().CPlusPlus11)
return false;
Result = LHS << SA;
return true;
}
Expand All @@ -2875,20 +2881,22 @@ static bool handleIntIntBinOp(EvalInfo &Info, const BinaryOperator *E,
// During constant-folding, a negative shift is an opposite shift. Such a
// shift is not a constant expression.
Info.CCEDiag(E, diag::note_constexpr_negative_shift) << RHS;
if (!Info.noteUndefinedBehavior())
return false;
RHS = -RHS;
goto shift_left;
}
shift_right:
// C++11 [expr.shift]p1: Shift width must be less than the bit width of the
// shifted type.
unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
if (SA != RHS)
if (SA != RHS) {
Info.CCEDiag(E, diag::note_constexpr_large_shift)
<< RHS << E->getType() << LHS.getBitWidth();
if (!Info.noteUndefinedBehavior())
return false;
}

if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
Info.getLangOpts().CPlusPlus11)
return false;
Result = LHS >> SA;
return true;
}
Expand Down
22 changes: 14 additions & 8 deletions clang/lib/AST/Interp/Interp.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ bool CheckShift(InterpState &S, CodePtr OpPC, const LT &LHS, const RT &RHS,
if (RHS.isNegative()) {
const SourceInfo &Loc = S.Current->getSource(OpPC);
S.CCEDiag(Loc, diag::note_constexpr_negative_shift) << RHS.toAPSInt();
return false;
if (!S.noteUndefinedBehavior())
return false;
}

// C++11 [expr.shift]p1: Shift width must be less than the bit width of
Expand All @@ -163,17 +164,24 @@ bool CheckShift(InterpState &S, CodePtr OpPC, const LT &LHS, const RT &RHS,
const APSInt Val = RHS.toAPSInt();
QualType Ty = E->getType();
S.CCEDiag(E, diag::note_constexpr_large_shift) << Val << Ty << Bits;
return !(S.getEvalStatus().Diag && !S.getEvalStatus().Diag->empty() && S.getLangOpts().CPlusPlus11);
if (!S.noteUndefinedBehavior())
return false;
}

if (LHS.isSigned() && !S.getLangOpts().CPlusPlus20) {
const Expr *E = S.Current->getExpr(OpPC);
// C++11 [expr.shift]p2: A signed left shift must have a non-negative
// operand, and must not overflow the corresponding unsigned type.
if (LHS.isNegative())
if (LHS.isNegative()) {
S.CCEDiag(E, diag::note_constexpr_lshift_of_negative) << LHS.toAPSInt();
else if (LHS.toUnsigned().countLeadingZeros() < static_cast<unsigned>(RHS))
if (!S.noteUndefinedBehavior())
return false;
} else if (LHS.toUnsigned().countLeadingZeros() <
static_cast<unsigned>(RHS)) {
S.CCEDiag(E, diag::note_constexpr_lshift_discards);
if (!S.noteUndefinedBehavior())
return false;
}
}

// C++2a [expr.shift]p2: [P0907R4]:
Expand Down Expand Up @@ -2269,8 +2277,7 @@ inline bool DoShift(InterpState &S, CodePtr OpPC, LT &LHS, RT &RHS) {
// shift is not a constant expression.
const SourceInfo &Loc = S.Current->getSource(OpPC);
S.CCEDiag(Loc, diag::note_constexpr_negative_shift) << RHS.toAPSInt();
if (S.getLangOpts().CPlusPlus11 && S.getEvalStatus().Diag &&
!S.getEvalStatus().Diag->empty())
if (!S.noteUndefinedBehavior())
return false;
RHS = -RHS;
return DoShift < LT, RT,
Expand All @@ -2286,8 +2293,7 @@ inline bool DoShift(InterpState &S, CodePtr OpPC, LT &LHS, RT &RHS) {
// E1 x 2^E2 module 2^N.
const SourceInfo &Loc = S.Current->getSource(OpPC);
S.CCEDiag(Loc, diag::note_constexpr_lshift_of_negative) << LHS.toAPSInt();
if (S.getLangOpts().CPlusPlus11 && S.getEvalStatus().Diag &&
!S.getEvalStatus().Diag->empty())
if (!S.noteUndefinedBehavior())
return false;
}
}
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17045,7 +17045,8 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
// not a constant expression as a side-effect.
bool Folded =
E->EvaluateAsRValue(EvalResult, Context, /*isConstantContext*/ true) &&
EvalResult.Val.isInt() && !EvalResult.HasSideEffects;
EvalResult.Val.isInt() && !EvalResult.HasSideEffects &&
(!getLangOpts().CPlusPlus || !EvalResult.HasUndefinedBehavior);

if (!isa<ConstantExpr>(E))
E = ConstantExpr::Create(Context, E, EvalResult.Val);
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CXX/basic/basic.types/p10.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ constexpr int arb(int n) { // expected-note {{declared here}}
expected-note {{function parameter 'n' with unknown value cannot be used in a constant expression}}
}
constexpr long Overflow[(1 << 30) << 2]{}; // expected-warning {{requires 34 bits to represent}} \
expected-warning {{variable length array folded to constant array as an extension}} \
expected-error {{variable length array declaration not allowed at file scope}} \
expected-warning {{variable length arrays in C++ are a Clang extension}} \
expected-note {{signed left shift discards bits}}

Expand Down
12 changes: 4 additions & 8 deletions clang/test/Sema/constant-builtins-2.c
Original file line number Diff line number Diff line change
Expand Up @@ -265,21 +265,17 @@ 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]; // 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)}}
char clz56[__builtin_clzg((unsigned __int128)(1 << (BITSIZE(__int128) - 1))) == 0 ? 1 : -1]; // expected-error {{variable length array declaration not allowed at file scope}}
char clz57[__builtin_clzg((unsigned __int128)(1 << (BITSIZE(__int128) - 1)), 42) == 0 ? 1 : -1]; // expected-error {{variable length array declaration not allowed at file scope}}
#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]; // 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 clz64[__builtin_clzg((unsigned _BitInt(128))(1 << (BITSIZE(_BitInt(128)) - 1))) == 0 ? 1 : -1]; // expected-error {{variable length array declaration not allowed at file scope}}
char clz65[__builtin_clzg((unsigned _BitInt(128))(1 << (BITSIZE(_BitInt(128)) - 1)), 42) == 0 ? 1 : -1]; // expected-error {{variable length array declaration not allowed at file scope}}

char ctz1[__builtin_ctz(1) == 0 ? 1 : -1];
char ctz2[__builtin_ctz(8) == 3 ? 1 : -1];
Expand Down
9 changes: 8 additions & 1 deletion clang/test/SemaCXX/class.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %clang_cc1 -fsyntax-only -verify -Wc++11-compat %s
// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx11 -Wc++11-compat %s
// RUN: %clang_cc1 -fsyntax-only -verify -Wc++11-compat %s -std=c++98
class C {
public:
Expand Down Expand Up @@ -55,6 +55,13 @@ class C {
// expected-error@-2 {{static const volatile data member must be initialized out of line}}
#endif
static const E evi = 0;
static const int overflow = 1000000*1000000; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
// expected-warning@-1 {{overflow in expression}}
static const int overflow_shift = 1<<32; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
static const int overflow_shift2 = 1>>32; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
static const int overflow_shift3 = 1<<-1; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
static const int overflow_shift4 = 1<<-1; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
static const int overflow_shift5 = -1<<1; // cxx11-error {{in-class initializer for static data member is not a constant expression}}

void m() {
sx = 0;
Expand Down
24 changes: 17 additions & 7 deletions clang/test/SemaCXX/enum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,14 @@ 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 };
#if __cplusplus >= 201103L
// 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'}}
// expected-error@-1 {{expression is not an integral constant expression}}
// expected-note@-2 {{value 28958703552 is outside the range of representable values of type 'int'}}
#if __cplusplus < 201103L
// expected-warning@-4 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
#endif
enum { overflow_shift = 1 << 32 };
// expected-error@-1 {{expression is not an integral constant expression}}
// expected-note@-2 {{shift count 32 >= width of type 'int' (32 bits)}}

// FIXME: This is not consistent with the above case.
enum NoFold : int { overflow2 = 123456 * 234567 };
Expand All @@ -123,6 +123,16 @@ enum NoFold : int { overflow2 = 123456 * 234567 };
// 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
enum : int { overflow2_shift = 1 << 32 };
#if __cplusplus >= 201103L
// expected-error@-2 {{enumerator value is not a constant expression}}
// expected-note@-3 {{shift count 32 >= width of type 'int' (32 bits)}}
#else
// expected-error@-5 {{expression is not an integral constant expression}}
// expected-note@-6 {{shift count 32 >= width of type 'int' (32 bits)}}
// expected-warning@-7 {{enumeration types with a fixed underlying type are a C++11 extension}}
#endif


// PR28903
struct PR28903 {
Expand Down
Loading