[PATCH] D86041: RFC [libunwind] Convert the register restore functions to C functions

Tue Aug 18 02:16:18 PDT 2020

mstorsjo added a comment.

In D86041#2222017 <https://reviews.llvm.org/D86041#2222017>, @compnerd wrote:

> The idea itself is reasonable.  The summary description is very confusing would you mind reworking that?

Sure - does this sound more understandable?

[libunwind] Convert the register restore functions to C functions

Currently, the assembly functions for restoring register state have been direct implementations of the C++ method `Registers_*::jumpto()` (contrary to the functions for saving register state, which are implementations of the extern C function `__unw_getcontext`). This has included having the assembly function name match the C++ mangling of that method name (and having the function match the C++ member function calling convention). To simplify the interface between the C++ code and the assembly, make the assembly function have a plain C function interface.

This fixes building the library in with a MSVC C++ ABI, which uses a significantly different method name mangling scheme. (The library might not be of much use as C++ exception unwinder in such an environment, but the libunwind.h interface for stepwise unwinding still is usable, as is the _Unwind_Backtrace function.)

> Could you check what happens for the `jumpto` member?  Ideally it would just be an alias for the function to avoid a frame setup and unnecessary jump.  I think that is the only thing that really stands out of needing work in the implementation.

Yeah, on platforms where the C++ member function calling convention is identical to the one of regular C functions, it's a no-op. On i386 windows, where the thiscall convention takes the `this` pointer in ecx instead of as a parameter on the stack, it's a couple instructions of difference - but it's all inlined into the caller in `UnwindCursor::jumpto()` anyway, so there's no extra stack frame inbetween.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86041/new/

https://reviews.llvm.org/D86041