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