Skip to content

Commit 741124c

Browse files
efriedma-quicllvmbot
efriedma-quic
authored and
llvmbot
committed
[ExprConstant] Handle shift overflow the same way as other kinds of overflow (#99579)
We have a mechanism to allow folding expressions that aren't ICEs as an extension; use it more consistently. This ends up causing bad effects on diagnostics in a few cases, but that's not specific to shifts; it's a general issue with the way those uses handle overflow diagnostics. (cherry picked from commit 20eff68)
1 parent 7af27be commit 741124c

File tree

8 files changed

+65
-35
lines changed

8 files changed

+65
-35
lines changed

‎clang/docs/ReleaseNotes.rst

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

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

768+
- Clang now diagnoses undefined behavior in constant expressions more consistently. This includes invalid shifts, and signed overflow in arithmetic.
769+
768770
Improvements to Clang's time-trace
769771
----------------------------------
770772

‎clang/lib/AST/ExprConstant.cpp

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2839,6 +2839,8 @@ static bool handleIntIntBinOp(EvalInfo &Info, const BinaryOperator *E,
28392839
// During constant-folding, a negative shift is an opposite shift. Such
28402840
// a shift is not a constant expression.
28412841
Info.CCEDiag(E, diag::note_constexpr_negative_shift) << RHS;
2842+
if (!Info.noteUndefinedBehavior())
2843+
return false;
28422844
RHS = -RHS;
28432845
goto shift_right;
28442846
}
@@ -2849,19 +2851,23 @@ static bool handleIntIntBinOp(EvalInfo &Info, const BinaryOperator *E,
28492851
if (SA != RHS) {
28502852
Info.CCEDiag(E, diag::note_constexpr_large_shift)
28512853
<< RHS << E->getType() << LHS.getBitWidth();
2854+
if (!Info.noteUndefinedBehavior())
2855+
return false;
28522856
} else if (LHS.isSigned() && !Info.getLangOpts().CPlusPlus20) {
28532857
// C++11 [expr.shift]p2: A signed left shift must have a non-negative
28542858
// operand, and must not overflow the corresponding unsigned type.
28552859
// C++2a [expr.shift]p2: E1 << E2 is the unique value congruent to
28562860
// E1 x 2^E2 module 2^N.
2857-
if (LHS.isNegative())
2861+
if (LHS.isNegative()) {
28582862
Info.CCEDiag(E, diag::note_constexpr_lshift_of_negative) << LHS;
2859-
else if (LHS.countl_zero() < SA)
2863+
if (!Info.noteUndefinedBehavior())
2864+
return false;
2865+
} else if (LHS.countl_zero() < SA) {
28602866
Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
2867+
if (!Info.noteUndefinedBehavior())
2868+
return false;
2869+
}
28612870
}
2862-
if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
2863-
Info.getLangOpts().CPlusPlus11)
2864-
return false;
28652871
Result = LHS << SA;
28662872
return true;
28672873
}
@@ -2875,20 +2881,22 @@ static bool handleIntIntBinOp(EvalInfo &Info, const BinaryOperator *E,
28752881
// During constant-folding, a negative shift is an opposite shift. Such a
28762882
// shift is not a constant expression.
28772883
Info.CCEDiag(E, diag::note_constexpr_negative_shift) << RHS;
2884+
if (!Info.noteUndefinedBehavior())
2885+
return false;
28782886
RHS = -RHS;
28792887
goto shift_left;
28802888
}
28812889
shift_right:
28822890
// C++11 [expr.shift]p1: Shift width must be less than the bit width of the
28832891
// shifted type.
28842892
unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
2885-
if (SA != RHS)
2893+
if (SA != RHS) {
28862894
Info.CCEDiag(E, diag::note_constexpr_large_shift)
28872895
<< RHS << E->getType() << LHS.getBitWidth();
2896+
if (!Info.noteUndefinedBehavior())
2897+
return false;
2898+
}
28882899

2889-
if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
2890-
Info.getLangOpts().CPlusPlus11)
2891-
return false;
28922900
Result = LHS >> SA;
28932901
return true;
28942902
}

