Fixing LLVM_BUILTIN_TRAP on Windows

[Michael Spencer]

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.

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