[sanitizer_common] Remove hacks for __builtin_return_address abuse on SPARC

[ro]

Summary

As detailed in Issue #57624, the introduction of __builtin_extract_return_address to GET_CALLER_PC in 4248f32b9ebe87c7af8ee53911efd47c2652f488 broke TestCases/Misc/missing_return.cpp on Solaris/SPARC. Unlike most other targets, the builtin isn't a no-op on SPARC and thus has always been necessary. Its lack had previously been worked around by calls to GetNextInstructionPc in sanitizer_stacktrace_sparc.cpp (BufferedStackTrace::UnwindFast) and sanitizer_unwind_linux_libcdep.cpp (BufferedStackTrace::UnwindSlow). However, those calls are superfluous now and actually harmful.

This patch removes those hacks, fixing the failure.

Tested on sparcv9-sun-solaris2.11 and on sparc-sun-solaris2.11 in the GCC tree. On the latter, several more testcase failures had been caused by this issue since ASan actually works with gcc on SPARC, unlike clang.

[ro]

Ping? It's been almost a week and the bug is a regression from LLVM 16. Thus it would be good to get the patch into the LLVM 17 release. Thanks.

[jrtc27]

Seems reasonable if tested. OOI is there a major reason ASan doesn’t work with Clang other than that the Sparc backend is a bit unloved?

[ro]

In D156504#4557512, @jrtc27 wrote:

Seems reasonable if tested. OOI is there a major reason ASan doesn’t work with Clang other than that the Sparc backend is a bit unloved?

There's SPARC lacks support for over-aligned dynamic alloca. That Issue has been closed, however only the part breaking the test-suite build. has been fixed (or rather avoided by disabling the affected test. The underlying issue is still unfixed.

compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_sparc.cpp

 
 namespace __sanitizer {
 
 void BufferedStackTrace::UnwindFast(uptr pc, uptr bp, uptr stack_top,
                                     uptr stack_bottom, u32 max_depth) {
   // TODO(yln): add arg sanity check for stack_top/stack_bottom
   CHECK_GE(max_depth, 2);
   const uptr kPageSize = GetPageSizeCached();
-#if defined(__GNUC__)
-  // __builtin_return_address returns the address of the call instruction
-  // on the SPARC and not the return address, so we need to compensate.
-  trace_buffer[0] = GetNextInstructionPc(pc);
-#else
   trace_buffer[0] = pc;
-#endif
   size = 1;
   if (stack_top < 4096) return;  // Sanity check for stack top.
   // Flush register windows to memory
 #if defined(__sparc_v9__) || defined(__sparcv9__) || defined(__sparcv9)
   asm volatile("flushw" ::: "memory");
 #else
   asm volatile("ta 3" ::: "memory");
 #endif

compiler-rt/lib/sanitizer_common/sanitizer_unwind_linux_libcdep.cpp

void BufferedStackTrace::UnwindSlow(uptr pc, u32 max_depth) {
   // unless there is only 1 frame in the stack trace (1 frame is always better
   // than 0!).
   // 1-frame stacks don't normally happen, but this depends on the actual
   // unwinder implementation (libgcc, libunwind, etc) which is outside of our
   // control.
   if (to_pop == 0 && size > 1)
     to_pop = 1;
   PopStackFrames(to_pop);
-#if defined(__GNUC__) && defined(__sparc__)
-  // __builtin_return_address returns the address of the call instruction
-  // on the SPARC and not the return address, so we need to compensate.
-  trace_buffer[0] = GetNextInstructionPc(pc);
-#else
   trace_buffer[0] = pc;
-#endif
 }
 
 void BufferedStackTrace::UnwindSlow(uptr pc, void *context, u32 max_depth) {
   CHECK(context);
   CHECK_GE(max_depth, 2);
   if (!unwind_backtrace_signal_arch) {
     UnwindSlow(pc, max_depth);
     return;