[llvm] r228748 - X86: Make @llvm.frameaddress work correctly with Windows unwind codes

David Majnemer david.majnemer at gmail.com
Tue Apr 28 17:04:34 PDT 2015


On Tue, Apr 28, 2015 at 7:56 PM, Justin Bogner <mail at justinbogner.com>
wrote:

> 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?
>

Yep, pretty much.


>
> >
> >     > +      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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150428/87c196a2/attachment.html>


More information about the llvm-commits mailing list