]> granicus.if.org Git - clang/commitdiff
PR17381: Treat undefined behavior during expression evaluation as an unmodeled
authorRichard Smith <richard-llvm@metafoo.co.uk>
Thu, 3 Dec 2015 01:36:22 +0000 (01:36 +0000)
committerRichard Smith <richard-llvm@metafoo.co.uk>
Thu, 3 Dec 2015 01:36:22 +0000 (01:36 +0000)
side-effect, so that we don't allow speculative evaluation of such expressions
during code generation.

This caused a diagnostic quality regression, so fix constant expression
diagnostics to prefer either the first "can't be constant folded" diagnostic or
the first "not a constant expression" diagnostic depending on the kind of
evaluation we're doing. This was always the intent, but didn't quite work
correctly before.

This results in certain initializers that used to be constant initializers to
no longer be; in particular, things like:

  float f = 1e100;

are no longer accepted in C. This seems appropriate, as such constructs would
lead to code being executed if sanitizers are enabled.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@254574 91177308-0d34-0410-b5e6-96231b3b80d8

lib/AST/ExprConstant.cpp
lib/Sema/SemaChecking.cpp
test/CXX/expr/expr.const/p2-0x.cpp
test/CodeGen/complex-init-list.c
test/CodeGen/ubsan-conditional.c [new file with mode: 0644]
test/PCH/floating-literal.c
test/Sema/const-eval.c
test/Sema/integer-overflow.c
test/Sema/switch-1.c
test/SemaCXX/constant-expression-cxx11.cpp
test/SemaCXX/constexpr-printing.cpp

