This supports the soft-float ABI only and has been tested with both clang
and gcc on FreeBSD.
Details
Diff Detail
- Build Status
Buildable 10756 Build 10756: arc lint + arc unit
Event Timeline
I have not managed to get the libunwind tests to run, but I was able to run test C++ programs on FreeBSD for both O32 and N64 that caught exceptions. The first test program just uses _Unwind_Backtrace() with a callback to print out the PC values, throws an integer whose value is output in a catch clause, and finally throws a structure whose members are output in a catch clause. I have also built and run a larger real-world C++ program which makes extensive use of exceptions (gdb) on both O32 and N64.
If desired, I can split the O32 vs N64 into two separate patches. I also have another patchset for MIPS N32 that I will post in a separate review once this one has been reviewed.
Note: My n64 changes (and, by the look of it, the o32 changes) only supported the soft-float ABI and so need a check for defined(__mips_soft_float), and should still #error if we're not saving the floating point registers. We're currently using soft float for CHERI, I wasn't able to test a hard-float version (though it should be fairly trivial to add).
+CC cfe-commits, -CC llvm-commits.
Some comments inlined.
I've managed to run the test-suite on one of my machines here and I'm seeing 3 failures:
libunwind :: libunwind_01.pass.cpp libunwind :: libunwind_02.pass.cpp libunwind :: unw_getcontext.pass.cpp
This is with a GCC toolchain, 6.3. Did you test with clang?
include/__libunwind_config.h | ||
---|---|---|
54 | Can you avoid using the mips_o32 macro and instead use defined(mips__) && _MIPS_SIM == _ABIO32 ? It appears the mips_o32 is a FreeBSD specific macro. Also, test for mips_soft_float . | |
59 | Likewise "defined(mips) && _MIPS_SIM == _ABI64 && __mips_soft_float" | |
src/UnwindRegistersRestore.S | ||
492 | defined(mips_o32) -> defined(mips__) && _MIPS_SIM == _ABIO32 | |
500 | Prefix this with: .set push .set nomacro Also, the assembler directives should come after the function declaration / defines. | |
539 | .set pop | |
540 | defined(mips) && _MIPS_SIM == _ABI64 | |
550 | See my comments on the o32 assembly. | |
src/UnwindRegistersSave.S | ||
90 | See my previous comments about this preprocessor macro. | |
101 | Wrap this with .set push .set noat .set nomacro | |
120 | and: .set pop | |
121 | Again, see my previous comments on this preprocessor define. | |
132 | Wrap this with .set push .set noat .set nomacro | |
151 | .set pop | |
src/libunwind.cpp | ||
66 | The MIPS architecture is not supported with this ABI and environment! |
I have only tested this (test programs as mentioned earlier) with clang 5.0.0 (with a few patches) on o32 and n64. I am in the process of performing the same tests with GCC 6.3.0. I will also spend some time figuring out how to cross-build libunwind tests and run them inside of a qemu instance.
FYI, I was able to do simple testing (C++ programs throwing exceptions) using GCC 6.3 to compile a FreeBSD userland for both O32 and N64. Still working on cross-compiling LLVM so I can run the tests under qemu.
I keep running into issues trying to cross-build LLVM (it keeps wanting to use /usr/bin/cc or host libraries even though cross-compiling is enabled). I wonder if the failures you saw Simon might be due to https://bugs.llvm.org/show_bug.cgi?id=33858? Could you try applying the patch from D37484 along with this patchset to see if it fixes the test failures for you?
I have tested this on one of my machines after removing the checks for soft float (my debian install doesn't have the necessary headers for soft-float). With the patch you've pointed out and my inline comments addressed (bar the HI / LO register comments), it passes the supplied test suite.
Can you change the checks for ABI from '_MIPS_SIM == $ABI' to 'defined($ABI)'? I mistakenly assumed they were always defined and that _MIPS_SIM was defined to be one of them but they are only defined when that particular ABI is being used.
include/libunwind.h | ||
---|---|---|
584 | Requires HI / LO registers. | |
src/Registers.hpp | ||
2014 | This appears to be missing the HI / LO registers. | |
2039 | FIXME: Hard float, DSP accumulator registers, MSA registers | |
src/UnwindRegistersSave.S | ||
120–122 | Move the last store out of the delay slot and put 'or $2, $zero, $zero' in the delay slot to return UNW_ESUCCESS. | |
155–157 | Move the last store out of the delay slot and put 'or $2, $zero, $zero' in the delay slot to return UNW_ESUCCESS. |
- Check _ABI* rather than _MIPS_SIM.
- Save and restore lo/hi.
- Expand FIXME comment for more missing registers.
- Return UNW_SUCCESS from unw_getcontext().
- Use correct DWARF numbers for hi and lo and put hi first.
- Bump highest DWARF number for hi and lo.
src/UnwindRegistersSave.S | ||
---|---|---|
100 | I was about to ask if that would be better as well. |
include/__libunwind_config.h | ||
---|---|---|
63 | Can we sink the two cases into the __mips__ case? Something like: #if defined(__mips__) # if defined(_ABIO32) # elif defined(_ABIO64) # elif defined(_ABIN32) # elif defined(_ABI64) # else # error "unknown MIPS ABI" # endif #else | |
src/Registers.hpp | ||
2003 | I believe we have a macro for this. |
- Rebase on more MAX_REGISTER changes.
- Use macro for lastDwarfRegisterNumber.
- Move MIPS ABI constants under a single #ifdef mips.
Just a heads up WRT this patch; we're discussing changing the size of unw_word_t to match uintptr_t in D39365. Does that break anything for your case? It shouldn't affect what's stored in the Register class, only pointers in the unw_proc_info_t struct. Not sure which patch will get completed/merged first though.
Yes, I saw that. It will probably cause some breakage for the cursor size depending on which path (uint64_t always vs uintptr_t). I'm not quite sure which of those approaches is more correct to be honest. I also have an N32 patch in review but will wait until this one is finally committed before updating that further.
Can you avoid using the mips_o32 macro and instead use defined(mips__) && _MIPS_SIM == _ABIO32 ?
It appears the mips_o32 is a FreeBSD specific macro. Also, test for mips_soft_float .