<div dir="ltr">Just catching up on this now.<br>r319363 looks good to me, It makes little sense to use GNU SEH in that case.<br><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">My remaining concern is mostly about why we still need the workaround
for x86 in the function getting the default (returning None instead of
WinEH for that case). But as long as this works and the rest of this
change can settle, we can look at that later if any further change is
warranted at all.<br></blockquote><div>It seemed to work fine but the sanitizer does not support this setup for x86 quite yet.<br></div><div>It is a little outside my scope of focus right now so will probably come back to it much later.<br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 29, 2017 at 2:07 PM, Martin Storsjö <span dir="ltr"><<a href="mailto:martin@martin.st" target="_blank">martin@martin.st</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wed, 29 Nov 2017, Martin Storsjö via cfe-commits wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Wed, 29 Nov 2017, Reid Kleckner wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Wed, Nov 29, 2017 at 12:21 PM, Martin Storsjö <<a href="mailto:martin@martin.st" target="_blank">martin@martin.st</a>> wrote:<br>
On Wed, 29 Nov 2017, Martell Malone via cfe-commits wrote:<br>
Thanks for letting me know Reid.<br>
I’ll in work and won’t be able to access the repo<br>
until lunch time. (~3<br>
hours)<br>
Feel free to revert if it is not trivial.<br>
<br>
The easy fix might be to change to == x86_64 from !=<br>
x86 For is Windows in<br>
the default toolchain. That should restore the old<br>
behavior.<br>
<br>
<br>
My suggestion would be to just return None for all architectures<br>
for the default windows (msvc) case. We didn't use to set any<br>
defines to indicate EH mode there before anyway, so setting it<br>
to None should make things behave just as before, right?<br>
<br>
<br>
I did this slightly differently in r319363, but maybe that's silly. My<br>
reasoning was that `clang -cc1 -fseh-exceptions -fexceptions` in the MSVC<br>
environment should still use __CxxFrameHandler3. -fseh-exceptions indicates<br>
what format of unwind information we should use, and we're still using the<br>
normal SEH .xdata opcodes.<br>
</blockquote>
<br>
No, I think this change makes sense - I was just about to write a comment pointing out this spot in the code but was waiting for a compile to finish before posting.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Alternatively, you could view -fseh-exceptions, -fdwarf-exceptions, and<br>
-fsjlj-exceptions as choices of EH personality function,<br>
</blockquote>
<br>
No, I don't think that'd make sense<br>
</blockquote>
<br></div></div>
FWIW, another reason I don't think that makes sense, is that making a GNU_CPlusPlus_SEH + MSVC combination probably would require quite a bit more changes as well.<br>
<br>
When I tested the same in a build with assertions enabled, I didn't get the references to _Unwind_Resume as you did, but the compilation failed on some internal assertion. So making GNU_CPlusPlus_SEH usable with the microsoft C++ ABI would probably require a significant amount of more work, for very little value.<span class="HOEnZb"><font color="#888888"><br>
<br>
// Martin</font></span></blockquote></div><br></div>