[llvm] r275103 - [X86] Fix tailcall return address clobber bug.
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 11 14:03:03 PDT 2016
Author: qcolombet
Date: Mon Jul 11 16:03:03 2016
New Revision: 275103
URL: http://llvm.org/viewvc/llvm-project?rev=275103&view=rev
Log:
[X86] Fix tailcall return address clobber bug.
This bug (llvm.org/PR28124) was introduced by r237977, which refactored
the tail call sequence to be generated in two passes instead of one.
Unfortunately, the stack adjustment produced by the first pass was not
recognized by X86FrameLowering::mergeSPUpdates() in all cases, causing
code such as the following, which clobbers the return address, to be
generated:
popl %edi
popl %edi
pushl %eax
jmp tailcallee # TAILCALL
To fix the problem, the entire stack adjustment is performed in
X86ExpandPseudo::ExpandMI() for tail calls.
Patch by Magnus Lång <margnus1 at gmail.com>
Differential Revision: http://reviews.llvm.org/D21325
Modified:
llvm/trunk/lib/Target/X86/X86ExpandPseudo.cpp
llvm/trunk/lib/Target/X86/X86FrameLowering.cpp
llvm/trunk/test/CodeGen/X86/hipe-cc.ll
llvm/trunk/test/CodeGen/X86/hipe-cc64.ll
Modified: llvm/trunk/lib/Target/X86/X86ExpandPseudo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ExpandPseudo.cpp?rev=275103&r1=275102&r2=275103&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86ExpandPseudo.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86ExpandPseudo.cpp Mon Jul 11 16:03:03 2016
@@ -44,6 +44,7 @@ public:
const X86Subtarget *STI;
const X86InstrInfo *TII;
const X86RegisterInfo *TRI;
+ const X86MachineFunctionInfo *X86FI;
const X86FrameLowering *X86FL;
bool runOnMachineFunction(MachineFunction &Fn) override;
@@ -88,11 +89,18 @@ bool X86ExpandPseudo::ExpandMI(MachineBa
// Adjust stack pointer.
int StackAdj = StackAdjust.getImm();
+ int MaxTCDelta = X86FI->getTCReturnAddrDelta();
+ int Offset = 0;
+ assert(MaxTCDelta <= 0 && "MaxTCDelta should never be positive");
+
+ // Incoporate the retaddr area.
+ Offset = StackAdj-MaxTCDelta;
+ assert(Offset >= 0 && "Offset should never be negative");
- if (StackAdj) {
+ if (Offset) {
// Check for possible merge with preceding ADD instruction.
- StackAdj += X86FL->mergeSPUpdates(MBB, MBBI, true);
- X86FL->emitSPUpdate(MBB, MBBI, StackAdj, /*InEpilogue=*/true);
+ Offset += X86FL->mergeSPUpdates(MBB, MBBI, true);
+ X86FL->emitSPUpdate(MBB, MBBI, Offset, /*InEpilogue=*/true);
}
// Jump to label or value in register.
@@ -247,6 +255,7 @@ bool X86ExpandPseudo::runOnMachineFuncti
STI = &static_cast<const X86Subtarget &>(MF.getSubtarget());
TII = STI->getInstrInfo();
TRI = STI->getRegisterInfo();
+ X86FI = MF.getInfo<X86MachineFunctionInfo>();
X86FL = STI->getFrameLowering();
bool Modified = false;
Modified: llvm/trunk/lib/Target/X86/X86FrameLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FrameLowering.cpp?rev=275103&r1=275102&r2=275103&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86FrameLowering.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86FrameLowering.cpp Mon Jul 11 16:03:03 2016
@@ -1467,11 +1467,19 @@ X86FrameLowering::getWinEHFuncletFrameSi
return FrameSizeMinusRBP - CSSize;
}
+static bool isTailCallOpcode(unsigned Opc) {
+ return Opc == X86::TCRETURNri || Opc == X86::TCRETURNdi ||
+ Opc == X86::TCRETURNmi ||
+ Opc == X86::TCRETURNri64 || Opc == X86::TCRETURNdi64 ||
+ Opc == X86::TCRETURNmi64;
+}
+
void X86FrameLowering::emitEpilogue(MachineFunction &MF,
MachineBasicBlock &MBB) const {
const MachineFrameInfo *MFI = MF.getFrameInfo();
X86MachineFunctionInfo *X86FI = MF.getInfo<X86MachineFunctionInfo>();
MachineBasicBlock::iterator MBBI = MBB.getFirstTerminator();
+ unsigned RetOpcode = MBBI->getOpcode();
DebugLoc DL;
if (MBBI != MBB.end())
DL = MBBI->getDebugLoc();
@@ -1620,15 +1628,17 @@ void X86FrameLowering::emitEpilogue(Mach
if (NeedsWinCFI)
BuildMI(MBB, MBBI, DL, TII.get(X86::SEH_Epilogue));
- // Add the return addr area delta back since we are not tail calling.
- int Offset = -1 * X86FI->getTCReturnAddrDelta();
- assert(Offset >= 0 && "TCDelta should never be positive");
- if (Offset) {
- MBBI = MBB.getFirstTerminator();
-
- // Check for possible merge with preceding ADD instruction.
- Offset += mergeSPUpdates(MBB, MBBI, true);
- emitSPUpdate(MBB, MBBI, Offset, /*InEpilogue=*/true);
+ if (!isTailCallOpcode(RetOpcode)) {
+ // Add the return addr area delta back since we are not tail calling.
+ int Offset = -1 * X86FI->getTCReturnAddrDelta();
+ assert(Offset >= 0 && "TCDelta should never be positive");
+ if (Offset) {
+ MBBI = MBB.getFirstTerminator();
+
+ // Check for possible merge with preceding ADD instruction.
+ Offset += mergeSPUpdates(MBB, MBBI, true);
+ emitSPUpdate(MBB, MBBI, Offset, /*InEpilogue=*/true);
+ }
}
}
Modified: llvm/trunk/test/CodeGen/X86/hipe-cc.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/hipe-cc.ll?rev=275103&r1=275102&r2=275103&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/hipe-cc.ll (original)
+++ llvm/trunk/test/CodeGen/X86/hipe-cc.ll Mon Jul 11 16:03:03 2016
@@ -73,9 +73,23 @@ define cc 11 void @baz() nounwind {
ret void
}
+; Sanity-check the tail call sequence. Number of arguments was chosen as to
+; expose a bug where the tail call sequence clobbered the stack.
+define cc 11 { i32, i32, i32 } @tailcaller(i32 %hp, i32 %p) nounwind {
+ ; CHECK: movl $15, %eax
+ ; CHECK-NEXT: movl $31, %edx
+ ; CHECK-NEXT: movl $47, %ecx
+ ; CHECK-NEXT: popl %edi
+ ; CHECK-NEXT: jmp tailcallee
+ %ret = tail call cc11 { i32, i32, i32 } @tailcallee(i32 %hp, i32 %p, i32 15,
+ i32 31, i32 47, i32 63) nounwind
+ ret { i32, i32, i32 } %ret
+}
+
!hipe.literals = !{ !0, !1, !2 }
!0 = !{ !"P_NSP_LIMIT", i32 84 }
!1 = !{ !"X86_LEAF_WORDS", i32 24 }
!2 = !{ !"AMD64_LEAF_WORDS", i32 24 }
@clos = external constant i32
declare cc 11 void @bar(i32, i32, i32, i32, i32)
+declare cc 11 { i32, i32, i32 } @tailcallee(i32, i32, i32, i32, i32, i32)
Modified: llvm/trunk/test/CodeGen/X86/hipe-cc64.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/hipe-cc64.ll?rev=275103&r1=275102&r2=275103&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/hipe-cc64.ll (original)
+++ llvm/trunk/test/CodeGen/X86/hipe-cc64.ll Mon Jul 11 16:03:03 2016
@@ -83,9 +83,24 @@ define cc 11 void @baz() nounwind {
ret void
}
+; Sanity-check the tail call sequence. Number of arguments was chosen as to
+; expose a bug where the tail call sequence clobbered the stack.
+define cc 11 { i64, i64, i64 } @tailcaller(i64 %hp, i64 %p) #0 {
+ ; CHECK: movl $15, %esi
+ ; CHECK-NEXT: movl $31, %edx
+ ; CHECK-NEXT: movl $47, %ecx
+ ; CHECK-NEXT: movl $63, %r8d
+ ; CHECK-NEXT: popq %rax
+ ; CHECK-NEXT: jmp tailcallee
+ %ret = tail call cc11 { i64, i64, i64 } @tailcallee(i64 %hp, i64 %p, i64 15,
+ i64 31, i64 47, i64 63, i64 79) #1
+ ret { i64, i64, i64 } %ret
+}
+
!hipe.literals = !{ !0, !1, !2 }
!0 = !{ !"P_NSP_LIMIT", i32 160 }
!1 = !{ !"X86_LEAF_WORDS", i32 24 }
!2 = !{ !"AMD64_LEAF_WORDS", i32 24 }
@clos = external constant i64
declare cc 11 void @bar(i64, i64, i64, i64, i64, i64)
+declare cc 11 { i64, i64, i64 } @tailcallee(i64, i64, i64, i64, i64, i64, i64)
More information about the llvm-commits
mailing list