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

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Not much more comments from me. The implementation seems reasonable, and works for one simple test I did (with an earlier revision of the patch at least), and further refinement can happen in-tree I guess.

I'd like to have someone else (@rnk @compnerd?) give it a more proper approval though, at least a general ack for the style/structure.



================
Comment at: include/__libunwind_config.h:66
+#    if defined(__ARM_WMMX)
+#      define _LIBUNWIND_CONTEXT_SIZE 61
+#    else
----------------
I don't think the `__ARM_WMMX` case here is relevant; there are no ARM CPUs with WMMX running modern windows on arm, afaik (and the size number here I presume only is a copy from the one below); sorry for not pointing it out earlier.


================
Comment at: src/UnwindCursor.hpp:54
 #include "EHHeaderParser.hpp"
-#include "libunwind.h"
+#include "libunwind_ext.h"
 #include "Registers.hpp"
----------------
mstorsjo wrote:
> This looks like another leftover; there's no `libunwind_ext.h` any longer here.
Sorry I think I misread and looked for `libunwind_ext.h` in the `include` dir only. But in case it isn't really needed here, keep the old `libunwind.h` include at least, in order not to break other potential build configurations that might need it.


================
Comment at: src/UnwindCursor.hpp:1157
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+  pint_t getLastPC() const { /* FIXME: Implement */ return 0; }
----------------
cdavis5x wrote:
> mstorsjo wrote:
> > What does this whole block do here? Isn't this within an !SEH ifdef block? The same methods are declared for the SEH version of this struct further above.
> Right now, it doesn't do anything. I added it so @kristina has someplace to put the code for unwinding with SEH data when we're //not// on Windows and we //don't// have the `RtlUnwindEx()` function available. You'll note that the '!SEH' `ifdef` now says:
> 
> ```lang=c++
> #if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND) && defined(_WIN32)
> ```
Ah, I see. Maybe a comment clarifying that here as well?


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564