From 42d13bb475bd6123088361524db4211d0857341d Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Tue, 8 Dec 2015 03:21:47 +0000 Subject: [PATCH] Explicitly permit undefined behavior in constant initializers for global variables in C, in the cases where we can constant-fold it to a value regardless (such as floating-point division by zero and signed integer overflow). Strictly enforcing this rule breaks too much code. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@254992 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/Expr.h | 20 ++++++++++++--- lib/AST/Expr.cpp | 5 +++- lib/AST/ExprConstant.cpp | 44 +++++++++++++++++++++++++++----- test/CodeGen/complex-init-list.c | 4 +-- test/PCH/floating-literal.c | 4 +-- test/Sema/const-eval.c | 2 +- test/Sema/integer-overflow.c | 1 - test/Sema/switch-1.c | 3 --- test/SemaCXX/enum.cpp | 17 +++++++++++- 9 files changed, 79 insertions(+), 21 deletions(-) diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h index f6f6b1f580..284bc87b3f 100644 --- a/include/clang/AST/Expr.h +++ b/include/clang/AST/Expr.h @@ -529,10 +529,15 @@ public: /// EvalStatus is a struct with detailed info about an evaluation in progress. struct EvalStatus { - /// HasSideEffects - Whether the evaluated expression has side effects. + /// \brief Whether the evaluated expression has side effects. /// For example, (f() && 0) can be folded, but it still has side effects. bool HasSideEffects; + /// \brief Whether the evaluation hit undefined behavior. + /// For example, 1.0 / 0.0 can be folded to Inf, but has undefined behavior. + /// Likewise, INT_MAX + 1 can be folded to INT_MIN, but has UB. + bool HasUndefinedBehavior; + /// Diag - If this is non-null, it will be filled in with a stack of notes /// indicating why evaluation failed (or why it failed to produce a constant /// expression). @@ -542,7 +547,8 @@ public: /// expression *is* a constant expression, no notes will be produced. SmallVectorImpl *Diag; - EvalStatus() : HasSideEffects(false), Diag(nullptr) {} + EvalStatus() + : HasSideEffects(false), HasUndefinedBehavior(false), Diag(nullptr) {} // hasSideEffects - Return true if the evaluated expression has // side effects. @@ -575,7 +581,12 @@ public: /// side-effects. bool EvaluateAsBooleanCondition(bool &Result, const ASTContext &Ctx) const; - enum SideEffectsKind { SE_NoSideEffects, SE_AllowSideEffects }; + enum SideEffectsKind { + SE_NoSideEffects, ///< Strictly evaluate the expression. + SE_AllowUndefinedBehavior, ///< Allow UB that we can give a value, but not + ///< arbitrary unmodeled side effects. + SE_AllowSideEffects ///< Allow any unmodeled side effect. + }; /// EvaluateAsInt - Return true if this is a constant which we can fold and /// convert to an integer, using any crazy technique that we want to. @@ -584,7 +595,8 @@ public: /// isEvaluatable - Call EvaluateAsRValue to see if this expression can be /// constant folded without side-effects, but discard the result. - bool isEvaluatable(const ASTContext &Ctx) const; + bool isEvaluatable(const ASTContext &Ctx, + SideEffectsKind AllowSideEffects = SE_NoSideEffects) const; /// HasSideEffects - This routine returns true for all those expressions /// which have any effect other than producing a value. Example is a function diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index 33ea76ea74..9996546a17 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -2891,7 +2891,10 @@ bool Expr::isConstantInitializer(ASTContext &Ctx, bool IsForRef, return cast(this)->getExpr() ->isConstantInitializer(Ctx, false, Culprit); } - if (isEvaluatable(Ctx)) + // Allow certain forms of UB in constant initializers: signed integer + // overflow and floating-point division by zero. We'll give a warning on + // these, but they're common enough that we have to accept them. + if (isEvaluatable(Ctx, SE_AllowUndefinedBehavior)) return true; if (Culprit) *Culprit = this; diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp index ffaf742030..3c8868c2fc 100644 --- a/lib/AST/ExprConstant.cpp +++ b/lib/AST/ExprConstant.cpp @@ -715,6 +715,32 @@ namespace { return keepEvaluatingAfterSideEffect(); } + /// Should we continue evaluation after encountering undefined behavior? + bool keepEvaluatingAfterUndefinedBehavior() { + switch (EvalMode) { + case EM_EvaluateForOverflow: + case EM_IgnoreSideEffects: + case EM_ConstantFold: + case EM_DesignatorFold: + return true; + + case EM_PotentialConstantExpression: + case EM_PotentialConstantExpressionUnevaluated: + case EM_ConstantExpression: + case EM_ConstantExpressionUnevaluated: + return false; + } + llvm_unreachable("Missed EvalMode case"); + } + + /// Note that we hit something that was technically undefined behavior, but + /// that we can evaluate past it (such as signed overflow or floating-point + /// division by zero.) + bool noteUndefinedBehavior() { + EvalStatus.HasUndefinedBehavior = true; + return keepEvaluatingAfterUndefinedBehavior(); + } + /// Should we continue evaluation as much as possible after encountering a /// construct which can't be reduced to a value? bool keepEvaluatingAfterFailure() { @@ -1549,7 +1575,7 @@ static bool HandleOverflow(EvalInfo &Info, const Expr *E, const T &SrcValue, QualType DestType) { Info.CCEDiag(E, diag::note_constexpr_overflow) << SrcValue << DestType; - return Info.noteSideEffect(); + return Info.noteUndefinedBehavior(); } static bool HandleFloatToIntCast(EvalInfo &Info, const Expr *E, @@ -1818,8 +1844,7 @@ static bool handleFloatFloatBinOp(EvalInfo &Info, const Expr *E, if (LHS.isInfinity() || LHS.isNaN()) { Info.CCEDiag(E, diag::note_constexpr_float_arithmetic) << LHS.isNaN(); - // Undefined behavior is a side-effect. - return Info.noteSideEffect(); + return Info.noteUndefinedBehavior(); } return true; } @@ -8835,6 +8860,12 @@ bool Expr::EvaluateAsBooleanCondition(bool &Result, HandleConversionToBool(Scratch.Val, Result); } +static bool hasUnacceptableSideEffect(Expr::EvalStatus &Result, + Expr::SideEffectsKind SEK) { + return (SEK < Expr::SE_AllowSideEffects && Result.HasSideEffects) || + (SEK < Expr::SE_AllowUndefinedBehavior && Result.HasUndefinedBehavior); +} + bool Expr::EvaluateAsInt(APSInt &Result, const ASTContext &Ctx, SideEffectsKind AllowSideEffects) const { if (!getType()->isIntegralOrEnumerationType()) @@ -8842,7 +8873,7 @@ bool Expr::EvaluateAsInt(APSInt &Result, const ASTContext &Ctx, EvalResult ExprResult; if (!EvaluateAsRValue(ExprResult, Ctx) || !ExprResult.Val.isInt() || - (!AllowSideEffects && ExprResult.HasSideEffects)) + hasUnacceptableSideEffect(ExprResult, AllowSideEffects)) return false; Result = ExprResult.Val.getInt(); @@ -8905,9 +8936,10 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx, /// isEvaluatable - Call EvaluateAsRValue to see if this expression can be /// constant folded, but discard the result. -bool Expr::isEvaluatable(const ASTContext &Ctx) const { +bool Expr::isEvaluatable(const ASTContext &Ctx, SideEffectsKind SEK) const { EvalResult Result; - return EvaluateAsRValue(Result, Ctx) && !Result.HasSideEffects; + return EvaluateAsRValue(Result, Ctx) && + !hasUnacceptableSideEffect(Result, SEK); } APSInt Expr::EvaluateKnownConstInt(const ASTContext &Ctx, diff --git a/test/CodeGen/complex-init-list.c b/test/CodeGen/complex-init-list.c index fa266b26bb..bc38e2caf2 100644 --- a/test/CodeGen/complex-init-list.c +++ b/test/CodeGen/complex-init-list.c @@ -4,8 +4,8 @@ // of a complex number individually using an initialization list. (There is a // extensive description and test in test/Sema/complex-init-list.c.) -_Complex float x = { 1.0f, -1.0f }; -// CHECK: @x = global { float, float } { float 1.000000e+00, float -1.000000e+00 }, align 4 +_Complex float x = { 1.0f, 1.0f/0.0f }; +// CHECK: @x = global { float, float } { float 1.000000e+00, float 0x7FF0000000000000 }, align 4 _Complex float f(float x, float y) { _Complex float z = { x, y }; return z; } // CHECK-LABEL: define <2 x float> @f diff --git a/test/PCH/floating-literal.c b/test/PCH/floating-literal.c index 9037deaf4e..b5ff6fe84b 100644 --- a/test/PCH/floating-literal.c +++ b/test/PCH/floating-literal.c @@ -15,5 +15,5 @@ long double foo = 1.0E4000L; double bar = 1.0E300; // CHECK: double bar = 1.0000000000000001E+300; -float wibble = 2.0E38; -// CHECK: float wibble = 2.0E+38; +float wibble = 1.0E40; +// CHECK: float wibble = 1.0E+40; diff --git a/test/Sema/const-eval.c b/test/Sema/const-eval.c index 7c2cd0efd5..bfb58bc573 100644 --- a/test/Sema/const-eval.c +++ b/test/Sema/const-eval.c @@ -129,7 +129,7 @@ extern struct Test50S Test50; EVAL_EXPR(50, &Test50 < (struct Test50S*)((unsigned)&Test50 + 10)) // expected-error {{must have a constant size}} // -EVAL_EXPR(51, 0 != (float)1e38) +EVAL_EXPR(51, 0 != (float)1e99) // PR21945 void PR21945() { int i = (({}), 0l); } diff --git a/test/Sema/integer-overflow.c b/test/Sema/integer-overflow.c index 8f74873ed9..44fbcd4c81 100644 --- a/test/Sema/integer-overflow.c +++ b/test/Sema/integer-overflow.c @@ -7,7 +7,6 @@ uint64_t f1(uint64_t, uint32_t); uint64_t f2(uint64_t, ...); static const uint64_t overflow = 1 * 4608 * 1024 * 1024; // expected-warning {{overflow in expression; result is 536870912 with type 'int'}} -// expected-error@-1 {{not a compile-time constant}} uint64_t check_integer_overflows(int i) { // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}} diff --git a/test/Sema/switch-1.c b/test/Sema/switch-1.c index 0d31a191c4..144c3607f5 100644 --- a/test/Sema/switch-1.c +++ b/test/Sema/switch-1.c @@ -55,8 +55,5 @@ int f(int i) { // rdar://18405357 unsigned long long l = 65536 * 65536; // expected-warning {{overflow in expression; result is 0 with type 'int'}} -#ifndef __cplusplus -// expected-error@-2 {{not a compile-time constant}} -#endif unsigned long long l2 = 65536 * (unsigned)65536; unsigned long long l3 = 65536 * 65536ULL; diff --git a/test/SemaCXX/enum.cpp b/test/SemaCXX/enum.cpp index 370e1c34d2..6b0824b751 100644 --- a/test/SemaCXX/enum.cpp +++ b/test/SemaCXX/enum.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -fsyntax-only -pedantic -std=c++98 -verify -triple x86_64-apple-darwin %s +// RUN: %clang_cc1 -fsyntax-only -pedantic -std=c++11 -verify -triple x86_64-apple-darwin %s enum E { // expected-note{{previous definition is here}} Val1, Val2 @@ -88,10 +89,24 @@ typedef enum { }; // expected-warning{{typedef requires a name}} // PR7921 enum PR7921E { - PR7921V = (PR7921E)(123) // expected-error {{expression is not an integral constant expression}} + PR7921V = (PR7921E)(123) +#if __cplusplus < 201103L +// expected-error@-2 {{expression is not an integral constant expression}} +#else +// expected-error@-4 {{must have integral or unscoped enumeration type}} +// FIXME: The above diagnostic isn't very good; we should instead complain about the type being incomplete. +#endif }; void PR8089() { enum E; // expected-error{{ISO C++ forbids forward references to 'enum' types}} int a = (E)3; // expected-error{{cannot initialize a variable of type 'int' with an rvalue of type 'E'}} } + +// 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 {{not an integral constant expression}} +// expected-note@-3 {{value 28958703552 is outside the range of representable values}} +#endif -- 2.40.0