From 1f0a44e6626f8ecb9814818f1c6f9738b447737f Mon Sep 17 00:00:00 2001 From: Jay Foad Date: Mon, 1 Dec 2014 09:42:32 +0000 Subject: [PATCH] [PowerPC] Fix unwind info with dynamic stack realignment Summary: PowerPC DWARF unwind info defined CFA as SP + offset even in a function where the stack had been dynamically realigned. This clearly doesn't work because the offset from SP to CFA is not a constant. Fix it by defining CFA as BP instead. This was causing the AddressSanitizer null_deref test to fail 50% of the time, depending on whether SP happened to be 32-byte aligned on entry to a particular function or not. Reviewers: willschm, uweigand, hfinkel Reviewed By: hfinkel Subscribers: llvm-commits Differential Revision: http://reviews.llvm.org/D6410 llvm-svn: 222996 --- llvm/lib/Target/PowerPC/PPCFrameLowering.cpp | 39 +++++++++++++++++++--------- llvm/test/CodeGen/PowerPC/stack-realign.ll | 8 ++++++ 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp b/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp index dc87a6c..a81131b 100644 --- a/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp +++ b/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp @@ -505,7 +505,7 @@ void PPCFrameLowering::emitPrologue(MachineFunction &MF) const { MachineModuleInfo &MMI = MF.getMMI(); const MCRegisterInfo *MRI = MMI.getContext().getRegisterInfo(); DebugLoc dl; - bool needsFrameMoves = MMI.hasDebugInfo() || + bool needsCFI = MMI.hasDebugInfo() || MF.getFunction()->needsUnwindTableEntry(); bool isPIC = MF.getTarget().getRelocationModel() == Reloc::PIC_; @@ -726,17 +726,28 @@ void PPCFrameLowering::emitPrologue(MachineFunction &MF) const { .addReg(ScratchReg); } - // Add the "machine moves" for the instructions we generated above, but in - // reverse order. - if (needsFrameMoves) { - // Show update of SP. - assert(NegFrameSize); - unsigned CFIIndex = MMI.addFrameInst( - MCCFIInstruction::createDefCfaOffset(nullptr, NegFrameSize)); + // Add Call Frame Information for the instructions we generated above. + if (needsCFI) { + unsigned CFIIndex; + + if (HasBP) { + // Define CFA in terms of BP. Do this in preference to using FP/SP, + // because if the stack needed aligning then CFA won't be at a fixed + // offset from FP/SP. + unsigned Reg = MRI->getDwarfRegNum(BPReg, true); + CFIIndex = MMI.addFrameInst( + MCCFIInstruction::createDefCfaRegister(nullptr, Reg)); + } else { + // Adjust the definition of CFA to account for the change in SP. + assert(NegFrameSize); + CFIIndex = MMI.addFrameInst( + MCCFIInstruction::createDefCfaOffset(nullptr, NegFrameSize)); + } BuildMI(MBB, MBBI, dl, TII.get(TargetOpcode::CFI_INSTRUCTION)) .addCFIIndex(CFIIndex); if (HasFP) { + // Describe where FP was saved, at a fixed offset from CFA. unsigned Reg = MRI->getDwarfRegNum(FPReg, true); CFIIndex = MMI.addFrameInst( MCCFIInstruction::createOffset(nullptr, Reg, FPOffset)); @@ -745,6 +756,7 @@ void PPCFrameLowering::emitPrologue(MachineFunction &MF) const { } if (HasBP) { + // Describe where BP was saved, at a fixed offset from CFA. unsigned Reg = MRI->getDwarfRegNum(BPReg, true); CFIIndex = MMI.addFrameInst( MCCFIInstruction::createOffset(nullptr, Reg, BPOffset)); @@ -753,6 +765,7 @@ void PPCFrameLowering::emitPrologue(MachineFunction &MF) const { } if (MustSaveLR) { + // Describe where LR was saved, at a fixed offset from CFA. unsigned Reg = MRI->getDwarfRegNum(LRReg, true); CFIIndex = MMI.addFrameInst( MCCFIInstruction::createOffset(nullptr, Reg, LROffset)); @@ -767,8 +780,9 @@ void PPCFrameLowering::emitPrologue(MachineFunction &MF) const { .addReg(SPReg) .addReg(SPReg); - if (needsFrameMoves) { - // Mark effective beginning of when frame pointer is ready. + if (!HasBP && needsCFI) { + // Change the definition of CFA from SP+offset to FP+offset, because SP + // will change at every alloca. unsigned Reg = MRI->getDwarfRegNum(FPReg, true); unsigned CFIIndex = MMI.addFrameInst( MCCFIInstruction::createDefCfaRegister(nullptr, Reg)); @@ -778,8 +792,9 @@ void PPCFrameLowering::emitPrologue(MachineFunction &MF) const { } } - if (needsFrameMoves) { - // Add callee saved registers to move list. + if (needsCFI) { + // Describe where callee saved registers were saved, at fixed offsets from + // CFA. const std::vector &CSI = MFI->getCalleeSavedInfo(); for (unsigned I = 0, E = CSI.size(); I != E; ++I) { unsigned Reg = CSI[I].getReg(); diff --git a/llvm/test/CodeGen/PowerPC/stack-realign.ll b/llvm/test/CodeGen/PowerPC/stack-realign.ll index a59fceb..762f50a 100644 --- a/llvm/test/CodeGen/PowerPC/stack-realign.ll +++ b/llvm/test/CodeGen/PowerPC/stack-realign.ll @@ -37,6 +37,7 @@ entry: ; CHECK-DAG: subfic 0, [[REG]], -160 ; CHECK: stdux 1, 1, 0 +; CHECK: .cfi_def_cfa_register r30 ; CHECK: .cfi_offset r30, -16 ; CHECK: .cfi_offset lr, 16 @@ -59,6 +60,7 @@ entry: ; CHECK-FP-DAG: subfic 0, [[REG]], -160 ; CHECK-FP: stdux 1, 1, 0 +; CHECK-FP: .cfi_def_cfa_register r30 ; CHECK-FP: .cfi_offset r31, -8 ; CHECK-FP: .cfi_offset r30, -16 ; CHECK-FP: .cfi_offset lr, 16 @@ -120,6 +122,8 @@ entry: ; CHECK-DAG: subfc 0, [[REG3]], [[REG2]] ; CHECK: stdux 1, 1, 0 +; CHECK: .cfi_def_cfa_register r30 + ; CHECK: blr ; CHECK-32-LABEL: @hoo @@ -178,6 +182,8 @@ entry: ; CHECK-DAG: subfic 0, [[REG]], -192 ; CHECK: stdux 1, 1, 0 +; CHECK: .cfi_def_cfa_register r30 + ; CHECK: stfd 30, -16(30) ; CHECK: blr @@ -193,6 +199,8 @@ entry: ; CHECK-FP-DAG: subfic 0, [[REG]], -192 ; CHECK-FP: stdux 1, 1, 0 +; CHECK-FP: .cfi_def_cfa_register r30 + ; CHECK-FP: stfd 30, -16(30) ; CHECK-FP: blr -- 2.7.4