cdavis5x marked 2 inline comments as done.
cdavis5x added inline comments.
================
Comment at: src/Unwind-seh.cpp:163
+#ifdef __x86_64__
+ unw_get_reg(&cursor, UNW_X86_64_RAX, &retval);
+ unw_get_reg(&cursor, UNW_X86_64_RDX, &exc->private_[3]);
----------------
mstorsjo wrote:
> Without understanding the code flow completely - is there a risk that this
> tries to use an uninitialized cursor, in case we hit `ctx = (struct
> _Unwind_Context *)ms_exc->ExceptionInformation[1];` above and never
> initialized the local cursor?
We should only hit this point if we're in phase 2 (i.e.
`IS_UNWINDING(exc->ExceptionFlags)` is true). But you've now got me worried
about the potential for a rogue personality function surreptitiously returning
this to exploit libunwind. I've added a check here.
================
Comment at: src/Unwind-seh.cpp:174
+ CONTEXT new_ctx;
+ RtlUnwindEx(frame, (PVOID)target, ms_exc, (PVOID)retval, &new_ctx,
disp->HistoryTable);
+ _LIBUNWIND_ABORT("RtlUnwindEx() failed");
----------------
mstorsjo wrote:
> Who will get this new uninitialized CONTEXT here, and will they try to use it
> for something?
It won't be uninitialized. `RtlUnwindEx()` always starts unwinding from the
current frame; it fills in the passed in `CONTEXT` and passes its address to
any handlers it encounters along the way. In other words, it's there so
`RtlUnwindEx()` has a place to keep track of the unwind context.
================
Comment at: src/UnwindCursor.hpp:1157
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+ pint_t getLastPC() const { /* FIXME: Implement */ return 0; }
----------------
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)
```
================
Comment at: src/UnwindCursor.hpp:1801
+ if (pc != getLastPC()) {
+ UNWIND_INFO *xdata = reinterpret_cast<UNWIND_INFO *>(base +
unwindEntry->UnwindData);
+ if (xdata->Flags & (UNW_FLAG_EHANDLER|UNW_FLAG_UHANDLER)) {
----------------
kristina wrote:
> mstorsjo wrote:
> > I can't say I understand all of this yet, but I'm slowly getting there, and
> > I'm trying to compare this to what libgcc does.
> >
> > libgcc doesn't use any definition of UNWIND_INFO and doesn't need to do the
> > equivalent of `getInfoFromSEH`, used by `step()`, anywhere. `unw_step()` is
> > used in `_Unwind_ForcedUnwind`, which in libgcc is implemented using
> > `RaiseException (STATUS_GCC_FORCED, ...`.
> >
> > I guess if you happen to have all of the `unw_step` API available, it's
> > easier to just do it like this, in custom code without relying on the NTDLL
> > functions for it, while the libgcc version relies more on the NTDLL API.
> This primarily deals with the SEH exceptions re-purposed as a C++ exception
> mechanism on x86_64 (if I understood this right), it's possible to set a
> custom filter using a runtime call so I suspect GCC does that or defines a
> translation function (also via a runtime call) which acts as a filter for
> "true" SEH exceptions behind the scenes deep within the runtime. Typially
> "true" SEH exceptions don't, outside of runtime magic, play nicely with C++
> exceptions, with the `__C_specific_handler` ones being a completely different
> paradigm that falls far outside the scope of libunwind (those ones being the
> "true"/explicit SEH exceptions).
>
> (Don't take my word for it, it's been a while and I only implemented the
> "true" variation for 64-bit Linux by reserving some RT signals and using that
> to invoke additional runtime glue that would then do the unwinding,
> completely ignoring DWARF since CFI exceptions and SEH exceptions really
> don't mix especially on platforms that are not Windows-like)
Actually, it's just to implement the lower-level `libunwind`
APIs--specifically, `unw_get_info()`. The code @kristina is talking about is
all in Unwind-seh.cpp.
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
