PR28739: Check that integer values fit into 64 bits before extracting them as 64 bit values for pointer arithmetic.

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.

llvm-svn: 293568

3 files changed, 61 insertions, 32 deletions

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 92c81072517..f2e76288cb8 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/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/clang/test/Sema/const-eval.c b/clang/test/Sema/const-eval.c
index bfb58bc573e..8a8e6fddd12 100644
--- a/clang/test/Sema/const-eval.c
+++ b/clang/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/clang/test/SemaCXX/constant-expression-cxx1y.cpp b/clang/test/SemaCXX/constant-expression-cxx1y.cpp
index dfdf50ad548..274730d923d 100644
--- a/clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ b/clang/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}}
+}