PR28739: Check that integer values fit into 64 bits before extracting them as 64...

This fixes various ways to tickle an assertion in constant expression
evaluation when using __int128. Longer term, we need to figure out what should
happen here: either any kind of overflow in offset calculation should result in
a non-constant value or we should truncate to 64 bits. In C++11 onwards, we're
effectively already checking for overflow because we strictly enforce array
bounds checks, but even there some forms of overflow can slip past undetected.

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

diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp
index 92c81072517f7bc6a5ad529e9481b3fb71b7af48..f2e76288cb8b7005a4df9ea5f877045b67c4b505 100644 (file)
--- a/lib/AST/ExprConstant.cpp
+++ b/lib/AST/ExprConstant.cpp
@@ -1460,9 +1460,17 @@ static bool EvaluateIgnoredValue(EvalInfo &Info, const Expr *E) {
 
 /// Sign- or zero-extend a value to 64 bits. If it's already 64 bits, just
 /// return its existing value.
-static int64_t getExtValue(const APSInt &Value) {
-  return Value.isSigned() ? Value.getSExtValue()
-                          : static_cast<int64_t>(Value.getZExtValue());
+static bool getExtValue(EvalInfo &Info, const Expr *E, const APSInt &Value,
+                        int64_t &Result) {
+  if (Value.isSigned() ? Value.getMinSignedBits() > 64
+                       : Value.getActiveBits() > 64) {
+    Info.FFDiag(E);
+    return false;
+  }
+
+  Result = Value.isSigned() ? Value.getSExtValue()
+                            : static_cast<int64_t>(Value.getZExtValue());
+  return true;
 }
 
 /// Should this call expression be treated as a string literal?
@@ -3184,7 +3192,9 @@ struct CompoundAssignSubobjectHandler {
       return false;
     }
 
-    int64_t Offset = getExtValue(RHS.getInt());
+    int64_t Offset;
+    if (!getExtValue(Info, E, RHS.getInt(), Offset))
+      return false;
     if (Opcode == BO_Sub)
       Offset = -Offset;
 
@@ -5148,8 +5158,10 @@ bool LValueExprEvaluator::VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
   if (!EvaluateInteger(E->getIdx(), Index, Info))
     return false;
 
-  return HandleLValueArrayAdjustment(Info, E, Result, E->getType(),
-                                     getExtValue(Index));
+  int64_t Offset;
+  if (!getExtValue(Info, E, Index, Offset))
+    return false;
+  return HandleLValueArrayAdjustment(Info, E, Result, E->getType(), Offset);
 }
 
 bool LValueExprEvaluator::VisitUnaryDeref(const UnaryOperator *E) {
@@ -5415,7 +5427,9 @@ bool PointerExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
   if (!EvaluateInteger(IExp, Offset, Info) || !EvalPtrOK)
     return false;
 
-  int64_t AdditionalOffset = getExtValue(Offset);
+  int64_t AdditionalOffset;
+  if (!getExtValue(Info, E, Offset, AdditionalOffset))
+    return false;
   if (E->getOpcode() == BO_Sub)
     AdditionalOffset = -AdditionalOffset;
 
@@ -5614,14 +5628,14 @@ bool PointerExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
     APSInt Alignment;
     if (!EvaluateInteger(E->getArg(1), Alignment, Info))
       return false;
-    CharUnits Align = CharUnits::fromQuantity(getExtValue(Alignment));
+    CharUnits Align = CharUnits::fromQuantity(Alignment.getZExtValue());
 
     if (E->getNumArgs() > 2) {
       APSInt Offset;
       if (!EvaluateInteger(E->getArg(2), Offset, Info))
         return false;
 
-      int64_t AdditionalOffset = -getExtValue(Offset);
+      int64_t AdditionalOffset = -Offset.getZExtValue();
       OffsetResult.Offset += CharUnits::fromQuantity(AdditionalOffset);
     }
 
@@ -5638,12 +5652,11 @@ bool PointerExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
 
       if (BaseAlignment < Align) {
         Result.Designator.setInvalid();
-        // FIXME: Quantities here cast to integers because the plural modifier
-        // does not work on APSInts yet.
+        // FIXME: Add support to Diagnostic for long / long long.
         CCEDiag(E->getArg(0),
                 diag::note_constexpr_baa_insufficient_alignment) << 0
-          << (int) BaseAlignment.getQuantity()
-          << (unsigned) getExtValue(Alignment);
+          << (unsigned)BaseAlignment.getQuantity()
+          << (unsigned)Align.getQuantity();
         return false;
       }
     }
@@ -5651,18 +5664,14 @@ bool PointerExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
     // The offset must also have the correct alignment.
     if (OffsetResult.Offset.alignTo(Align) != OffsetResult.Offset) {
       Result.Designator.setInvalid();
-      APSInt Offset(64, false);
-      Offset = OffsetResult.Offset.getQuantity();
-
-      if (OffsetResult.Base)
-        CCEDiag(E->getArg(0),
-                diag::note_constexpr_baa_insufficient_alignment) << 1
-          << (int) getExtValue(Offset) << (unsigned) getExtValue(Alignment);
-      else
-        CCEDiag(E->getArg(0),
-                diag::note_constexpr_baa_value_insufficient_alignment)
-          << Offset << (unsigned) getExtValue(Alignment);
 
+      (OffsetResult.Base
+           ? CCEDiag(E->getArg(0),
+                     diag::note_constexpr_baa_insufficient_alignment) << 1
+           : CCEDiag(E->getArg(0),
+                     diag::note_constexpr_baa_value_insufficient_alignment))
+        << (int)OffsetResult.Offset.getQuantity()
+        << (unsigned)Align.getQuantity();
       return false;
     }
 
@@ -7968,12 +7977,13 @@ bool DataRecursiveIntBinOpEvaluator::
   // Handle cases like (unsigned long)&a + 4.
   if (E->isAdditiveOp() && LHSVal.isLValue() && RHSVal.isInt()) {
     Result = LHSVal;
-    CharUnits AdditionalOffset =
-        CharUnits::fromQuantity(RHSVal.getInt().getZExtValue());
+    int64_t Offset;
+    if (!getExtValue(Info, E, RHSVal.getInt(), Offset))
+      return false;
     if (E->getOpcode() == BO_Add)
-      Result.getLValueOffset() += AdditionalOffset;
+      Result.getLValueOffset() += CharUnits::fromQuantity(Offset);
     else
-      Result.getLValueOffset() -= AdditionalOffset;
+      Result.getLValueOffset() -= CharUnits::fromQuantity(Offset);
     return true;
   }
   
@@ -7981,8 +7991,10 @@ bool DataRecursiveIntBinOpEvaluator::
   if (E->getOpcode() == BO_Add &&
       RHSVal.isLValue() && LHSVal.isInt()) {
     Result = RHSVal;
-    Result.getLValueOffset() +=
-        CharUnits::fromQuantity(LHSVal.getInt().getZExtValue());
+    int64_t Offset;
+    if (!getExtValue(Info, E, LHSVal.getInt(), Offset))
+      return false;
+    Result.getLValueOffset() += CharUnits::fromQuantity(Offset);
     return true;
   }
   

diff --git a/test/Sema/const-eval.c b/test/Sema/const-eval.c
index bfb58bc573e1e4abea4d97f395a63ee152848ccb..8a8e6fddd12c6b4563f2027a90f361b02be97212 100644 (file)
--- a/test/Sema/const-eval.c
+++ b/test/Sema/const-eval.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -triple i686-linux %s -Wno-tautological-pointer-compare
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux %s -Wno-tautological-pointer-compare
 
 #define EVAL_EXPR(testno, expr) int test##testno = sizeof(struct{char qq[expr];});
 int x;
@@ -126,7 +126,7 @@ EVAL_EXPR(48, &x != &x - 1 ? 1 : -1)
 EVAL_EXPR(49, &x < &x - 100 ? 1 : -1) // expected-error {{must have a constant size}}
 
 extern struct Test50S Test50;
-EVAL_EXPR(50, &Test50 < (struct Test50S*)((unsigned)&Test50 + 10)) // expected-error {{must have a constant size}}
+EVAL_EXPR(50, &Test50 < (struct Test50S*)((unsigned long)&Test50 + 10)) // expected-error {{must have a constant size}}
 
 // <rdar://problem/11874571>
 EVAL_EXPR(51, 0 != (float)1e99)
@@ -137,3 +137,15 @@ void PR21945() { int i = (({}), 0l); }
 void PR24622();
 struct PR24622 {} pr24622;
 EVAL_EXPR(52, &pr24622 == (void *)&PR24622); // expected-error {{must have a constant size}}
+
+// Reject cases that would require more than 64 bits of pointer offset to
+// represent.
+// FIXME: We don't check for all offset overflow, just immediate adjustments
+// that don't fit in 64 bits. We should consistently either check all offset
+// adjustments or truncate to 64 bits everywhere.
+void *PR28739a = (__int128)(unsigned long)-1 + &PR28739a; // expected-error {{not a compile-time constant}}
+void *PR28739b = &PR28739b + (__int128)(unsigned long)-1; // expected-error {{not a compile-time constant}}
+__int128 PR28739c = (&PR28739c + (__int128)(unsigned long)-1) - &PR28739c; // expected-error {{not a compile-time constant}}
+void *PR28739d = &(&PR28739d)[(__int128)(unsigned long)-1]; // expected-error {{not a compile-time constant}}
+__int128 PR28739e = (__int128)(unsigned long)-1 + (long)&PR28739e; // expected-error {{not a compile-time constant}}
+__int128 PR28739f = (long)&PR28739f + (__int128)(unsigned long)-1; // expected-error {{not a compile-time constant}}

diff --git a/test/SemaCXX/constant-expression-cxx1y.cpp b/test/SemaCXX/constant-expression-cxx1y.cpp
index dfdf50ad54849dcd3e926f4b2c87451047c7c15c..274730d923df20f51b461a8f8cde257bfb5f800c 100644 (file)
--- a/test/SemaCXX/constant-expression-cxx1y.cpp
+++ b/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -977,3 +977,8 @@ void run() { foo(); }
 static_assert(sum(Cs) == 'a' + 'b', ""); // expected-error{{not an integral constant expression}} expected-note{{in call to 'sum(Cs)'}}
 constexpr int S = sum(Cs); // expected-error{{must be initialized by a constant expression}} expected-note{{in call}}
 }
+
+constexpr void PR28739(int n) { // expected-error {{never produces a constant}}
+  int *p = &n;
+  p += (__int128)(unsigned long)-1; // expected-note {{subexpression}}
+}