Adjust formatting.

Rebase

• Add tests.

I ran into this error in a perhaps unusual environment which is using GCC 12.0 to compile LLVM 14 (included in FreeBSD) using libc++ as the C++ runtime (rather than using libstdc++). The exact error from GCC is included in the review I opened for FreeBSD at https://reviews.freebsd.org/D36898. I forward ported the patch from LLVM 14 but do not have an easy way to test the setup I used to reproduce the GCC error on LLVM 14. I did not use __add_pointer_t because line 49 uses add_pointer<>, but if __add_pointer_t is preferred I'm happy to switch it to that instead.

I think this is fine.

For FreeBSD, MAP_ALIGNED_SUPER probably isn't what you want. The default mmap() for MAP_ANON on FreeBSD tries to do what MAP_ALIGNED_SUPER does if the request is at least one large page in size first, and if that fails to find available address space, falls back to finding any possible virtual addresses (it does the two steps you are doing inside of mmap() itself). Specifying MAP_ALIGNED_SUPER tells mmap() to only try the first pass and fail without trying the second pass, but then the second call to mmap() that you would make as a result would just do both passes anyway.

• Add a comment and expand the description in the commit log.
• Rename test from riscv-binary.s to riscv-elf-flags.s

This is needed to link kernel modules for firmware blobs used in a FreeBSD/riscv64 kernel.

This is needed to link a FreeBSD/riscv64 kernel which uses undefined weak symbols in the uart(4) driver.

FYI, this change allowed me to compile and run a hard-float FreeBSD RISC-V userland. (With stock LLVM the build trips over this assertion)

In D69212#1716991, @jrtc27 wrote:

I was not aware of that aside in the spec, which does indeed suggest this specific case should sign-extend, and I guess gives us a reason why this isn't a slippery slope. However, even if this is the case, I still feel code shouldn't be relying on this, simply because it's unclear (and perhaps surprising to those who don't know this particular note from the spec). One might perfectly reasonably assume that both signed and unsigned i32 types (in the original source) are zero-extended, or (probably the default position of people) that the signed type is sign-extended and the unsigned type is zero-extended (which would require passing the extension information through via the asm call instruction's operands).

My only thought is that the RISC-V spec seems to require sign-extending unsigned 32-bit ints when stored in 64-bit registers, so folks may very well assume this behavior in the future when writing inline assembly. The spec doesn't say anything about types other than 32-bits that I found, and the section from the 2.2 user spec (quoted below) is in italics (meaning it's commentary to explain that spec that technically isn't part of the spec itself IIRC which perhaps puts it in a grey area). For FreeBSD it seems a simple cast in the constraint is sufficient to resolve our immediate issue FWIW.

Sorry for the delay. This does still need review from folks familiar with sanitizers. I haven't yet rebased it to a more recent tree due to lack of review feedback. If there is interest in reviewing it I can update it to a newer tree and retest (or if there is feedback on the current patches I'm happy to update them).

The LLVM changes to use the correct shadow offset on FreeBSD/mips64 are in D49939.

Add tests.

AFAICT, I don't see a way to ask clang "which sanitizers are supported by this target". Hmm, the CHECK-SANL-MIPS test in tests/Driver/fsanitize.c seems like the closest match.

FWIW, for FreeBSD I defined __CLANG_MAX_ALIGN_T and _GCC_MAX_ALIGN_T in FreeBSD's <stddef.h> when defining the typedef to handle this.

Ping. I'd like this to go in and get merged into FreeBSD's clang so we can test this for a few months before 12.0 branches.

• Add N32EL.

To be clear, even though FreeBSD does support n32, I think we want this patch for FreeBSD certainly as the forward path for N32 on FreeBSD is to get IAS N32 working rather than other solutions. Also, N64 on FreeBSD needs IAS today, so I'd rather this go in sooner rather than waiting for the N32 IAS issues to be fully resolved.

FWIW, I ended up fixing FreeBSD to only use <machine/stdarg.h> in freestanding environments and always use <stdarg.h> in userland which makes this patch no longer necessary. (Only one place needed to be fixed.)

Ping @sdardis @compnerd

