constexpr: check for overflow in pointer subtraction.

This is a mess. According to the C++11 standard, pointer subtraction only has
undefined behavior if the difference of the array indices does not fit into a
ptrdiff_t.

However, common implementations effectively perform a char* subtraction first,
and then divide the result by the element size, which can cause overflows in
some cases. Those cases are not considered to be undefined behavior by this
change; perhaps they should be.

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

diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp
index 731bddd6b8c842c5cff8031e97e53a02325d7c3c..3209bb343b05537a75ef3d18bcf65c4f33990491 100644 (file)
--- a/lib/AST/ExprConstant.cpp
+++ b/lib/AST/ExprConstant.cpp
@@ -4347,6 +4347,9 @@ bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
       //  - Pointer subtractions must be on elements of the same array.
       //  - Pointer comparisons must be between members with the same access.
 
+      const CharUnits &LHSOffset = LHSValue.getLValueOffset();
+      const CharUnits &RHSOffset = RHSValue.getLValueOffset();
+
       if (E->getOpcode() == BO_Sub) {
         QualType Type = E->getLHS()->getType();
         QualType ElementType = Type->getAs<PointerType>()->getPointeeType();
@@ -4355,14 +4358,28 @@ bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
         if (!HandleSizeof(Info, ElementType, ElementSize))
           return false;
 
-        CharUnits Diff = LHSValue.getLValueOffset() -
-                             RHSValue.getLValueOffset();
-        return Success(Diff / ElementSize, E);
+        // FIXME: LLVM and GCC both compute LHSOffset - RHSOffset at runtime,
+        // and produce incorrect results when it overflows. Such behavior
+        // appears to be non-conforming, but is common, so perhaps we should
+        // assume the standard intended for such cases to be undefined behavior
+        // and check for them.
+
+        // Compute (LHSOffset - RHSOffset) / Size carefully, checking for
+        // overflow in the final conversion to ptrdiff_t.
+        APSInt LHS(
+          llvm::APInt(65, (int64_t)LHSOffset.getQuantity(), true), false);
+        APSInt RHS(
+          llvm::APInt(65, (int64_t)RHSOffset.getQuantity(), true), false);
+        APSInt ElemSize(
+          llvm::APInt(65, (int64_t)ElementSize.getQuantity(), true), false);
+        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());
+        return Success(Result, E);
       }
 
-      const CharUnits &LHSOffset = LHSValue.getLValueOffset();
-      const CharUnits &RHSOffset = RHSValue.getLValueOffset();
-
       // C++11 [expr.rel]p3:
       //   Pointers to void (after pointer conversions) can be compared, with a
       //   result defined as follows: If both pointers represent the same

diff --git a/test/CXX/expr/expr.const/p2-0x.cpp b/test/CXX/expr/expr.const/p2-0x.cpp
index 857f02960f8a98ffddd8588f386b45fa7e127c25..8e68b9a0fb6348bf47e2ce7a1da60a210992c138 100644 (file)
--- a/test/CXX/expr/expr.const/p2-0x.cpp
+++ b/test/CXX/expr/expr.const/p2-0x.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -pedantic -verify -fcxx-exceptions %s -fconstexpr-depth 128
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -pedantic -verify -fcxx-exceptions %s -fconstexpr-depth 128 -triple i686-pc-linux-gnu
 
 // A conditional-expression is a core constant expression unless it involves one
 // of the following as a potentially evaluated subexpression [...]:
@@ -222,6 +222,14 @@ namespace UndefinedBehavior {
     constexpr long long ll4 = ll3 - 1; // expected-error {{constant}} expected-note {{ -9223372036854775809 }}
     constexpr long long ll5 = ll3 * ll3; // expected-error {{constant}} expected-note {{ 85070591730234615865843651857942052864 }}
 
+    // Yikes.
+    char melchizedek[2200000000];
+    typedef decltype(melchizedek[1] - melchizedek[0]) ptrdiff_t;
+    constexpr ptrdiff_t d1 = &melchizedek[0x7fffffff] - &melchizedek[0]; // ok
+    constexpr ptrdiff_t d2 = &melchizedek[0x80000000u] - &melchizedek[0]; // expected-error {{constant expression}} expected-note {{ 2147483648 }}
+    constexpr ptrdiff_t d3 = &melchizedek[0] - &melchizedek[0x80000000u]; // ok
+    constexpr ptrdiff_t d4 = &melchizedek[0] - &melchizedek[0x80000001u]; // expected-error {{constant expression}} expected-note {{ -2147483649 }}
+
     // Unsigned int overflow.
     static_assert(65536u * 65536u == 0u, ""); // ok
     static_assert(4294967295u + 1u == 0u, ""); // ok