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

[David Majnemer via llvm-commits]

Fixed in r278144.

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

>
>
> On Mon, Aug 8, 2016 at 2:46 PM, David Majnemer <david.majnemer at gmail.com>
> wrote:
>
>>
>>
>> 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/unwi
>>> nder/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...
>>
>
> On second thought, it would be even better to introduce another trap
> instruction which is not a terminator.  This way we can preserve the
> invariant that terminators are grouped at the end of MBBs.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160809/99da514c/attachment.html>