[PATCH] D38337: Check for overflows when calculating the offset in GetGEPCost.

[Justin Lebar via Phabricator via llvm-commits]

jlebar created this revision.

This avoids C++ UB if the GEP is weird and the calculation overflows
int64_t, and it's also observable in the cost model's results.

Such GEPs are almost surely not valid pointers, but LLVM nonetheless
generates them sometimes.


https://reviews.llvm.org/D38337

Files:
  llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
  llvm/test/Analysis/CostModel/X86/gep.ll


Index: llvm/test/Analysis/CostModel/X86/gep.ll
===================================================================
--- llvm/test/Analysis/CostModel/X86/gep.ll
+++ llvm/test/Analysis/CostModel/X86/gep.ll
@@ -35,6 +35,11 @@
 ;CHECK: cost of 0 for instruction: {{.*}} getelementptr inbounds <4 x double>, <4 x double>*
   %a12 = getelementptr inbounds <4 x double>, <4 x double>* undef, i32 0
 
+  ; Check that we handle outlandishly large GEPs properly.  This is unlikely to
+  ; be a valid pointer, but LLVM still generates GEPs like this sometimes in
+  ; dead code.
+;CHECK: cost of 1 for instruction: {{.*}} getelementptr inbounds i64, i64*
+  %giant_gep_idx = getelementptr inbounds i64, i64* undef, i64 9223372036854775807
 
   ret void
 }
Index: llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
===================================================================
--- llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
+++ llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
@@ -674,7 +674,11 @@
       BaseGV = dyn_cast<GlobalValue>(Ptr->stripPointerCasts());
     }
     bool HasBaseReg = (BaseGV == nullptr);
-    int64_t BaseOffset = 0;
+
+    // Given a weird GEP, BaseOffset may overflow 64 bits, and we need to check
+    // for this, if only so that we don't have C++ signed integer overflows in
+    // this function.
+    APInt BaseOffset(/*numbits=*/64, 0);
     int64_t Scale = 0;
 
     auto GTI = gep_type_begin(PointeeType, Operands);
@@ -697,12 +701,31 @@
         // For structures the index is always splat or scalar constant
         assert(ConstIdx && "Unexpected GEP index");
         uint64_t Field = ConstIdx->getZExtValue();
-        BaseOffset += DL.getStructLayout(STy)->getElementOffset(Field);
+
+        // BaseOffset += DL.getStructLayout(STy)->getElementOffset(Field).
+        bool overflow = false;
+        BaseOffset = BaseOffset.sadd_ov(
+            APInt(
+                /*numbits=*/64,
+                DL.getStructLayout(STy)->getElementOffset(Field)),
+            overflow);
+        if (overflow)
+          return TTI::TCC_Basic;
       } else {
         int64_t ElementSize = DL.getTypeAllocSize(GTI.getIndexedType());
-        if (ConstIdx)
-          BaseOffset += ConstIdx->getSExtValue() * ElementSize;
-        else {
+        if (ConstIdx) {
+          // BaseOffset += ConstIdx->getSextValue() * ElementSize,
+          bool overflow;
+          APInt Inc =
+              APInt(/*numbits=*/64, ConstIdx->getSExtValue())
+                  .smul_ov(APInt(/*numbits=*/64, ElementSize), overflow);
+          if (!overflow) {
+            BaseOffset = BaseOffset.sadd_ov(Inc, overflow);
+          }
+
+          if (overflow)
+            return TTI::TCC_Basic;
+        } else {
           // Needs scale register.
           if (Scale != 0)
             // No addressing mode takes two scale registers.
@@ -716,8 +739,9 @@
     unsigned AS =
         (Ptr == nullptr ? 0 : Ptr->getType()->getPointerAddressSpace());
     if (static_cast<T *>(this)->isLegalAddressingMode(
-            TargetType, const_cast<GlobalValue *>(BaseGV), BaseOffset,
-            HasBaseReg, Scale, AS))
+            TargetType, const_cast<GlobalValue *>(BaseGV),
+            static_cast<int64_t>(BaseOffset.getLimitedValue()), HasBaseReg,
+            Scale, AS))
       return TTI::TCC_Free;
     return TTI::TCC_Basic;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38337.116893.patch
Type: text/x-patch
Size: 3371 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170927/c19893a6/attachment-0001.bin>