<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Apr 28, 2015 at 7:56 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">David Majnemer <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>> writes:<br>
> On Mon, Apr 27, 2015 at 6:30 PM, Justin Bogner <<a href="mailto:mail@justinbogner.com">mail@justinbogner.com</a>> wrote:<br>
><br>
> 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/" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/</a><br>
> X86FrameLowering.cpp?rev=228748&r1=228747&r2=228748&view=diff<br>
> > ========================================================================<br>
> ======<br>
> > --- llvm/trunk/lib/Target/X86/X86FrameLowering.cpp (original)<br>
> > +++ llvm/trunk/lib/Target/X86/X86FrameLowering.cpp Tue Feb 10 15:22:05<br>
> 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<br>
> old base<br>
> > // pointer followed by return address and the location required by<br>
> 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/" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/</a><br>
> X86ISelLowering.cpp?rev=228748&r1=228747&r2=228748&view=diff<br>
> > ========================================================================<br>
> ======<br>
> > --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)<br>
> > +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Tue Feb 10 15:22:05<br>
> 2015<br>
> > @@ -17953,15 +17953,33 @@ SDValue X86TargetLowering::LowerRETURNAD<br>
> > }<br>
> ><br>
> > SDValue X86TargetLowering::LowerFRAMEADDR(SDValue Op, SelectionDAG &<br>
> 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>
> ();<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<br>
> codes. It<br>
> > + // is not possible to crawl up the stack without looking at the<br>
> 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>
> 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<br>
> 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>
><br>
> Could we do INT64_MIN+1?<br>
<br>
</div></div>I don't see why not. Is there any particular reason that's better than<br>
0? I guess just to make it obvious that it's special?<br></blockquote><div><br></div><div>Yep, pretty much.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
><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>
> ();<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/" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/</a><br>
> X86MachineFunctionInfo.h?rev=228748&r1=228747&r2=228748&view=diff<br>
> > ========================================================================<br>
> ======<br>
> > --- llvm/trunk/lib/Target/X86/X86MachineFunctionInfo.h (original)<br>
> > +++ llvm/trunk/lib/Target/X86/X86MachineFunctionInfo.h Tue Feb 10<br>
> 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<br>
> 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 =<br>
> 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/" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/</a><br>
> frameallocate.ll?rev=228748&r1=228747&r2=228748&view=diff<br>
> > ========================================================================<br>
> ======<br>
> > --- llvm/trunk/test/CodeGen/X86/frameallocate.ll (original)<br>
> > +++ llvm/trunk/test/CodeGen/X86/frameallocate.ll Tue Feb 10 15:22:05<br>
> 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>