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

[bsdjhb]

Summary

N32 uses the same register context as N64. However, N32 requires one
change to properly fetch addresses from registers stored in memory.
Since N32 is an ILP32 platform, getP() only fetches the first 32-bits
of a stored register. For a big-endian platform this fetches the
upper 32-bits which will be zero. To fix this, add a new
getRegister() method to AddressSpace which is responsible for
extracting the address stored in a register in memory. This matches
getP() for all current ABIs except for N32 where it reads the 64-bit
register and returns the low 32-bits as an address. The
DwarfInstructions::getSavedRegister() method uses
AddressSpace::getRegister() instead of AddressSpace::getP().

Possibly, DwarfInstructions::getSavedRegister()'s return type should
be changed from pint_t to uint64_t.

[sdardis]

include/__libunwind_config.h

71–80

Change the define of _LIBUNWIND_TARGET_MIPS_N64 to _LIBUNWIND_TARGET_MIPS_NEWABI, then these two elif branches can have the condition (defined(_ABIN32) && defined(_ABI64) and refactored into one elif branch.

74

Shouldn't this 46 be 47?

src/AddressSpace.hpp

233

defined(__mips_n32) -> (defined(__mips__) && defined(_ABIN32))

src/UnwindRegistersRestore.S

688–689

This line is overly long, break with '\' after the second &&.

src/UnwindRegistersSave.S

175–176

This line looks overly long, break before the second &&.

src/libunwind.cpp

66–69

Fold these two branches together and have the condition (defined(_ABIN32) || defined(_ABI64), then follow-up by renaming Registers_mips_n64 to Register_mips_NEWABI. Add a comment to the definition of that class stating that it covers both n32 and n64.

[bsdjhb]

• Use correct #ifdef for N32.
• Rename N64 to newabi.

[bsdjhb]

include/__libunwind_config.h

74

No, the other parts of a cursor besides the register context use ILP32 layout for N32 instead of LP64. In particular, I think N32 has less padding after the two 'bool' members at the end of UnwindCursor<>.

[bsdjhb]

• Rebase after MAX_REGISTER change.

[bsdjhb]

• Rebase after O32/N64 commit.

[compnerd]

include/__libunwind_config.h

79

Minor nit: I prefer either NABI or NEW_ABI.

src/AddressSpace.hpp

233

Can you use __SIZEOF_POINTER__ rather than __LP64__ please? The former accounts for LLP64 environments as well.

src/UnwindRegistersRestore.S

688–689

clang-format should also fix the width correctly.

[sdardis]

include/__libunwind_config.h

79

Normally mips documentation/source code either spells out n32/n64 when referring to the 64 bit abis or calls it a variation of NewABI/newabi/NEWABI (all one word).

src/Registers.hpp

2252–2253 ↗

(On Diff #126620)

Nit on the wording:

Registers_mips_newabi holds the register state of a thread in a NEWABI MIPS process including both the N32 and N64 ABIs. ->

Registers_mips_newabi holds the register state of a thread in a MIPS process using NEWABI (the N32 or N64 ABIs).

[bsdjhb]

src/AddressSpace.hpp

233

I wonder if we should adjust the #if condition used to control the types of pint_t and sint_t earlier in this file to also use SIZEOF_POINTER (as a separate change)?

[bsdjhb]

• Use SIZEOF_POINTER instead of LP64.
• Adjust comment for newabi register class.

[bsdjhb]

ping @sdardis, @compnerd

[compnerd]

Looking over this patch again, I think I really would prefer that this was split up into two patches. The first one should be entirely mechanical, replacing n64 with newabi. The second patch would actually make the changes that you are are after. That would really help with focusing what the issue here actually is. I don't see anything technically that is an issue (I admit I didn't verify the sizes, but the assertions should hopefully catch that). Beyond that split up, Id like to get a signoff from @sdardis for the MIPS specific bits, but from the libunwind side, LGTM.

[bsdjhb]

Ok, I'm definitely fine with splitting the rename out into a separate patch. Will wait for @sdardis to be sure.

[bsdjhb]

• Rebase after N64 -> newabi commit.

[sdardis]

Sorry for the delay in getting back to this, but testing this by building it explicitly for N32 (I built a full N32 compiler + libunwind and modified the test setup py to compile for N322) and I'm getting crashes on the return path in _Unwind_Backtrace.

The problem occurs when saving the registers, as saving the (second I believe) last register clobbers the saved return address. I've seen crashes on MIPS64 (eb). Qemu however is aborting at runtime, but I haven't had the time to trace the execution, and I haven't found a way yet to dump the locations of the shared libraries at runtime to determine where the fault lies.

This was with GCC 4.9.2 (debian) on MIPS64 and our 2016.05-06 toolchain for qemu.

[bsdjhb]

To be clear, are you getting the failure running libunwind's test suite or your own test? I've managed to get libunwind to cross-compile for me using GCC 6.3.0 on FreeBSD for O32, N32, and N64, but only to build the library, not the tests. I've been running a simple C++ test program (which is using a patched libunwind along with libc++ as it's C++ runtime) for testing. The program uses _Unwind_Backtrace() as well as throws a couple of C++ exceptions with catch handlers that print out the values thrown. If you are able to cross-build the libunwind tests and then run them under qemu I'd appreciate a pointer to get that working as I'd be happier running libunwind's tests than my own.

[sdardis]

This was libunwind's test suite:

Compiled test failed unexpectedly!
********************
Testing Time: 0.53s
********************
Failing Tests (1):
    libunwind :: libunwind_02.pass.cpp

  Expected Passes    : 3
  Unexpected Failures: 1

The hacky patch I used to test n32:

--- a/test/libunwind/test/config.py
+++ b/test/libunwind/test/config.py
@@ -48,6 +48,8 @@ class Configuration(LibcxxConfiguration):
         # Stack unwinding tests need unwinding tables and these are not
         # generated by default on all Targets.
         self.cxx.compile_flags += ['-funwind-tables']
+        self.cxx.compile_flags += ['-mabi=n33']
+        self.cxx.link_flags += ['-mabi=n32']
         if not self.get_lit_bool('enable_threads', True):
             self.cxx.compile_flags += ['-D_LIBUNWIND_HAS_NO_THREADS']
             self.config.available_features.add('libunwind-no-threads')

[sdardis]

Um, I now appear to be getting different results for running under QEMU doing it the proper way. I was previously rebuilding the failing test by hand and running under qemu. I don't believe I changed anything important, I'll have to take a longer look.

If you define an Executor for libunwind you can run the testsuite under QEMU, automagically.

I have:

LIBUNWIND_EXECUTOR               PrefixExecutor(['/home/sdardis/bin/qemu-mipsn32.sh'],LocalExecutor())

/home/sdardis/bin/qemu-mips32.sh for is a simple bash script:

#!/bin/bash
~/mips-mti-linux-gnu/2016.05-06/bin/qemu-mipsn32 -L /home/sdardis/mips-mti-linux-gnu/2016.05-06/sysroot/mips-r2-hard/ -E LD_LIBRARY_PATH=/home/sdardis/mips-mti-linux-gnu/2016.05-06/mips-mti-linux-gnu/lib/mips-r2-hard/lib32/ "$@"

Hope this helps.

Thanks for all the work you're putting into this.

[bsdjhb]

In D39074#974913, @sdardis wrote:

This was libunwind's test suite:

Compiled test failed unexpectedly!
********************
Testing Time: 0.53s
********************
Failing Tests (1):
    libunwind :: libunwind_02.pass.cpp

  Expected Passes    : 3
  Unexpected Failures: 1

The hacky patch I used to test n32:

--- a/test/libunwind/test/config.py
+++ b/test/libunwind/test/config.py
@@ -48,6 +48,8 @@ class Configuration(LibcxxConfiguration):
         # Stack unwinding tests need unwinding tables and these are not
         # generated by default on all Targets.
         self.cxx.compile_flags += ['-funwind-tables']
+        self.cxx.compile_flags += ['-mabi=n33']
+        self.cxx.link_flags += ['-mabi=n32']
         if not self.get_lit_bool('enable_threads', True):
             self.cxx.compile_flags += ['-D_LIBUNWIND_HAS_NO_THREADS']
             self.config.available_features.add('libunwind-no-threads')

Just to be sure, is that '-mabi=n33' in the compile flags a copy and paste typo in the diff or do you have it locally in the real change as well?

[sdardis]

I just checked both my qemu copy and on my mips64 machine and it seems to be a a copy / paste error. Reposting here directly from my machines:

MIPS64:

diff --git a/test/libunwind/test/config.py b/test/libunwind/test/config.py
index 2a0c828..a8952c3 100644
--- a/test/libunwind/test/config.py
+++ b/test/libunwind/test/config.py
@@ -48,6 +48,8 @@ class Configuration(LibcxxConfiguration):
         # Stack unwinding tests need unwinding tables and these are not
         # generated by default on all Targets.
         self.cxx.compile_flags += ['-funwind-tables']
+       self.cxx.compile_flags += ['-mabi=n32']
+        self.cxx.link_flags += ['-mabi=n32']
         if not self.get_lit_bool('enable_threads', True):
             self.cxx.compile_flags += ['-D_LIBUNWIND_HAS_NO_THREADS']
             self.config.available_features.add('libunwind-no-threads')

X86_64:

diff --git a/test/libunwind/test/config.py b/test/libunwind/test/config.py
index 2a0c828..f1953e2 100644
--- a/test/libunwind/test/config.py
+++ b/test/libunwind/test/config.py
@@ -48,6 +48,8 @@ class Configuration(LibcxxConfiguration):
         # Stack unwinding tests need unwinding tables and these are not
         # generated by default on all Targets.
         self.cxx.compile_flags += ['-funwind-tables']
+       self.cxx.compile_flags += ['-mabi=n32']
+        self.cxx.link_flags += ['-mabi=n32', '/home/sdardis/mips-mti-linux-gnu/2016.05-06/bin/../lib/gcc/mips-mti-linux-gnu/4.9.2/../../../../mips-mti-linux-gnu/lib/mips-r2-hard/lib32/libgcc_s.so.1']
         if not self.get_lit_bool('enable_threads', True):
             self.cxx.compile_flags += ['-D_LIBUNWIND_HAS_NO_THREADS']
             self.config.available_features.add('libunwind-no-threads')

I corrected whitespace nit before I posted the comment.

[bsdjhb]

After fighting with cmake for a bit, I just broke down and cross-compiled the tests by hand and then ran them under a qemu system (rather than using qemu user mode). All of the tests ran fine for me without crashing using GCC 6.3.0 for FreeBSD 12 with N32. Given the save/restore code is identical for N32 and N64 I would have expected N64 to fail previously if it was restoring the wrong register?

[bsdjhb]

@sdardis Can you confirm if the existing N64 bits work fine for you or if the tests crash similarly?

[sdardis]

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 O3, 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.

include/__libunwind_config.h

74

See my comment about the N32 abi check.

78

This needs to be:

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

82

Same here.

src/AddressSpace.hpp

233

Change that mips check to defined(__mips64) which covers the target being mips and it being n64 or n32.

src/UnwindRegistersRestore.S

688

Rather checking that the _ABI64 or _ABIN32 are defined, check that __mips64 is defined.

src/UnwindRegistersSave.S

175

See my comment on register restore file.

src/libunwind.cpp

66

Check that __mips64 is defined rather than the specific _ABI macros.

[bsdjhb]

• Rebase.
• Rework ABI macro checks.

[bsdjhb]

Nice sleuthing!

[sdardis]

The only thing this needs now is to get correct testing support. Could you add support for passing down to the configuration script an additional set of cflags like compiler-rt and libomp do (as a separate patch)? If you look at libomp, you'll see LIBOMP_TEST_CFLAGS defined in the cmake build system and threaded through various places so the built tests can have the correct options. Without supporting that we get a mixed set of objects, which isn't correct.

[bsdjhb]

Hmmm, I'm a bit lost on the CFLAGS bit. I couldn't find a reference to LIBOMP_TEST_CFLAGS anywhere in the openmp tree. There is a LIBOMP_CFLAGS that doesn't appear to be test specific.

To try to find a way to modify the CFLAGS for tests I looked at how LIBUNWIND_BUILD_32_BITS works. It sets a value (config.enable_32bit) in test/lit.site.cfg.in. This is then used by the utils/libcxx/test/config.py script in the libcxx repository in the configure_default_compile_flags() function to add -m32 to CFLAGS. There isn't an existing config.* knob that can add arbitrary things to CFLAGS though, so I think adding a LIBUNWIND_TEST_CFLAGS would mean changing the config.py to learn about a new 'config.cflags' or the like and then mapping LIBUNWIND_TEST_CFLAGS to that via lit.site.cfg.in?

[sdardis]

Yes. Something like LIBUNWIND_TEST_CFLAGS mapping to config.test_cflags, which libunwind/test/libunwind/test/config.py then appends when building the tests.

Thanks.

[sdardis]

LGTM after D43585 lands.

include/__libunwind_config.h

 #  else
 #    define _LIBUNWIND_CONTEXT_SIZE 42
 #    define _LIBUNWIND_CURSOR_SIZE 49
 #  endif
 #  define _LIBUNWIND_HIGHEST_DWARF_REGISTER _LIBUNWIND_HIGHEST_DWARF_REGISTER_ARM
 # elif defined(__or1k__)
 #  define _LIBUNWIND_TARGET_OR1K 1
 #  define _LIBUNWIND_CONTEXT_SIZE 16
 #  define _LIBUNWIND_CURSOR_SIZE 24
 #  define _LIBUNWIND_HIGHEST_DWARF_REGISTER _LIBUNWIND_HIGHEST_DWARF_REGISTER_OR1K
 # elif defined(__mips__)
 #  if defined(_ABIO32) && defined(__mips_soft_float)
 #    define _LIBUNWIND_TARGET_MIPS_O32 1
 #    define _LIBUNWIND_CONTEXT_SIZE 18
 #    define _LIBUNWIND_CURSOR_SIZE 24
+#  elif defined(_ABIN32) && defined(__mips_soft_float)
+#    define _LIBUNWIND_TARGET_MIPS_NEWABI 1
+#    define _LIBUNWIND_CONTEXT_SIZE 35
+#    define _LIBUNWIND_CURSOR_SIZE 42
 #  elif defined(_ABI64) && defined(__mips_soft_float)
 #    define _LIBUNWIND_TARGET_MIPS_NEWABI 1
 #    define _LIBUNWIND_CONTEXT_SIZE 35
 #    define _LIBUNWIND_CURSOR_SIZE 47
 #  else
 #    error "Unsupported MIPS ABI and/or environment"
 #  endif
 #  define _LIBUNWIND_HIGHEST_DWARF_REGISTER _LIBUNWIND_HIGHEST_DWARF_REGISTER_MIPS
 # else

src/AddressSpace.hpp

double           getDouble(pint_t addr) {
     return val;
   }
   v128             getVector(pint_t addr) {
     v128 val;
     memcpy(&val, (void *)addr, sizeof(val));
     return val;
   }
   uintptr_t       getP(pint_t addr);
+  uint64_t        getRegister(pint_t addr);
   static uint64_t getULEB128(pint_t &addr, pint_t end);
   static int64_t  getSLEB128(pint_t &addr, pint_t end);
 
   pint_t getEncodedP(pint_t &addr, pint_t end, uint8_t encoding,
                      pint_t datarelBase = 0);
   bool findFunctionName(pint_t addr, char *buf, size_t bufLen,
                         unw_word_t *offset);
   bool findUnwindSections(pint_t targetAddr, UnwindInfoSections &info);
   bool findOtherFDE(pint_t targetAddr, pint_t &fde);
 
   static LocalAddressSpace sThisAddressSpace;
 };
 
 inline uintptr_t LocalAddressSpace::getP(pint_t addr) {
 #if __SIZEOF_POINTER__ == 8
   return get64(addr);
 #else
   return get32(addr);
 #endif
 }
 
