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

Justin Bogner mail at justinbogner.com
Mon Apr 27 15:30:53 PDT 2015


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?

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