index a4d6a8950e41f8fea20fa6b33d358b7ce9954765..d80f466d99f5ea779eae96b21ee480a82ae49124 100644 (file)
@@ -473,6 +473,10 @@ namespace {
     /// notes attached to it will also be stored, otherwise they will not be.
     bool HasActiveDiagnostic;
 
+    /// \brief Have we emitted a diagnostic explaining why we couldn't constant
+    /// fold (not just why it's not strictly a constant expression)?
+    bool HasFoldFailureDiagnostic;
+
     enum EvaluationMode {
       /// Evaluate as a constant expression. Stop if we find that the expression
       /// is not a constant expression.
@@ -537,7 +541,7 @@ namespace {
         BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
         EvaluatingDecl((const ValueDecl *)nullptr),
         EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
-        EvalMode(Mode) {}
+        HasFoldFailureDiagnostic(false), EvalMode(Mode) {}
 
     void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value) {
       EvaluatingDecl = Base;
@@ -597,7 +601,7 @@ namespace {
     /// Diagnose that the evaluation cannot be folded.
     OptionalDiagnostic Diag(SourceLocation Loc, diag::kind DiagId
                               = diag::note_invalid_subexpr_in_const_expr,
-                            unsigned ExtraNotes = 0) {
+                            unsigned ExtraNotes = 0, bool IsCCEDiag = false) {
       if (EvalStatus.Diag) {
         // If we have a prior diagnostic, it will be noting that the expression
         // isn't a constant expression. This diagnostic is more important,
@@ -610,10 +614,9 @@ namespace {
           case EM_ConstantFold:
           case EM_IgnoreSideEffects:
           case EM_EvaluateForOverflow:
-            if (!EvalStatus.HasSideEffects)
+            if (!HasFoldFailureDiagnostic)
               break;
-            // We've had side-effects; we want the diagnostic from them, not
-            // some later problem.
+            // We've already failed to fold something. Keep that diagnostic.
           case EM_ConstantExpression:
           case EM_PotentialConstantExpression:
           case EM_ConstantExpressionUnevaluated:
@@ -632,6 +635,7 @@ namespace {
           CallStackNotes = 0;
 
         HasActiveDiagnostic = true;
+        HasFoldFailureDiagnostic = !IsCCEDiag;
         EvalStatus.Diag->clear();
         EvalStatus.Diag->reserve(1 + ExtraNotes + CallStackNotes);
         addDiag(Loc, DiagId);
@@ -645,9 +649,9 @@ namespace {
 
     OptionalDiagnostic Diag(const Expr *E, diag::kind DiagId
                               = diag::note_invalid_subexpr_in_const_expr,
-                            unsigned ExtraNotes = 0) {
+                            unsigned ExtraNotes = 0, bool IsCCEDiag = false) {
       if (EvalStatus.Diag)
-        return Diag(E->getExprLoc(), DiagId, ExtraNotes);
+        return Diag(E->getExprLoc(), DiagId, ExtraNotes, IsCCEDiag);
       HasActiveDiagnostic = false;
       return OptionalDiagnostic();
     }
@@ -667,7 +671,7 @@ namespace {
         HasActiveDiagnostic = false;
         return OptionalDiagnostic();
       }
-      return Diag(Loc, DiagId, ExtraNotes);
+      return Diag(Loc, DiagId, ExtraNotes, true);
     }
 
     /// Add a note to a prior diagnostic.
@@ -1541,10 +1545,11 @@ static bool EvaluateAsBooleanCondition(const Expr *E, bool &Result,
 }
 
 template<typename T>
-static void HandleOverflow(EvalInfo &Info, const Expr *E,
+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();
 }
 
 static bool HandleFloatToIntCast(EvalInfo &Info, const Expr *E,
@@ -1558,7 +1563,7 @@ static bool HandleFloatToIntCast(EvalInfo &Info, const Expr *E,
   bool ignored;
   if (Value.convertToInteger(Result, llvm::APFloat::rmTowardZero, &ignored)
       & APFloat::opInvalidOp)
-    HandleOverflow(Info, E, Value, DestType);
+    return HandleOverflow(Info, E, Value, DestType);
   return true;
 }
 
@@ -1570,7 +1575,7 @@ static bool HandleFloatToFloatCast(EvalInfo &Info, const Expr *E,
   if (Result.convert(Info.Ctx.getFloatTypeSemantics(DestType),
                      APFloat::rmNearestTiesToEven, &ignored)
       & APFloat::opOverflow)
-    HandleOverflow(Info, E, Value, DestType);
+    return HandleOverflow(Info, E, Value, DestType);
   return true;
 }
 
@@ -1593,7 +1598,7 @@ static bool HandleIntToFloatCast(EvalInfo &Info, const Expr *E,
   if (Result.convertFromAPInt(Value, Value.isSigned(),
                               APFloat::rmNearestTiesToEven)
       & APFloat::opOverflow)
-    HandleOverflow(Info, E, Value, DestType);
+    return HandleOverflow(Info, E, Value, DestType);
   return true;
 }
 
@@ -1669,23 +1674,26 @@ static bool EvalAndBitcastToAPInt(EvalInfo &Info, const Expr *E,
 /// bits, and check for overflow in the original type (if that type was not an
 /// unsigned type).
 template<typename Operation>
-static APSInt CheckedIntArithmetic(EvalInfo &Info, const Expr *E,
-                                   const APSInt &LHS, const APSInt &RHS,
-                                   unsigned BitWidth, Operation Op) {
-  if (LHS.isUnsigned())
-    return Op(LHS, RHS);
+static bool CheckedIntArithmetic(EvalInfo &Info, const Expr *E,
+                                 const APSInt &LHS, const APSInt &RHS,
+                                 unsigned BitWidth, Operation Op,
+                                 APSInt &Result) {
+  if (LHS.isUnsigned()) {
+    Result = Op(LHS, RHS);
+    return true;
+  }
 
   APSInt Value(Op(LHS.extend(BitWidth), RHS.extend(BitWidth)), false);
-  APSInt Result = Value.trunc(LHS.getBitWidth());
+  Result = Value.trunc(LHS.getBitWidth());
   if (Result.extend(BitWidth) != Value) {
     if (Info.checkingForOverflow())
       Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
-        diag::warn_integer_constant_overflow)
+                                       diag::warn_integer_constant_overflow)
           << Result.toString(10) << E->getType();
     else
-      HandleOverflow(Info, E, Value, E->getType());
+      return HandleOverflow(Info, E, Value, E->getType());
   }
-  return Result;
+  return true;
 }
 
 /// Perform the given binary integer operation.
@@ -1697,17 +1705,14 @@ static bool handleIntIntBinOp(EvalInfo &Info, const Expr *E, const APSInt &LHS,
     Info.Diag(E);
     return false;
   case BO_Mul:
-    Result = CheckedIntArithmetic(Info, E, LHS, RHS, LHS.getBitWidth() * 2,
-                                  std::multiplies<APSInt>());
-    return true;
+    return CheckedIntArithmetic(Info, E, LHS, RHS, LHS.getBitWidth() * 2,
+                                std::multiplies<APSInt>(), Result);
   case BO_Add:
-    Result = CheckedIntArithmetic(Info, E, LHS, RHS, LHS.getBitWidth() + 1,
-                                  std::plus<APSInt>());
-    return true;
+    return CheckedIntArithmetic(Info, E, LHS, RHS, LHS.getBitWidth() + 1,
+                                std::plus<APSInt>(), Result);
   case BO_Sub:
-    Result = CheckedIntArithmetic(Info, E, LHS, RHS, LHS.getBitWidth() + 1,
-                                  std::minus<APSInt>());
-    return true;
+    return CheckedIntArithmetic(Info, E, LHS, RHS, LHS.getBitWidth() + 1,
+                                std::minus<APSInt>(), Result);
   case BO_And: Result = LHS & RHS; return true;
   case BO_Xor: Result = LHS ^ RHS; return true;
   case BO_Or:  Result = LHS | RHS; return true;
@@ -1717,11 +1722,13 @@ static bool handleIntIntBinOp(EvalInfo &Info, const Expr *E, const APSInt &LHS,
       Info.Diag(E, diag::note_expr_divide_by_zero);
       return false;
     }
-    // Check for overflow case: INT_MIN / -1 or INT_MIN % -1.
+    Result = (Opcode == BO_Rem ? LHS % RHS : LHS / RHS);
+    // Check for overflow case: INT_MIN / -1 or INT_MIN % -1. APSInt supports
+    // this operation and gives the two's complement result.
     if (RHS.isNegative() && RHS.isAllOnesValue() &&
         LHS.isSigned() && LHS.isMinSignedValue())
-      HandleOverflow(Info, E, -LHS.extend(LHS.getBitWidth() + 1), E->getType());
-    Result = (Opcode == BO_Rem ? LHS % RHS : LHS / RHS);
+      return HandleOverflow(Info, E, -LHS.extend(LHS.getBitWidth() + 1),
+                            E->getType());
     return true;
   case BO_Shl: {
     if (Info.getLangOpts().OpenCL)
@@ -1809,8 +1816,11 @@ static bool handleFloatFloatBinOp(EvalInfo &Info, const Expr *E,
     break;
   }
 
-  if (LHS.isInfinity() || LHS.isNaN())
+  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 true;
 }
 
@@ -3009,7 +3019,7 @@ struct IncDecSubobjectHandler {
       if (!WasNegative && Value.isNegative() &&
           isOverflowingIntegerType(Info.Ctx, SubobjType)) {
         APSInt ActualValue(Value, /*IsUnsigned*/true);
-        HandleOverflow(Info, E, ActualValue, SubobjType);
+        return HandleOverflow(Info, E, ActualValue, SubobjType);
       }
     } else {
       --Value;
@@ -3019,7 +3029,7 @@ struct IncDecSubobjectHandler {
         unsigned BitWidth = Value.getBitWidth();
         APSInt ActualValue(Value.sext(BitWidth + 1), /*IsUnsigned*/false);
         ActualValue.setBit(BitWidth);
-        HandleOverflow(Info, E, ActualValue, SubobjType);
+        return HandleOverflow(Info, E, ActualValue, SubobjType);
       }
     }
     return true;
@@ -6240,8 +6250,8 @@ static bool EvaluateBuiltinConstantP(ASTContext &Ctx, const Expr *Arg) {
     APValue &V = Result.Val;
     if (V.getKind() == APValue::Int)
       return true;
-
-    return EvaluateBuiltinConstantPForLValue(V);
+    if (V.getKind() == APValue::LValue)
+      return EvaluateBuiltinConstantPForLValue(V);
   } else if (ArgType->isFloatingType() || ArgType->isAnyComplexType()) {
     return Arg->isEvaluatable(Ctx);
   } else if (ArgType->isPointerType() || Arg->isGLValue()) {
@@ -7258,7 +7268,7 @@ bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
       LValue LHSValue, RHSValue;
 
       bool LHSOK = EvaluatePointer(E->getLHS(), LHSValue, Info);
-      if (!LHSOK && Info.keepEvaluatingAfterFailure())
+      if (!LHSOK && !Info.keepEvaluatingAfterFailure())
         return false;
 
       if (!EvaluatePointer(E->getRHS(), RHSValue, Info) || !LHSOK)
@@ -7270,21 +7280,20 @@ bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
         if (E->getOpcode() == BO_Sub) {
           // Handle &&A - &&B.
           if (!LHSValue.Offset.isZero() || !RHSValue.Offset.isZero())
-            return false;
+            return Error(E);
           const Expr *LHSExpr = LHSValue.Base.dyn_cast<const Expr*>();
           const Expr *RHSExpr = RHSValue.Base.dyn_cast<const Expr*>();
           if (!LHSExpr || !RHSExpr)
-            return false;
+            return Error(E);
           const AddrLabelExpr *LHSAddrExpr = dyn_cast<AddrLabelExpr>(LHSExpr);
           const AddrLabelExpr *RHSAddrExpr = dyn_cast<AddrLabelExpr>(RHSExpr);
           if (!LHSAddrExpr || !RHSAddrExpr)
-            return false;
+            return Error(E);
           // Make sure both labels come from the same function.
           if (LHSAddrExpr->getLabel()->getDeclContext() !=
               RHSAddrExpr->getLabel()->getDeclContext())
-            return false;
-          Result = APValue(LHSAddrExpr, RHSAddrExpr);
-          return true;
+            return Error(E);
+          return Success(APValue(LHSAddrExpr, RHSAddrExpr), E);
         }
         // Inequalities and subtractions between unrelated pointers have
         // unspecified or undefined behavior.
@@ -7375,8 +7384,9 @@ bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
         APSInt TrueResult = (LHS - RHS) / ElemSize;
         APSInt Result = TrueResult.trunc(Info.Ctx.getIntWidth(E->getType()));
 
-        if (Result.extend(65) != TrueResult)
-          HandleOverflow(Info, E, TrueResult, E->getType());
+        if (Result.extend(65) != TrueResult &&
+            !HandleOverflow(Info, E, TrueResult, E->getType()))
+          return false;
         return Success(Result, E);
       }
 
@@ -7662,9 +7672,10 @@ bool IntExprEvaluator::VisitUnaryOperator(const UnaryOperator *E) {
       return false;
     if (!Result.isInt()) return Error(E);
     const APSInt &Value = Result.getInt();
-    if (Value.isSigned() && Value.isMinSignedValue())
-      HandleOverflow(Info, E, -Value.extend(Value.getBitWidth() + 1),
-                     E->getType());
+    if (Value.isSigned() && Value.isMinSignedValue() &&
+        !HandleOverflow(Info, E, -Value.extend(Value.getBitWidth() + 1),
+                        E->getType()))
+      return false;
     return Success(-Value, E);
   }
   case UO_Not: {
@@ -8863,7 +8874,9 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx,
   Expr::EvalStatus EStatus;
   EStatus.Diag = &Notes;
 
-  EvalInfo InitInfo(Ctx, EStatus, EvalInfo::EM_ConstantFold);
+  EvalInfo InitInfo(Ctx, EStatus, VD->isConstexpr()
+                                      ? EvalInfo::EM_ConstantExpression
+                                      : EvalInfo::EM_ConstantFold);
   InitInfo.setEvaluatingDecl(VD, Value);
 
   LValue LVal;
index 31e952cef2171f77682891c6c03d5ffbd6810332..9d8a3f5240daa28411636ca79c34728c128591f6 100644 (file)
@@ -8547,7 +8547,7 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
     return;
 
   llvm::APSInt index;
-  if (!IndexExpr->EvaluateAsInt(index, Context))
+  if (!IndexExpr->EvaluateAsInt(index, Context, Expr::SE_AllowSideEffects))
     return;
   if (IndexNegated)
     index = -index;
index 2adefd928afb6282cf13c030f8c812c0c5987a77..c519ecbda2280f38a1f9920ae1a5f48bc59d2271 100644 (file)
@@ -601,11 +601,11 @@ namespace rdar13090123 {
   typedef __INTPTR_TYPE__ intptr_t;
 
   constexpr intptr_t f(intptr_t x) {
-    return (((x) >> 21) * 8); // expected-note{{subexpression not valid in a constant expression}}
+    return (((x) >> 21) * 8);
   }
 
   extern "C" int foo;
 
   constexpr intptr_t i = f((intptr_t)&foo - 10); // expected-error{{constexpr variable 'i' must be initialized by a constant expression}} \
-  // expected-note{{in call to 'f((char*)&foo + -10)'}}
+  // expected-note{{reinterpret_cast}}
 }
index bc38e2caf21fe7182c318226d24d0d24b96dcc3f..fa266b26bb962b520ba21e39dd7f4c37882260ad 100644 (file)
@@ -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/0.0f };
-// CHECK: @x = global { float, float } { float 1.000000e+00, float 0x7FF0000000000000 }, align 4
+_Complex float x = { 1.0f, -1.0f };
+// CHECK: @x = global { float, float } { float 1.000000e+00, float -1.000000e+00 }, 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/CodeGen/ubsan-conditional.c b/test/CodeGen/ubsan-conditional.c
new file mode 100644 (file)
index 0000000..7f63b39
--- /dev/null
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 %s -emit-llvm -fsanitize=float-divide-by-zero -o - | FileCheck %s
+
+_Bool b;
+// CHECK: @f(
+double f() {
+  // CHECK: %[[B:.*]] = load {{.*}} @b
+  // CHECK: %[[COND:.*]] = trunc {{.*}} %[[B]] to i1
+  // CHECK: br i1 %[[COND]]
+  return b ? 0.0 / 0.0 : 0.0;
+}
index b5ff6fe84b88ced44974a8945d9fd00092e7d7db..9037deaf4e97a280ef2751fc7b793f940357dec9 100644 (file)
@@ -15,5 +15,5 @@ long double foo = 1.0E4000L;
 double bar = 1.0E300;
 // CHECK: double bar = 1.0000000000000001E+300;
 
-float wibble = 1.0E40;
-// CHECK: float wibble = 1.0E+40;
+float wibble = 2.0E38;
+// CHECK: float wibble = 2.0E+38;
index bfb58bc573e1e4abea4d97f395a63ee152848ccb..7c2cd0efd5e8c017ddbbd79edc758bcac2e8e04b 100644 (file)
@@ -129,7 +129,7 @@ extern struct Test50S Test50;
 EVAL_EXPR(50, &Test50 < (struct Test50S*)((unsigned)&Test50 + 10)) // expected-error {{must have a constant size}}
 
 // <rdar://problem/11874571>
-EVAL_EXPR(51, 0 != (float)1e99)
+EVAL_EXPR(51, 0 != (float)1e38)
 
 // PR21945
 void PR21945() { int i = (({}), 0l); }
index 44fbcd4c818e621283591ca1cf07e6abd7bdbb8f..8f74873ed94830950ce713b8936c01351c5d83cc 100644 (file)
@@ -7,6 +7,7 @@ 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'}}
index 144c3607f57038ce2ea7618508057201b6ead68d..0d31a191c4ff180f2e231f0ba6cdfc811105a727 100644 (file)
@@ -55,5 +55,8 @@ 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;
index 794932df40956d7315a2ee75cba95eaa5aa0400d..7b9d0150e1e82b8f22a825d3beef96f655627196 100644 (file)
@@ -327,7 +327,7 @@ struct Str {
 };
 
 extern char externalvar[];
-constexpr bool constaddress = (void *)externalvar == (void *)0x4000UL; // expected-error {{must be initialized by a constant expression}}
+constexpr bool constaddress = (void *)externalvar == (void *)0x4000UL; // expected-error {{must be initialized by a constant expression}} expected-note {{reinterpret_cast}}
 constexpr bool litaddress = "foo" == "foo"; // expected-error {{must be initialized by a constant expression}} expected-warning {{unspecified}}
 static_assert(0 != "foo", "");
 
@@ -1874,10 +1874,9 @@ namespace NeverConstantTwoWays {
         0;
   }
 
-  // FIXME: We should diagnose the cast to long here, not the division by zero.
   constexpr int n = // expected-error {{must be initialized by a constant expression}}
-      (int *)(long)&n == &n ?
-        1 / 0 : // expected-warning {{division by zero}} expected-note {{division by zero}}
+      (int *)(long)&n == &n ? // expected-note {{reinterpret_cast}}
+        1 / 0 : // expected-warning {{division by zero}}
         0;
 }
 
index e545f45d6011ebeb69cf9898d8f4c546870abc4d..7f6a9c6a82f0deac8da8c7e786df6c3bafa68537 100644 (file)
@@ -90,10 +90,12 @@ constexpr wchar_t wc = get(L"test\0\\\"\t\a\b\234\u1234\xffffffff"); // \
 
 constexpr char32_t c32_err = get(U"\U00110000"); // expected-error {{invalid universal character}}
 
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
 typedef decltype(sizeof(int)) LabelDiffTy;
 constexpr LabelDiffTy mulBy3(LabelDiffTy x) { return x * 3; } // expected-note {{subexpression}}
 void LabelDiffTest() {
-  static_assert(mulBy3((LabelDiffTy)&&a-(LabelDiffTy)&&b) == 3, ""); // expected-error {{constant expression}} expected-note {{call to 'mulBy3(&&a - &&b)'}}
+  static_assert(mulBy3(fold((LabelDiffTy)&&a-(LabelDiffTy)&&b)) == 3, ""); // expected-error {{constant expression}} expected-note {{call to 'mulBy3(&&a - &&b)'}}
   a:b:return;
 }