‎clang/lib/AST/Interp/Interp.h

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,8 @@ bool CheckShift(InterpState &S, CodePtr OpPC, const LT &LHS, const RT &RHS,
153153
if (RHS.isNegative()) {
154154
const SourceInfo &Loc = S.Current->getSource(OpPC);
155155
S.CCEDiag(Loc, diag::note_constexpr_negative_shift) << RHS.toAPSInt();
156-
return false;
156+
if (!S.noteUndefinedBehavior())
157+
return false;
157158
}
158159

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

169171
if (LHS.isSigned() && !S.getLangOpts().CPlusPlus20) {
170172
const Expr *E = S.Current->getExpr(OpPC);
171173
// C++11 [expr.shift]p2: A signed left shift must have a non-negative
172174
// operand, and must not overflow the corresponding unsigned type.
173-
if (LHS.isNegative())
175+
if (LHS.isNegative()) {
174176
S.CCEDiag(E, diag::note_constexpr_lshift_of_negative) << LHS.toAPSInt();
175-
else if (LHS.toUnsigned().countLeadingZeros() < static_cast<unsigned>(RHS))
177+
if (!S.noteUndefinedBehavior())
178+
return false;
179+
} else if (LHS.toUnsigned().countLeadingZeros() <
180+
static_cast<unsigned>(RHS)) {
176181
S.CCEDiag(E, diag::note_constexpr_lshift_discards);
182+
if (!S.noteUndefinedBehavior())
183+
return false;
184+
}
177185
}
178186

179187
// C++2a [expr.shift]p2: [P0907R4]:
@@ -2269,8 +2277,7 @@ inline bool DoShift(InterpState &S, CodePtr OpPC, LT &LHS, RT &RHS) {
22692277
// shift is not a constant expression.
22702278
const SourceInfo &Loc = S.Current->getSource(OpPC);
22712279
S.CCEDiag(Loc, diag::note_constexpr_negative_shift) << RHS.toAPSInt();
2272-
if (S.getLangOpts().CPlusPlus11 && S.getEvalStatus().Diag &&
2273-
!S.getEvalStatus().Diag->empty())
2280+
if (!S.noteUndefinedBehavior())
22742281
return false;
22752282
RHS = -RHS;
22762283
return DoShift < LT, RT,
@@ -2286,8 +2293,7 @@ inline bool DoShift(InterpState &S, CodePtr OpPC, LT &LHS, RT &RHS) {
22862293
// E1 x 2^E2 module 2^N.
22872294
const SourceInfo &Loc = S.Current->getSource(OpPC);
22882295
S.CCEDiag(Loc, diag::note_constexpr_lshift_of_negative) << LHS.toAPSInt();
2289-
if (S.getLangOpts().CPlusPlus11 && S.getEvalStatus().Diag &&
2290-
!S.getEvalStatus().Diag->empty())
2296+
if (!S.noteUndefinedBehavior())
22912297
return false;
22922298
}
22932299
}

‎clang/lib/Sema/SemaExpr.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17046,7 +17046,8 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
1704617046
// not a constant expression as a side-effect.
1704717047
bool Folded =
1704817048
E->EvaluateAsRValue(EvalResult, Context, /*isConstantContext*/ true) &&
17049-
EvalResult.Val.isInt() && !EvalResult.HasSideEffects;
17049+
EvalResult.Val.isInt() && !EvalResult.HasSideEffects &&
17050+
(!getLangOpts().CPlusPlus || !EvalResult.HasUndefinedBehavior);
1705017051

1705117052
if (!isa<ConstantExpr>(E))
1705217053
E = ConstantExpr::Create(Context, E, EvalResult.Val);

‎clang/test/CXX/basic/basic.types/p10.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ constexpr int arb(int n) { // expected-note {{declared here}}
142142
expected-note {{function parameter 'n' with unknown value cannot be used in a constant expression}}
143143
}
144144
constexpr long Overflow[(1 << 30) << 2]{}; // expected-warning {{requires 34 bits to represent}} \
145-
expected-warning {{variable length array folded to constant array as an extension}} \
145+
expected-error {{variable length array declaration not allowed at file scope}} \
146146
expected-warning {{variable length arrays in C++ are a Clang extension}} \
147147
expected-note {{signed left shift discards bits}}
148148

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -265,21 +265,17 @@ 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]; // 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)}}
268+
char clz56[__builtin_clzg((unsigned __int128)(1 << (BITSIZE(__int128) - 1))) == 0 ? 1 : -1]; // expected-error {{variable length array declaration not allowed at file scope}}
269+
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}}
272270
#endif
273271
int clz58 = __builtin_clzg((unsigned _BitInt(128))0); // expected-error {{not a compile-time constant}}
274272
char clz59[__builtin_clzg((unsigned _BitInt(128))0, 42) == 42 ? 1 : -1];
275273
char clz60[__builtin_clzg((unsigned _BitInt(128))0x1) == BITSIZE(_BitInt(128)) - 1 ? 1 : -1];
276274
char clz61[__builtin_clzg((unsigned _BitInt(128))0x1, 42) == BITSIZE(_BitInt(128)) - 1 ? 1 : -1];
277275
char clz62[__builtin_clzg((unsigned _BitInt(128))0xf) == BITSIZE(_BitInt(128)) - 4 ? 1 : -1];
278276
char clz63[__builtin_clzg((unsigned _BitInt(128))0xf, 42) == BITSIZE(_BitInt(128)) - 4 ? 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)}}
277+
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}}
278+
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}}
283279

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