@sdardis ping. I think the approach I've used for O32 is probably the right one in that it matches what DWARF expects (DWARF doesn't treat the 32-bit floating point registers as pairs but as individual registers). I think the question is if I O32 with 32-bit FP registers should use an array of float[]s instead of an array of double[]s though (which adds an extra #ifdef for context size) vs keeping the current layout.

This version follows the suggestion I made earlier of treating 32-bit floating point registers as a "plain" register for O32 rather than a floating-point register. It seems to work for me though for O32 I've only tested with 32-bit floating point registers. If we stick with this approach for 32-bit floating point it might be simpler and/or more readable to make _floats[] an array of uint32_t for that case and just use different context sizes for the O32 with 32-bit fp vs O32 with 64-bit fp. This version works in my testing on FreeBSD for O32 with 32-bit FPR, N32, and N64.

• Add a comment about using a single FP layout for O32.
• Treat 32-bit floating point registers on O32 as plain registers.

• Rebase after N32 commit.
• Use ldc1/sdc1 rather than l.d and s.d.

Hmmm, so I was somewhat mistaken as DwarfInstructions.hpp::stepWithDwarf() does use the Register class's setFloatRegister(), however, it assumes that the floating point register is always a double (DwarfInstructions.hpp::getSavedFloatRegister() uses AddressSpace::getDouble() to read a double from memory). So I think that means that the NEWABI cases work as-is. O32 with 64-bit FP registers should also work as-is. However, O32 with 32-bit FP registers will not quite work. The DWARF info for O32 with 32-bit registers does save/restore individual 32-bit registers:

After thinking about this some more, I need to rework this a bit. The choice of how to expose the floating point registers via getFloatingPointRegister / setFloatingPointRegister only affects consumers of the libunwind unw_get_fpreg/unw_set_fpreg. I think a bigger factor is that we need to honor unwind rules in frames that describe the saved location of FP registers on the stack (which this patchset doesn't do). I think that's fairly trivial for the case where an FP register is the same size as a GP register, and it probably argues for storing 32-bit FP registers as individual 'float' if that is how the unwind information is encoded. I haven't yet looked to see what the implications would be for O32 MIPS with 64-bit FP registers. It may be that I punt on that configuration for now.

• Match names used in libcxx and add LINKER_FLAGS.

• Use STRING instead of PATH.
• Update for committed libcxx change.

My only question is if this should be named LIBUNWIND_TEST_COMPILER_FLAGS to match the name used in libc++?

Obsoleted by rCXX325914.

@sdardis requested this functionality in the review of D39074. Simon, can you confirm that this works for you in your testing?

• Unexpand tabs.

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.

Nice sleuthing!

• Rebase.
• Rework ABI macro checks.

• Add a test case.

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

My only question is if we want an OS version check as the FreeBSD driver does now for libc++ vs libstdc++? FreeBSD started using libcompiler_rt for libgcc.a in 9.0.

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?

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')

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.

• Rebase after N64 -> newabi commit.

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

ping @sdardis, @compnerd

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

Simon, FreeBSD does support n32 as others have noted. Do you have any thoughts on what is needed to allow N32 to work with IAS? It seems like there is some support in MipsAsmParser for N32 IIRC, though it seems like the integrated assembler thinks it is compiling as N64 when -mabi=n32 is passed to clang (my attempt to compile FreeBSD N32 with clang currently dies due to encountering 'la' and erroring since it thinks it is using N64 where 'dla' should be used instead.

• Rebase after O32/N64 commit.

Ping @compnerd, @sdardis

Ping

• Rebase.
• Update O32 cursor size after unw_word_t change.

In D38110#910526, @mstorsjo wrote:

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.

• Rebase on more MAX_REGISTER changes.
• Use macro for lastDwarfRegisterNumber.
• Move MIPS ABI constants under a single #ifdef mips.

• Rebase after MAX_REGISTER change.

• Rebase for recent change to MAX_REGISTER meaning.

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

I think one source of truth is better than two. I do find the constant's value a bit off in general though. That is, the name 'HIGHEST' implies to me that it is the highest value used, not N + 1 as we currently define it.

Ping?

• Save all of the general purpose registers.

FreeBSD/i386 also uses llvm libunwind by default.

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

• Add more soft-float checks.

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?

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