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.

llvm-svn: 149490

1 files changed, 23 insertions, 6 deletions

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 731bddd6b8c..3209bb343b0 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/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