Fixing LLVM_BUILTIN_TRAP on Windows

[Aaron Ballman]

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

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
>>
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Compiler.h.patch
Type: application/octet-stream
Size: 977 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150209/3f6d5f89/attachment.obj>