[PATCH] D50564: Add support for SEH unwinding on Windows.

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

> I've tested this implementation on x86-64 to ensure that it works. All libc++abi tests pass, as do all libc++ exception-related tests.

I tested it now as well, and seems to work for my simple testcase.

> ARM still remains to be implemented (@compnerd?).

LLVM doesn't generate SEH unwind data for ARM at all yet. ARM64 implementation is ongoing at the moment though, see https://reviews.llvm.org/D50166 and https://reviews.llvm.org/D50288.

> Special thanks to KJK::Hyperion for his excellent series of articles on how EH works on x86-64 Windows. (Seriously, check it out. It's awesome.)

Can you give some links to it? A brief googling didn't turn up much else than the PSEH library.

> I'm actually not sure if this should go in as is. I particularly don't like that I duplicated the UnwindCursor class for this special case.

As I'm not totally familiar with how it works, I can only give cursory comments and compare to the libgcc implementation when trying to wrap my head around it, but this does seem way, way more complex than the libgcc version of the same.

As for the duplicated UnwindCursor, I'm thinking if it'd become more straightforward by avoiding touching those parts altogether, and just reimplementing all the other public functions, `_Unwind_G/SetGR/IP`, `_Unwind_Backtrace` etc, directly in Unwind-seh.cpp, and not care about using the libunwind.h APIs (`unw_*`) inbetween altogether? Or perhaps my libgcc comparison is making me biased.



================
Comment at: src/Unwind-seh.cpp:53
+
+/// Exception cleanup routine used by \c __libunwind_frame_consolidate to
+/// regain control after handling an SEH exception.
----------------
I don't see any `__libunwind_frame_consolidate` anywhere, is this comment outdated?


================
Comment at: src/UnwindLevel1.c:35
 
+#if !_LIBUNWIND_SUPPORT_SEH_UNWIND
+
----------------
This probably works, but isn't `#if !defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)` more of the common form?


================
Comment at: src/libunwind_ext.h:43
+  #if defined(__x86_64__) && !defined(__MINGW64__)
+typedef struct _DISPATCHER_CONTEXT {
+  ULONG64 ControlPc;
----------------
What's this all about? winnt.h (from both MSVC and mingw-w64) should define this struct, no?


================
Comment at: src/libunwind_ext.h:73
+  #endif
+extern int _unw_init_seh(unw_cursor_t *cursor, CONTEXT *ctx);
+extern DISPATCHER_CONTEXT *_unw_seh_get_disp_ctx(unw_cursor_t *cursor);
----------------
These are all both defined and called from Unwind-seh.cpp, so couldn't they just be static functions within there?


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564