Make sure we don't combine a large displacement and a frame index in the same addressing mode on x86-64. It can overflow, leading to a crash/miscompile.

<rdar://problem/9763308>



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

2 files changed, 45 insertions, 8 deletions

diff --git a/lib/Target/X86/X86ISelDAGToDAG.cpp b/lib/Target/X86/X86ISelDAGToDAG.cpp
index b617e97..2b0f283 100644
--- a/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ b/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -548,17 +548,33 @@ void X86DAGToDAGISel::EmitFunctionEntryCode() {
       EmitSpecialCodeForMain(MF->begin(), MF->getFrameInfo());
 }
 
+static bool isDispSafeForFrameIndex(int64_t Val) {
+  // On 64-bit platforms, we can run into an issue where a frame index
+  // includes a displacement that, when added to the explicit displacement,
+  // will overflow the displacement field. Assuming that the frame index
+  // displacement fits into a 31-bit integer  (which is only slightly more
+  // aggressive than the current fundamental assumption that it fits into
+  // a 32-bit integer), a 31-bit disp should always be safe.
+  return isInt<31>(Val);
+}
+
 bool X86DAGToDAGISel::FoldOffsetIntoAddress(uint64_t Offset,
                                             X86ISelAddressMode &AM) {
   int64_t Val = AM.Disp + Offset;
   CodeModel::Model M = TM.getCodeModel();
-  if (!Subtarget->is64Bit() ||
-      X86::isOffsetSuitableForCodeModel(Val, M,
-                                        AM.hasSymbolicDisplacement())) {
-    AM.Disp = Val;
-    return false;
+  if (Subtarget->is64Bit()) {
+    if (!X86::isOffsetSuitableForCodeModel(Val, M,
+                                           AM.hasSymbolicDisplacement()))
+      return true;
+    // In addition to the checks required for a register base, check that
+    // we do not try to use an unsafe Disp with a frame index.
+    if (AM.BaseType == X86ISelAddressMode::FrameIndexBase &&
+        !isDispSafeForFrameIndex(Val))
+      return true;
   }
-  return true;
+  AM.Disp = Val;
+  return false;
+
 }
 
 bool X86DAGToDAGISel::MatchLoadInAddress(LoadSDNode *N, X86ISelAddressMode &AM){
@@ -751,8 +767,9 @@ bool X86DAGToDAGISel::MatchAddressRecursively(SDValue N, X86ISelAddressMode &AM,
     break;
 
   case ISD::FrameIndex:
-    if (AM.BaseType == X86ISelAddressMode::RegBase
-        && AM.Base_Reg.getNode() == 0) {
+    if (AM.BaseType == X86ISelAddressMode::RegBase &&
+        AM.Base_Reg.getNode() == 0 &&
+        (!Subtarget->is64Bit() || isDispSafeForFrameIndex(AM.Disp))) {
       AM.BaseType = X86ISelAddressMode::FrameIndexBase;
       AM.Base_FrameIndex = cast<FrameIndexSDNode>(N)->getIndex();
       return false;
diff --git a/test/CodeGen/X86/2011-07-13-BadFrameIndexDisplacement.ll b/test/CodeGen/X86/2011-07-13-BadFrameIndexDisplacement.ll
new file mode 100644
index 0000000..7632034
--- /dev/null
+++ b/test/CodeGen/X86/2011-07-13-BadFrameIndexDisplacement.ll
@@ -0,0 +1,20 @@
+; RUN: llc -march=x86-64 < %s -disable-fp-elim | FileCheck %s
+
+; This test is checking that we don't crash and we don't incorrectly fold
+; a large displacement and a frame index into a single lea.
+; <rdar://problem/9763308>
+
+declare void @bar([39 x i8]*)
+define i32 @f(i64 %a, i64 %b) nounwind readnone {
+entry:
+  %stack_main = alloca [39 x i8]
+  call void @bar([39 x i8]* %stack_main)
+  %tmp6 = add i64 %a, -2147483647
+  %.sum = add i64 %tmp6, %b
+  %tmp8 = getelementptr inbounds [39 x i8]* %stack_main, i64 0, i64 %.sum
+  %tmp9 = load i8* %tmp8, align 1
+  %tmp10 = sext i8 %tmp9 to i32
+  ret i32 %tmp10
+}
+; CHECK: f:
+; CHECK: movsbl	-2147483647