‎clang/test/SemaCXX/class.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %clang_cc1 -fsyntax-only -verify -Wc++11-compat %s
1+
// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx11 -Wc++11-compat %s
22
// RUN: %clang_cc1 -fsyntax-only -verify -Wc++11-compat %s -std=c++98
33
class C {
44
public:
@@ -55,6 +55,13 @@ class C {
5555
// expected-error@-2 {{static const volatile data member must be initialized out of line}}
5656
#endif
5757
static const E evi = 0;
58+
static const int overflow = 1000000*1000000; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
59+
// expected-warning@-1 {{overflow in expression}}
60+
static const int overflow_shift = 1<<32; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
61+
static const int overflow_shift2 = 1>>32; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
62+
static const int overflow_shift3 = 1<<-1; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
63+
static const int overflow_shift4 = 1<<-1; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
64+
static const int overflow_shift5 = -1<<1; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
5865

5966
void m() {
6067
sx = 0;

‎clang/test/SemaCXX/enum.cpp

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -103,14 +103,14 @@ void PR8089() {
103103
// This is accepted as a GNU extension. In C++98, there was no provision for
104104
// expressions with UB to be non-constant.
105105
enum { overflow = 123456 * 234567 };
106-
#if __cplusplus >= 201103L
107-
// expected-warning@-2 {{expression is not an integral constant expression; folding it to a constant is a GNU extension}}
108-
// expected-note@-3 {{value 28958703552 is outside the range of representable values of type 'int'}}
109-
#else
110-
// expected-error@-5 {{expression is not an integral constant expression}}
111-
// expected-note@-6 {{value 28958703552 is outside the range of representable values of type 'int'}}
112-
// expected-warning@-7 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
106+
// expected-error@-1 {{expression is not an integral constant expression}}
107+
// expected-note@-2 {{value 28958703552 is outside the range of representable values of type 'int'}}
108+
#if __cplusplus < 201103L
109+
// expected-warning@-4 {{overflow in expression; result is -1'106'067'520 with type 'int'}}
113110
#endif
111+
enum { overflow_shift = 1 << 32 };
112+
// expected-error@-1 {{expression is not an integral constant expression}}
113+
// expected-note@-2 {{shift count 32 >= width of type 'int' (32 bits)}}
114114

115115
// FIXME: This is not consistent with the above case.
116116
enum NoFold : int { overflow2 = 123456 * 234567 };
@@ -123,6 +123,16 @@ enum NoFold : int { overflow2 = 123456 * 234567 };
123123
// expected-error@-7 {{expression is not an integral constant expression}}
124124
// expected-note@-8 {{value 28958703552 is outside the range of representable values of type 'int'}}
125125
#endif
126+
enum : int { overflow2_shift = 1 << 32 };
127+
#if __cplusplus >= 201103L
128+
// expected-error@-2 {{enumerator value is not a constant expression}}
129+
// expected-note@-3 {{shift count 32 >= width of type 'int' (32 bits)}}
130+
#else
131+
// expected-error@-5 {{expression is not an integral constant expression}}
132+
// expected-note@-6 {{shift count 32 >= width of type 'int' (32 bits)}}
133+
// expected-warning@-7 {{enumeration types with a fixed underlying type are a C++11 extension}}
134+
#endif
135+
126136

127137
// PR28903
128138
struct PR28903 {

0 commit comments

Comments
 (0)