llvm dev - Nov 2017 - X86 llvm.frameaddress/returnaddress

Hello,

The llvm.frameaddress and llvm.returnaddress intrinsics seem broken on
64 bit X86 with -fomit-frame-pointer when given an argument greater than
zero.

We recently found on OpenBSD that code calling __builtin_return_address
and __builtin_frame_address with arguments greater than zero can
segfault if RBP has been allocated as a general purpose register
somewhere up the stack. Specifically, we found that chromium calls
backtrace(), and the library we were using to implement that function
was just using __builtin_return_address and __builtin_frame_address to
walk the stack backwards. The compiled code ended up something like
this:

     5bf:       48 8b 4d 00             mov    0x0(%rbp),%rcx
     5c3:       48 8b 09                mov    (%rcx),%rcx
     5c6:       48 8b 09                mov    (%rcx),%rcx
     5c9:       48 8b 09                mov    (%rcx),%rcx
     5cc:       48 83 39 00             cmpq   $0x0,(%rcx)

This works fine when %rbp is always holding a frame pointer, but is
unpredictable when it does not. In our case, C++ code in Chromium had
used %rbp for data, so segfaulted when dereferencing.

I see in the implementation of X86TargetLowering::LowerFRAMEADDR that
there is some special handling when
MF.getTarget().getMCAsmInfo()->usesWindowsCFI()
is true, and it is commented to the effect that depth > 0 makes no sense
when unwind info is needed. Should the same logic be applied when given
-fomit-frame-pointer, or just in general on 64 bit X86 where unwind
tables are required anyway? I realize that the documentation for the
llvm.frameaddress and llvm.returnaddress intrinsics are explicit that
the return value may be wrong when the argument is > 0, but in cases
where we know it will be unreliable / unstable, it might be useful to
limit the output to something that doesn't possibly cause runtime
failures?

Todd

We can defensively warn in the frontend when we see
__builtin_frame/return_address(n > 0) when we know frame pointers are
absent, but in general, without knowledge of all the callers, these
intrinsics can fault without warning. That's just a risk you have to be
aware of when calling them, and I thought it was called out in
documentation.

On Fri, Nov 17, 2017 at 7:44 AM, Todd Mortimer via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hello,
>
> The llvm.frameaddress and llvm.returnaddress intrinsics seem broken on
> 64 bit X86 with -fomit-frame-pointer when given an argument greater than
> zero.
>
> We recently found on OpenBSD that code calling __builtin_return_address
> and __builtin_frame_address with arguments greater than zero can
> segfault if RBP has been allocated as a general purpose register
> somewhere up the stack. Specifically, we found that chromium calls
> backtrace(), and the library we were using to implement that function
> was just using __builtin_return_address and __builtin_frame_address to
> walk the stack backwards. The compiled code ended up something like
> this:
>
>      5bf:       48 8b 4d 00             mov    0x0(%rbp),%rcx
>      5c3:       48 8b 09                mov    (%rcx),%rcx
>      5c6:       48 8b 09                mov    (%rcx),%rcx
>      5c9:       48 8b 09                mov    (%rcx),%rcx
>      5cc:       48 83 39 00             cmpq   $0x0,(%rcx)
>
> This works fine when %rbp is always holding a frame pointer, but is
> unpredictable when it does not. In our case, C++ code in Chromium had
> used %rbp for data, so segfaulted when dereferencing.
>
> I see in the implementation of X86TargetLowering::LowerFRAMEADDR that
> there is some special handling when MF.getTarget().getMCAsmInfo()-
> >usesWindowsCFI()
> is true, and it is commented to the effect that depth > 0 makes no sense
> when unwind info is needed. Should the same logic be applied when given
> -fomit-frame-pointer, or just in general on 64 bit X86 where unwind
> tables are required anyway? I realize that the documentation for the
> llvm.frameaddress and llvm.returnaddress intrinsics are explicit that
> the return value may be wrong when the argument is > 0, but in cases
> where we know it will be unreliable / unstable, it might be useful to
> limit the output to something that doesn't possibly cause runtime
> failures?
>
> Todd
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171117/7a3842b2/attachment.html>