RFC [libunwind] Convert the register restore functions to C functions

[mstorsjo]

Summary

This fixes linking if built in MSVC mode. Even though the main unwinder interface isn't of much use in an MSVC environment, one might plausibly want to use libunwind for backtraces even there.

Only handled x86, x86_64 and arm64 so far. In particular, on arm32, it's not one function but a number of smaller functions.

Just posting this as a RFC. It simplifies the interface of the assembly somewhat, but might also require changes to the code for the cases where the calling convention differs between C and C++ functions (like on i386 windows).

[compnerd]

The idea itself is reasonable. The summary description is very confusing would you mind reworking that? 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.

[mstorsjo]

In 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.

[compnerd]

I think that the confusing part is that it is not that they are C functions, they are still assembly, but with C calling convention. I think that you should state that in the description as it currently reads as though they are written in C, which they are not.

Right, there may need to be a thunk for some platforms but if the compiler does treat the call to the symbol as a tail call, that is fine.

Can you share the reasoning for building libunwind with MSVC? In particular, the calling convention isn't identical to the Unix calling convention, so you will need to use the Microsoft unwinder to get interoperability with MS ABI.

[mstorsjo]

In D86041#2228770, @compnerd wrote:

I think that the confusing part is that it is not that they are C functions, they are still assembly, but with C calling convention. I think that you should state that in the description as it currently reads as though they are written in C, which they are not.

Ah, I see. Rephrasing:

[libunwind] Convert the register restore functions to C calling convention and name mangling

Currently, the assembly functions for restoring register state have been direct implementations of the Registers_*::jumpto() method (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 of the assembly implementations, make the functions have C calling conventions and name mangling.

This fixes building the library in with a MSVC C++ ABI with clang-cl, 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.)

Right, there may need to be a thunk for some platforms but if the compiler does treat the call to the symbol as a tail call, that is fine.

Can you share the reasoning for building libunwind with MSVC? In particular, the calling convention isn't identical to the Unix calling convention, so you will need to use the Microsoft unwinder to get interoperability with MS ABI.

So for the actual calling convention, the existing assembly functions already have ifdefs so they work with the windows calling conventions for mingw.

Then for actual unwinding; libunwind, if operating in SEH mode, does call the Microsoft unwinder APIs, so it works in such an environment (just like it does in SEH mode for mingw). Most of the APIs from unwind.h doesn't make much sense to use there though (e.g. _Unwind_RaiseException and the related ones), but e.g. _Unwind_Backtrace should work just fine (just giving a callback for each stack frame), and the lower level libunwind.h APIs (unw_getcontext, unw_step, unw_get_reg etc) should work just fine.

If writing code from scratch for use on Windows, there'd be little point in using these instead of just calling the Microsoft unwinding APIs directly, but if porting user space code that already is calling these APIs on other platforms, it could make sense to use libunwind. That's the case that was posted on the llvm-dev list a few days ago.

[compnerd]

Sounds good to me; LGTM with the rephrased commit message.

libunwind/src/Registers.hpp

enum {
   REGISTERS_MIPS_O32,
   REGISTERS_MIPS_NEWABI,
   REGISTERS_SPARC,
   REGISTERS_HEXAGON,
   REGISTERS_RISCV,
 };
 
 #if defined(_LIBUNWIND_TARGET_I386)
