From 2a01946de4e510e42691f8dc5e7331fcecb67432 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Wed, 13 Jul 2011 21:29:53 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@135084 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/X86/X86ISelDAGToDAG.cpp | 33 ++++++++++++++++------ .../X86/2011-07-13-BadFrameIndexDisplacement.ll | 20 +++++++++++++ 2 files changed, 45 insertions(+), 8 deletions(-) create mode 100644 test/CodeGen/X86/2011-07-13-BadFrameIndexDisplacement.ll diff --git a/lib/Target/X86/X86ISelDAGToDAG.cpp b/lib/Target/X86/X86ISelDAGToDAG.cpp index b617e976314..2b0f283bec7 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(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 00000000000..7632034e13b --- /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. +; + +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 -- 2.11.0