[llvm] r228748 - X86: Make @llvm.frameaddress work correctly with Windows unwind codes
Justin Bogner
mail at justinbogner.com
Tue Apr 28 16:56:49 PDT 2015
David Majnemer <david.majnemer at gmail.com> writes:
> On Mon, Apr 27, 2015 at 6:30 PM, Justin Bogner <mail at justinbogner.com> wrote:
>
> David Majnemer <david.majnemer at gmail.com> writes:
> > Author: majnemer
> > Date: Tue Feb 10 15:22:05 2015
> > New Revision: 228748
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=228748&view=rev
> > Log:
> > X86: Make @llvm.frameaddress work correctly with Windows unwind codes
> >
> > Simply loading or storing the frame pointer is not sufficient for
> > Windows targets. Instead, create a synthetic frame object that we will
> > lower later. References to this synthetic object will be replaced with
> > the correct reference to the frame address.
> >
> > Modified:
> > llvm/trunk/lib/Target/X86/X86FrameLowering.cpp
> > llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> > llvm/trunk/lib/Target/X86/X86MachineFunctionInfo.h
> > llvm/trunk/test/CodeGen/X86/frameallocate.ll
> >
> > Modified: llvm/trunk/lib/Target/X86/X86FrameLowering.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/
> X86FrameLowering.cpp?rev=228748&r1=228747&r2=228748&view=diff
> > ========================================================================
> ======
> > --- llvm/trunk/lib/Target/X86/X86FrameLowering.cpp (original)
> > +++ llvm/trunk/lib/Target/X86/X86FrameLowering.cpp Tue Feb 10 15:22:05
> 2015
> > @@ -1242,6 +1242,9 @@ int X86FrameLowering::getFrameIndexOffse
> > NumBytes = FrameSize - CSSize;
> > }
> > uint64_t SEHFrameOffset = calculateSetFPREG(NumBytes);
> > + if (FI && FI == X86FI->getFAIndex())
> > + return -SEHFrameOffset;
> > +
> > // FPDelta is the offset from the "traditional" FP location of the
> old base
> > // pointer followed by return address and the location required by
> the
> > // restricted Win64 prologue.
> >
> > Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/
> X86ISelLowering.cpp?rev=228748&r1=228747&r2=228748&view=diff
> > ========================================================================
> ======
> > --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
> > +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Tue Feb 10 15:22:05
> 2015
> > @@ -17953,15 +17953,33 @@ SDValue X86TargetLowering::LowerRETURNAD
> > }
> >
> > SDValue X86TargetLowering::LowerFRAMEADDR(SDValue Op, SelectionDAG &
> DAG) const {
> > - MachineFrameInfo *MFI = DAG.getMachineFunction().getFrameInfo();
> > + MachineFunction &MF = DAG.getMachineFunction();
> > + MachineFrameInfo *MFI = MF.getFrameInfo();
> > + X86MachineFunctionInfo *FuncInfo = MF.getInfo<X86MachineFunctionInfo>
> ();
> > + const X86RegisterInfo *RegInfo = Subtarget->getRegisterInfo();
> > + EVT VT = Op.getValueType();
> > +
> > MFI->setFrameAddressIsTaken(true);
> >
> > - EVT VT = Op.getValueType();
> > + if (MF.getTarget().getMCAsmInfo()->usesWindowsCFI()) {
> > + // Depth > 0 makes no sense on targets which use Windows unwind
> codes. It
> > + // is not possible to crawl up the stack without looking at the
> unwind codes
> > + // simultaneously.
> > + int FrameAddrIndex = FuncInfo->getFAIndex();
> > + if (!FrameAddrIndex) {
> > + // Set up a frame object for the return address.
> > + unsigned SlotSize = RegInfo->getSlotSize();
> > + FrameAddrIndex = MF.getFrameInfo()->CreateFixedObject(
> > + SlotSize, /*Offset=*/INT64_MIN, /*IsImmutable=*/false);
>
> Sorry to dig up an old thread, but I was playing around with ubsan and
> it looks like this causes some undefined behaviour. I guess we set the
> Offset to INT64_MIN here as a dummy value, and it isn't intended to be
> used, but later, in PrologEpilogInserter.cpp, we hit this logic in
> PEI::calculateFrameObjectOffsets:
>
> if (StackGrowsDown) {
> // The maximum distance from the stack pointer is at lower address of
> // the object -- which is given by offset. For down growing stack
> // the offset is negative, so we negate the offset to get the
> distance.
> FixedOff = -MFI->getObjectOffset(i);
>
> Here, when MFI->getObjectOffset returns INT64_MIN and we negate it, we
> can't represent the result and thus hit undefined behaviour. It looks
> like the effect is mostly harmless in practice, but...
>
> I guess that 0 is a better dummy value here - all tests continue to pass
> if I change INT64_MIN to 0, anyway.
>
> WDYT?
>
> Could we do INT64_MIN+1?
I don't see why not. Is there any particular reason that's better than
0? I guess just to make it obvious that it's special?
>
> > + FuncInfo->setFAIndex(FrameAddrIndex);
> > + }
> > + return DAG.getFrameIndex(FrameAddrIndex, VT);
> > + }
> > +
> > + unsigned FrameReg =
> > + RegInfo->getPtrSizedFrameRegister(DAG.getMachineFunction());
> > SDLoc dl(Op); // FIXME probably not meaningful
> > unsigned Depth = cast<ConstantSDNode>(Op.getOperand(0))->getZExtValue
> ();
> > - const X86RegisterInfo *RegInfo = Subtarget->getRegisterInfo();
> > - unsigned FrameReg = RegInfo->getPtrSizedFrameRegister(
> > - DAG.getMachineFunction());
> > assert(((FrameReg == X86::RBP && VT == MVT::i64) ||
> > (FrameReg == X86::EBP && VT == MVT::i32)) &&
> > "Invalid Frame Register!");
> >
> > Modified: llvm/trunk/lib/Target/X86/X86MachineFunctionInfo.h
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/
> X86MachineFunctionInfo.h?rev=228748&r1=228747&r2=228748&view=diff
> > ========================================================================
> ======
> > --- llvm/trunk/lib/Target/X86/X86MachineFunctionInfo.h (original)
> > +++ llvm/trunk/lib/Target/X86/X86MachineFunctionInfo.h Tue Feb 10
> 15:22:05 2015
> > @@ -50,6 +50,9 @@ class X86MachineFunctionInfo : public Ma
> > /// ReturnAddrIndex - FrameIndex for return slot.
> > int ReturnAddrIndex;
> >
> > + /// \brief FrameIndex for return slot.
> > + int FrameAddrIndex;
> > +
> > /// TailCallReturnAddrDelta - The number of bytes by which return
> address
> > /// stack slot is moved as the result of tail call optimization.
> > int TailCallReturnAddrDelta;
> > @@ -92,6 +95,7 @@ public:
> > CalleeSavedFrameSize(0),
> > BytesToPopOnReturn(0),
> > ReturnAddrIndex(0),
> > + FrameAddrIndex(0),
> > TailCallReturnAddrDelta(0),
> > SRetReturnReg(0),
> > GlobalBaseReg(0),
> > @@ -109,6 +113,7 @@ public:
> > CalleeSavedFrameSize(0),
> > BytesToPopOnReturn(0),
> > ReturnAddrIndex(0),
> > + FrameAddrIndex(0),
> > TailCallReturnAddrDelta(0),
> > SRetReturnReg(0),
> > GlobalBaseReg(0),
> > @@ -139,6 +144,9 @@ public:
> > int getRAIndex() const { return ReturnAddrIndex; }
> > void setRAIndex(int Index) { ReturnAddrIndex = Index; }
> >
> > + int getFAIndex() const { return FrameAddrIndex; }
> > + void setFAIndex(int Index) { FrameAddrIndex = Index; }
> > +
> > int getTCReturnAddrDelta() const { return TailCallReturnAddrDelta; }
> > void setTCReturnAddrDelta(int delta) {TailCallReturnAddrDelta =
> delta;}
> >
> >
> > Modified: llvm/trunk/test/CodeGen/X86/frameallocate.ll
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/
> frameallocate.ll?rev=228748&r1=228747&r2=228748&view=diff
> > ========================================================================
> ======
> > --- llvm/trunk/test/CodeGen/X86/frameallocate.ll (original)
> > +++ llvm/trunk/test/CodeGen/X86/frameallocate.ll Tue Feb 10 15:22:05
> 2015
> > @@ -32,8 +32,12 @@ define void @alloc_func(i32* %s, i32* %d
> > }
> >
> > ; CHECK-LABEL: alloc_func:
> > +; CHECK: subq $48, %rsp
> > +; CHECK: .seh_stackalloc 48
> > +; CHECK: leaq 48(%rsp), %rbp
> > +; CHECK: .seh_setframe 5, 48
> > ; CHECK: .Lframeallocation_alloc_func = -[[offs:[0-9]+]]
> > ; CHECK: movl $42, -[[offs]](%rbp)
> > -; CHECK: movq %rbp, %rcx
> > +; CHECK: leaq -48(%rbp), %rcx
> > ; CHECK: callq print_framealloc_from_fp
> > ; CHECK: retq
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list