+inline uint64_t LocalAddressSpace::getRegister(pint_t addr) {
+#if __SIZEOF_POINTER__ == 8 || (defined(__mips__) && defined(_ABIN32))
+  return get64(addr);
+#else
+  return get32(addr);
+#endif
+}
+
 /// Read a ULEB128 into a 64-bit word.
 inline uint64_t LocalAddressSpace::getULEB128(pint_t &addr, pint_t end) {
   const uint8_t *p = (uint8_t *)addr;
   const uint8_t *pend = (uint8_t *)end;
   uint64_t result = 0;
   int bit = 0;
   do {
     uint64_t b;
public:
 
   typedef typename P::uint_t pint_t;
 
   uint8_t   get8(pint_t addr);
   uint16_t  get16(pint_t addr);
   uint32_t  get32(pint_t addr);
   uint64_t  get64(pint_t addr);
   pint_t    getP(pint_t addr);
+  uint64_t  getRegister(pint_t addr);
   uint64_t  getULEB128(pint_t &addr, pint_t end);
   int64_t   getSLEB128(pint_t &addr, pint_t end);
   pint_t    getEncodedP(pint_t &addr, pint_t end, uint8_t encoding,
                         pint_t datarelBase = 0);
   bool      findFunctionName(pint_t addr, char *buf, size_t bufLen,
                         unw_word_t *offset);
   bool      findUnwindSections(pint_t targetAddr, UnwindInfoSections &info);
   bool      findOtherFDE(pint_t targetAddr, pint_t &fde);

 }
 
 template <typename P>
 typename P::uint_t RemoteAddressSpace<P>::getP(pint_t addr) {
   return P::getP(*(uint64_t *)localCopy(addr));
 }
 
 template <typename P>
-uint64_t RemoteAddressSpace<P>::getULEB128(pint_t &addr, pint_t end) {
+typename P::uint_t OtherAddressSpace<P>::getRegister(pint_t addr) {
+  return P::getRegister(*(uint64_t *)localCopy(addr));
+}
+
+template <typename P>
+uint64_t OtherAddressSpace<P>::getULEB128(pint_t &addr, pint_t end) {
   uintptr_t size = (end - addr);
   LocalAddressSpace::pint_t laddr = (LocalAddressSpace::pint_t) localCopy(addr);
   LocalAddressSpace::pint_t sladdr = laddr;
   uint64_t result = LocalAddressSpace::getULEB128(laddr, laddr + size);
   addr += (laddr - sladdr);
   return result;
 }
 

src/DwarfInstructions.hpp

 
 
 template <typename A, typename R>
 typename A::pint_t DwarfInstructions<A, R>::getSavedRegister(
     A &addressSpace, const R &registers, pint_t cfa,
     const RegisterLocation &savedReg) {
   switch (savedReg.location) {
   case CFI_Parser<A>::kRegisterInCFA:
-    return addressSpace.getP(cfa + (pint_t)savedReg.value);
+    return addressSpace.getRegister(cfa + (pint_t)savedReg.value);
 
   case CFI_Parser<A>::kRegisterAtExpression:
-    return addressSpace.getP(
+    return addressSpace.getRegister(
         evaluateExpression((pint_t)savedReg.value, addressSpace,
                             registers, cfa));
 
   case CFI_Parser<A>::kRegisterIsExpression:
     return evaluateExpression((pint_t)savedReg.value, addressSpace,
                               registers, cfa);
 
   case CFI_Parser<A>::kRegisterInRegister:

src/UnwindRegistersRestore.S

DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind18Registers_mips_o326jumptoEv)
   lw    $30, (4 * 30)($4)
   // load new pc into ra
   lw    $31, (4 * 32)($4)
   // jump to ra, load a0 in the delay slot
   jr    $31
   lw    $4, (4 * 4)($4)
   .set pop
 
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32)) &&            \
+    defined(__mips_soft_float)
 
 //
 // void libunwind::Registers_mips_newabi::jumpto()
 //
 // On entry:
 //  thread state pointer is in a0 ($4)
 //
 DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind21Registers_mips_newabi6jumptoEv)

src/UnwindRegistersSave.S

DEFINE_LIBUNWIND_FUNCTION(unw_getcontext)
   sw    $8,  (4 * 33)($4)
   mflo  $8
   sw    $8,  (4 * 34)($4)
   jr  $31
   # return UNW_ESUCCESS
   or    $2, $0, $0
   .set pop
 
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32)) &&            \
+    defined(__mips_soft_float)
 
 #
 # extern int unw_getcontext(unw_context_t* thread_state)
 #
 # On entry:
 #  thread_state pointer is in a0 ($4)
 #
 DEFINE_LIBUNWIND_FUNCTION(unw_getcontext)

src/libunwind.cpp

 #elif defined(__aarch64__)
 # define REGISTER_KIND Registers_arm64
 #elif defined(__arm__)
 # define REGISTER_KIND Registers_arm
 #elif defined(__or1k__)
 # define REGISTER_KIND Registers_or1k
 #elif defined(__mips__) && defined(_ABIO32) && defined(__mips_soft_float)
 # 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)
 # define REGISTER_KIND Registers_mips_newabi
 #elif defined(__mips__)
 # warning The MIPS architecture is not supported with this ABI and environment!
 #else
 # error Architecture not supported
 #endif
   // Use "placement new" to allocate UnwindCursor in the cursor buffer.
   new ((void *)cursor) UnwindCursor<LocalAddressSpace, REGISTER_KIND>(
                                  context, LocalAddressSpace::sThisAddressSpace);
 #undef REGISTER_KIND