[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Ok, I've found my issue. inttypes.h in libcxx include_next's <inttypes.h>, which on my debian linux systems has the include chain <features.h>, <stubs.h>, <sgidefs.h>. <sgidefs.h> "helpfully" provides #defines of the various _ABIXXX macros, which normally the compiler defines depending on the ABI. The include chain for other parts of libunwind differ which means src/UnwindLevel1-gcc-ext.c had a different definition of the stuct unw_context_t, which was the version defined for O32. As GCC and clang laid out the stack differently for each ABI differently the bug was masked. However for N32 at https://reviews.llvm.org/owners/package/3/, the layout was such that the unwind context in the locals area was right below the saved registers for _Unwind_Backtrace which trashed older saved values, then triggered a SIGSEGV on return due to reloading the saved contents of the $lo  into $ra in my case.

The fix is fairly simple. Change the ABI #ifdef defined() checks to be:

  #ifdef _defined(_ABIO32) && _MIPS_SIM == _ABIO32

for O32 or similar for a single ABI. For N32 and N64 together, it's sufficient to test for the presence of __mips64.



================
Comment at: include/__libunwind_config.h:74
 # elif defined(__mips__)
 #  if defined(_ABIO32) && defined(__mips_soft_float)
 #    define _LIBUNWIND_TARGET_MIPS_O32 1
----------------
See my comment about the N32 abi check.


================
Comment at: include/__libunwind_config.h:78
 #    define _LIBUNWIND_CURSOR_SIZE 24
+#  elif defined(_ABIN32) && defined(__mips_soft_float)
+#    define _LIBUNWIND_TARGET_MIPS_NEWABI 1
----------------
This needs to be:

 
```
   #    elif defined(_ABIN32) && _MIPS_SIM == _ABIN32
```


================
Comment at: include/__libunwind_config.h:82
+#    define _LIBUNWIND_CURSOR_SIZE 42
 #  elif defined(_ABI64) && defined(__mips_soft_float)
 #    define _LIBUNWIND_TARGET_MIPS_NEWABI 1
----------------
Same here.


================
Comment at: src/AddressSpace.hpp:233
+inline uint64_t LocalAddressSpace::getRegister(pint_t addr) {
+#if __SIZEOF_POINTER__ == 8 || (defined(__mips__) && defined(_ABIN32))
+  return get64(addr);
----------------
Change that mips check to defined(__mips64) which covers the target being mips and it being n64 or n32.


================
Comment at: src/UnwindRegistersRestore.S:688
 
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32)) &&            \
+    defined(__mips_soft_float)
----------------
Rather checking that the _ABI64 or _ABIN32 are defined, check that __mips64 is defined.


================
Comment at: src/UnwindRegistersSave.S:175
 
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32)) &&            \
+    defined(__mips_soft_float)
----------------
See my comment on register restore file.


================
Comment at: src/libunwind.cpp:66
 # define REGISTER_KIND Registers_mips_o32
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABIN32) || defined(_ABI64)) &&            \
+    defined(__mips_soft_float)
----------------
Check that __mips64 is defined rather than the specific _ABI macros.


https://reviews.llvm.org/D39074