Page MenuHomePhabricator

Rename EHPersonality::MSVC_Win64SEH to EHPersonality::MSVC_TableSEH. NFC.
ClosedPublic

Authored by luqmana on Oct 25 2020, 6:50 AM.

Details

Summary

The types of SEH aren't x86(-32) vs x64 but rather stack-based exception chaining
vs table-based exception handling. x86-32 is the only arch for which Windows
uses the former. 32-bit ARM would use what is called Win64SEH today, which
is a bit confusing so instead let's just rename it to be a bit more clear.

Diff Detail

Event Timeline

luqmana created this revision.Oct 25 2020, 6:50 AM
luqmana requested review of this revision.Oct 25 2020, 6:50 AM
mstorsjo added a subscriber: mstorsjo.

FWIW, definite +1 from me, but I'll let the other reviewers comment as well. There's lots of other cases where things are named "Win64" when it comes to exception handling and unwind info management as well - are you going to clean up that naming as well? And does this imply you're planning on working on ARM unwinding? That'd be awesome.

compnerd accepted this revision.Oct 26 2020, 10:33 AM

The renaming makes sense to me.

This revision is now accepted and ready to land.Oct 26 2020, 10:33 AM
rnk accepted this revision.Oct 26 2020, 10:52 AM

My other naming suggestion would be MSVC_SEH_CSpecific, kind of inline with GNU_CXX[_SjLj], and the other would be MSVC_SEH_X86, I suppose. I'm assuming ARM and AArch64 all use a handler called __C_specific_handler?

Seems fine either way.

In D90117#2354170, @rnk wrote:

I'm assuming ARM and AArch64 all use a handler called __C_specific_handler?

Yeah, they do.

Personally, I think that table based makes more sense, since that is the defining characteristic of the xdata/pdata approach - there is a table that has that information and you look it up (similar to the zero-cost itanium model). This actually mirrors the GNU_CXX_SjLj better - that uses setjmp and longjmp as the mechanism.

I also think TableSEH is a bit clearer. And yea, I'll try to clean up other similar Win64 cases as I get around.

This revision was landed with ongoing or failed builds.Oct 27 2020, 11:42 PM
This revision was automatically updated to reflect the committed changes.