[compiler-rt] r277874 - Fix two tests in Win64 ASan

[David Majnemer via llvm-commits]

On Mon, Aug 8, 2016 at 12:51 PM, David Majnemer <david.majnemer at gmail.com>
wrote:

>
>
> On Mon, Aug 8, 2016 at 10:37 AM, Reid Kleckner <rnk at google.com> wrote:
>
>> On Fri, Aug 5, 2016 at 6:53 PM, David Majnemer <david.majnemer at gmail.com>
>> wrote:
>>
>>> On Fri, Aug 5, 2016 at 2:47 PM, Reid Kleckner via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> Author: rnk
>>>> Date: Fri Aug  5 16:47:46 2016
>>>> New Revision: 277874
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=277874&view=rev
>>>> Log:
>>>> Fix two tests in Win64 ASan
>>>>
>>>> Go back to intercepting kernel32!RaiseException, and only go for
>>>> ntdll!RtlRaiseException if that fails. Fixes throw_and_catch.cc test.
>>>>
>>>> Work around an issue in LLVM's win64 epilogues. We end up with an
>>>> epilogue that looks like this, and it drives the Win64 unwinder crazy
>>>> until stack overflow:
>>>>         call    ill_cc!__asan_handle_no_return
>>>>         xor     eax,eax
>>>>         add     rsp,40h // epilogue starts
>>>
>>>         pop     rbp     // CSR
>>>>         ud2             // Trap here
>>>>         ret             // Ret?
>>>>         nop     word ptr [rax+rax]
>>>>         sub     rsp,28h // Next function
>>>>
>>>
>>>
>>> Interesting.  So we started the epilogue but didn't finish it.  Happen
>>> to have some IR or C lying around?  I'm curious exactly when we decided
>>> that a ud2 between the pop and the ret was a smart move...
>>>
>>
>> It's pretty easy to convince LLVM to put the ud2 there, but it's hard to
>> build a test case that will cause the Win64 unwinder to fail in the same
>> way that I observed in this test without the extra conditional control flow
>> that I added.
>>
>> $ cat t.cpp
>> void foo();
>> int main() { foo(); __builtin_trap(); }
>>
>> $ clang -S --target=x86_64-windows t.cpp  -fno-omit-frame-pointer -o -
>> ...
>>         callq   "?foo@@YAXXZ"
>>         xorl    %eax, %eax
>>         addq    $32, %rsp
>>         popq    %rbp
>>         ud2
>>         retq
>> ...
>>
>> I think the issue is that the faulting PC does not appear to be within a
>> standard epilogue, so the unwinder re-emulates the add 32 and pop rbp.
>>
>
> Hrm, the CoreCLR copy of the x64 unwinder *should* handle this case:
> https://github.com/dotnet/coreclr/blob/master/src/unwinder/amd64/unwinder_
> amd64.cpp#L1304
>
> Our retq is encoded as 0xc3 which is their RET_OP.
>
> Using the same IR but forcing us out of FastISel leaves us with:
> callq "?foo@@YAXXZ"
> ud2
> xorl %eax, %eax
> addq $32, %rsp
> popq %rbp
> retq
>
> The difference is caused by a different selection of terminator in
> emitEpligoue (FastISel uses the TRAP while SDAG uses the RETQ).
>
> From reading the x64 ABI docs, I cannot find where we strongly violated a
> constraint.  My only guess is that it doesn't like us partially unwinding a
> frame function before performing the trap (something along the lines of the
> unwinder _expecting_ to see an SP adjustment but seeing nothing but the
> ret...).
>


After talking with Gor, the problem has been made more clear.  We are in a
frame function but we, not the unwinder, restored the FP.  This would be OK
if we were in an epilogue sequence but we are not because the faulting
instruction is a ud2 which makes the unwinder believe that we are in the
body of the function.

All of this started because we lowered the __builtin_trap to a TRAP which
is marked isTerminator.  emitEpilogue believes that this is our return
opcode because it is the first terminator in the MBB.  The easiest way of
fixing this, IMO, is to insert a NOP after the TRAP.  This will stop it
from looking like a terminator in the epilogue code...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160808/d62d4a3c/attachment.html>