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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
