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

[David Majnemer via llvm-commits]

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/
>> 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...
>

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/20160808/b9801f3f/attachment.html>