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

[David Majnemer]

On Thu, May 14, 2015 at 9:29 PM, Justin Bogner <mail at justinbogner.com>
wrote:

> David Majnemer <david.majnemer at gmail.com> writes:
> > On Tue, Apr 28, 2015 at 7:56 PM, Justin Bogner <mail at justinbogner.com>
> wrote:
> >>>> 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.
>
> Interestingly, setting this to 0 passes all of the tests, but setting it
> to INT64_MIN+1 does not. That's pretty scary - I guess we're using this
> value somewhere, somehow...
>

I'm pretty sure that's because INT64_MIN+1 isn't a multiple of SlotSize.  I
ended up committing r237474 which just uses 0 for the offset.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150515/5747ab54/attachment.html>