[cfe-commits] r149490 - in /cfe/trunk: lib/AST/ExprConstant.cpp test/CXX/expr/expr.const/p2-0x.cpp

[Richard Smith]

Author: rsmith
Date: Wed Feb  1 02:10:20 2012
New Revision: 149490

URL: http://llvm.org/viewvc/llvm-project?rev=149490&view=rev
Log:
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.

Modified:
    cfe/trunk/lib/AST/ExprConstant.cpp
    cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=149490&r1=149489&r2=149490&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Feb  1 02:10:20 2012
@@ -4347,6 +4347,9 @@
       //  - 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 @@
         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

Modified: cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp?rev=149490&r1=149489&r2=149490&view=diff
==============================================================================
--- cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp (original)
+++ cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp Wed Feb  1 02:10:20 2012
@@ -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 @@
     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