Fixing LLVM_BUILTIN_TRAP on Windows

[Aaron Ballman]

On Mon, Feb 9, 2015 at 5:34 PM, Michael Spencer <bigcheesegs at gmail.com> wrote:
> On Mon, Feb 9, 2015 at 11:49 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> After a quick conversation with Reid on IRC, he recommended adding
>> __assume(false) to the macro to designate the noreturn-ness of it.
>> Updated patch attached.
>>
>> ~Aaron
>
> lgtm.

Thanks! Commit in r228628

~Aaron

>
> - Michael Spencer
>
>>
>> On Mon, Feb 9, 2015 at 2:16 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>> On Mon, Feb 9, 2015 at 1:49 PM, Reid Kleckner <rnk at google.com> wrote:
>>>> How is raising a 0xdeadd0d0 exception code better than an access violation?
>>>
>>> It's considerably more explicit, isn't a hack, and has the extra-added
>>> bonus of not being UB (since we use LLVM_BUILTIN_TRAP from signal
>>> handlers, for instance). I'm not tied to the exception code, but
>>> picking one that's unlikely to be handled by some other exception
>>> filter is a wise choice.
>>>
>>>> The only way I think we could functionally improve over the access violation
>>>> is to have something that's known noreturn (like __builtin_trap) and encodes
>>>> smaller, like ud2a.
>>>
>>> I disagree, and would point out this isn't a functional improvement
>>> but a bug fix. Assuming that accessing 0x11 as a volatile piece of
>>> memory may suffice for platforms without any way to fail quickly to
>>> our abnormal termination handlers, but that doesn't apply on Windows
>>> where RaiseException will definitely result in calling our vectored
>>> exception handler (unlike __fastfail() on Windows 8 and higher), and
>>> won't be UB even from crazy places like signal handlers.
>>>
>>> ~Aaron
>>>
>>>>
>>>> On Mon, Feb 9, 2015 at 9:02 AM, Aaron Ballman <aaron at aaronballman.com>
>>>> wrote:
>>>>>
>>>>> The implementation for LLVM_BUILTIN_TRAP on Windows leaves quite a bit
>>>>> to be desired. Instead of relying on writing to an invalid volatile
>>>>> pointer, which is hoped to result in raising an exception we can catch
>>>>> with the vectored exception handler, simply raise an exception
>>>>> manually.
>>>>>
>>>>> To prevent having to #include a Win32 header from Compiler.h, this
>>>>> patch forward declares the RaiseException call.
>>>>>
>>>>> The only functional difference between the current implementation and
>>>>> the new one is that RaiseException will appear on the stack traces now
>>>>> when invoking this macro. Eg)
>>>>>
>>>>> e:\llvm\2013>clang -fsyntax-only "E:\Aaron Ballman\Desktop\test.cpp"
>>>>> 0x755EC42D (0xDEADD0D0 0x00000001 0x00000000 0x00000000), RaiseException()
>>>>> + 0x5
>>>>> 8 bytes(s)
>>>>> 0x03D9F222 (0x00692018 0x00000000 0x0685E3C4 0x0685E3A0), `anonymous
>>>>> namespace':
>>>>> :PragmaDebugHandler::HandlePragma() + 0xC2 bytes(s),
>>>>> e:\llvm\llvm\tools\clang\li
>>>>> b\lex\pragma.cpp, line 870 + 0x13 byte(s)
>>>>> 0x03D997DC (0x00692018 0x00000000 0x0685E3C4 0x0685E494),
>>>>> clang::PragmaNamespace
>>>>> ::HandlePragma() + 0xBC bytes(s),
>>>>> e:\llvm\llvm\tools\clang\lib\lex\pragma.cpp, l
>>>>> ine 95 + 0x1B byte(s)
>>>>> ...
>>>>>
>>>>> instead of:
>>>>>
>>>>> e:\llvm\2013>clang -fsyntax-only "E:\Aaron Ballman\Desktop\test.cpp"
>>>>> 0x03CDF1CF (0x00302018 0x00000000 0x0679E5F8 0x0679E5D4), `anonymous
>>>>> namespace':
>>>>> :PragmaDebugHandler::HandlePragma() + 0xAF bytes(s),
>>>>> e:\llvm\llvm\tools\clang\li
>>>>> b\lex\pragma.cpp, line 870
>>>>> 0x03CD979C (0x00302018 0x00000000 0x0679E5F8 0x0679E6C8),
>>>>> clang::PragmaNamespace
>>>>> ::HandlePragma() + 0xBC bytes(s),
>>>>> e:\llvm\llvm\tools\clang\lib\lex\pragma.cpp, l
>>>>> ine 95 + 0x1B byte(s)
>>>>> ...
>>>>>
>>>>> ~Aaron
>>>>
>>>>