+class _LIBUNWIND_HIDDEN Registers_x86;
+extern "C" void __libunwind_Registers_x86_jumpto(Registers_x86 *);
 /// Registers_x86 holds the register state of a thread in a 32-bit intel
 /// process.
 class _LIBUNWIND_HIDDEN Registers_x86 {
 public:
   Registers_x86();
   Registers_x86(const void *registers);
 
   bool        validRegister(int num) const;
   uint32_t    getRegister(int num) const;
   void        setRegister(int num, uint32_t value);
   bool        validFloatRegister(int) const { return false; }
   double      getFloatRegister(int num) const;
   void        setFloatRegister(int num, double value);
   bool        validVectorRegister(int) const { return false; }
   v128        getVectorRegister(int num) const;
   void        setVectorRegister(int num, v128 value);
   static const char *getRegisterName(int num);
-  void        jumpto();
+  void        jumpto() { __libunwind_Registers_x86_jumpto(this); }
   static int  lastDwarfRegNum() { return _LIBUNWIND_HIGHEST_DWARF_REGISTER_X86; }
   static int  getArch() { return REGISTERS_X86; }
 
   uint32_t  getSP() const          { return _registers.__esp; }
   void      setSP(uint32_t value)  { _registers.__esp = value; }
   uint32_t  getIP() const          { return _registers.__eip; }
   void      setIP(uint32_t value)  { _registers.__eip = value; }
   uint32_t  getEBP() const         { return _registers.__ebp; }
inline void Registers_x86::setVectorRegister(int, v128) {
   _LIBUNWIND_ABORT("no x86 vector registers");
 }
 #endif // _LIBUNWIND_TARGET_I386
 
 
 #if defined(_LIBUNWIND_TARGET_X86_64)
 /// Registers_x86_64  holds the register state of a thread in a 64-bit intel
 /// process.
+class _LIBUNWIND_HIDDEN Registers_x86_64;
+extern "C" void __libunwind_Registers_x86_64_jumpto(Registers_x86_64 *);
 class _LIBUNWIND_HIDDEN Registers_x86_64 {
 public:
   Registers_x86_64();
   Registers_x86_64(const void *registers);
 
   bool        validRegister(int num) const;
   uint64_t    getRegister(int num) const;
   void        setRegister(int num, uint64_t value);
   bool        validFloatRegister(int) const { return false; }
   double      getFloatRegister(int num) const;
   void        setFloatRegister(int num, double value);
   bool        validVectorRegister(int) const;
   v128        getVectorRegister(int num) const;
   void        setVectorRegister(int num, v128 value);
   static const char *getRegisterName(int num);
-  void        jumpto();
+  void        jumpto() { __libunwind_Registers_x86_64_jumpto(this); }
   static int  lastDwarfRegNum() { return _LIBUNWIND_HIGHEST_DWARF_REGISTER_X86_64; }
   static int  getArch() { return REGISTERS_X86_64; }
 
   uint64_t  getSP() const          { return _registers.__rsp; }
   void      setSP(uint64_t value)  { _registers.__rsp = value; }
   uint64_t  getIP() const          { return _registers.__rip; }
   void      setIP(uint64_t value)  { _registers.__rip = value; }
   uint64_t  getRBP() const         { return _registers.__rbp; }
inline const char *Registers_ppc64::getRegisterName(int regNum) {
   return "unknown register";
 }
 #endif // _LIBUNWIND_TARGET_PPC64
 
 
 #if defined(_LIBUNWIND_TARGET_AARCH64)
 /// Registers_arm64  holds the register state of a thread in a 64-bit arm
 /// process.
+class _LIBUNWIND_HIDDEN Registers_arm64;
+extern "C" void __libunwind_Registers_arm64_jumpto(Registers_arm64 *);
 class _LIBUNWIND_HIDDEN Registers_arm64 {
 public:
   Registers_arm64();
   Registers_arm64(const void *registers);
 
   bool        validRegister(int num) const;
   uint64_t    getRegister(int num) const;
   void        setRegister(int num, uint64_t value);
   bool        validFloatRegister(int num) const;
   double      getFloatRegister(int num) const;
   void        setFloatRegister(int num, double value);
   bool        validVectorRegister(int num) const;
   v128        getVectorRegister(int num) const;
   void        setVectorRegister(int num, v128 value);
   static const char *getRegisterName(int num);
-  void        jumpto();
+  void        jumpto() { __libunwind_Registers_arm64_jumpto(this); }
   static int  lastDwarfRegNum() { return _LIBUNWIND_HIGHEST_DWARF_REGISTER_ARM64; }
   static int  getArch() { return REGISTERS_ARM64; }
 
   uint64_t  getSP() const         { return _registers.__sp; }
   void      setSP(uint64_t value) { _registers.__sp = value; }
   uint64_t  getIP() const         { return _registers.__pc; }
   void      setIP(uint64_t value) { _registers.__pc = value; }
   uint64_t  getFP() const         { return _registers.__fp; }

libunwind/src/UnwindRegistersRestore.S

 //===-------------------- UnwindRegistersRestore.S ------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "assembly.h"
 
   .text
 
 #if !defined(__USING_SJLJ_EXCEPTIONS__)
 
 #if defined(__i386__)
-DEFINE_LIBUNWIND_FUNCTION(_ZN9libunwind13Registers_x866jumptoEv)
+DEFINE_LIBUNWIND_FUNCTION(__libunwind_Registers_x86_jumpto)
 #
-# void libunwind::Registers_x86::jumpto()
+# extern "C" void __libunwind_Registers_x86_jumpto(Registers_x86 *);
 #
-#if defined(_WIN32)
-# On windows, the 'this' pointer is passed in ecx instead of on the stack
-  movl   %ecx, %eax
-#else
 # On entry:
 #  +                       +
 #  +-----------------------+
 #  + thread_state pointer  +
 #  +-----------------------+
 #  + return address        +
 #  +-----------------------+   <-- SP
 #  +                       +
   movl   4(%esp), %eax
-#endif
   # set up eax and ret on new stack location
   movl  28(%eax), %edx # edx holds new stack pointer
   subl  $8,%edx
   movl  %edx, 28(%eax)
   movl  0(%eax), %ebx
   movl  %ebx, 0(%edx)
   movl  40(%eax), %ebx
   movl  %ebx, 4(%edx)
#  +                       +
   # skip cs
   # skip ds
   # skip es
   # skip fs
   # skip gs
 
 #elif defined(__x86_64__)
 
-DEFINE_LIBUNWIND_FUNCTION(_ZN9libunwind16Registers_x86_646jumptoEv)
+DEFINE_LIBUNWIND_FUNCTION(__libunwind_Registers_x86_64_jumpto)
 #
-# void libunwind::Registers_x86_64::jumpto()
+# extern "C" void __libunwind_Registers_x86_64_jumpto(Registers_x86_64 *);
 #
 #if defined(_WIN64)
 # On entry, thread_state pointer is in rcx; move it into rdi
 # to share restore code below. Since this routine restores and
 # overwrites all registers, we can use the same registers for
 # pointers and temporaries as on unix even though win64 normally
 # mustn't clobber some of them.
   movq  %rcx, %rdi
Lnovec:
   lwz     %r4,  24(%r3)   // do r4 now
   lwz     %r1,  12(%r3)   // do sp now
   lwz     %r3,  20(%r3)   // do r3 last
   bctr
 
 #elif defined(__aarch64__)
 
 //
-// void libunwind::Registers_arm64::jumpto()
+// extern "C" void __libunwind_Registers_arm64_jumpto(Registers_arm64 *);
 //
 // On entry:
 //  thread_state pointer is in x0
 //
   .p2align 2
-DEFINE_LIBUNWIND_FUNCTION(_ZN9libunwind15Registers_arm646jumptoEv)
+DEFINE_LIBUNWIND_FUNCTION(__libunwind_Registers_arm64_jumpto)
   // skip restore of x0,x1 for now
   ldp    x2, x3,  [x0, #0x010]
   ldp    x4, x5,  [x0, #0x020]
   ldp    x6, x7,  [x0, #0x030]
   ldp    x8, x9,  [x0, #0x040]
   ldp    x10,x11, [x0, #0x050]
   ldp    x12,x13, [x0, #0x060]
   ldp    x14,x15, [x0, #0x070]