[sanitizer_common] Fix UnwindFast on SPARC

[rorth]

  UBSan-Standalone-sparc :: TestCases/Misc/Linux/diag-stacktrace.cpp

FAILs on 32 and 64-bit Linux/sparc64 (and on Solaris/sparcv9, too: the test isn't Linux-specific at all). With
UBSAN_OPTIONS=fast_unwind_on_fatal=1, the stack trace shows a duplicate innermost frame:

compiler-rt/test/ubsan/TestCases/Misc/Linux/diag-stacktrace.cpp:14:31: runtime error: execution reached the end of a value-returning function without returning a value
    #0 0x7003a708 in f() compiler-rt/test/ubsan/TestCases/Misc/Linux/diag-stacktrace.cpp:14:35
    #1 0x7003a708 in f() compiler-rt/test/ubsan/TestCases/Misc/Linux/diag-stacktrace.cpp:14:35
    #2 0x7003a714 in g() compiler-rt/test/ubsan/TestCases/Misc/Linux/diag-stacktrace.cpp:17:38

which isn't seen with fast_unwind_on_fatal=0.

This turns out to be another fallout from fixing
__builtin_return_address/__builtin_extract_return_addr on SPARC. In sanitizer_stacktrace_sparc.cpp (BufferedStackTrace::UnwindFast) the pc arg is the return address, while pc1 from the stack frame (fr_savpc) is the address of the call insn, leading to a double entry for the innermost frame in trace_buffer[].

This patch fixes this by moving the adjustment before all uses.

Tested on sparc64-unknown-linux-gnu and sparcv9-sun-solaris2.11 (with the ubsan/TestCases/Misc/Linux tests enabled).

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Rainer Orth (rorth)

Changes

  UBSan-Standalone-sparc :: TestCases/Misc/Linux/diag-stacktrace.cpp

FAILs on 32 and 64-bit Linux/sparc64 (and on Solaris/sparcv9, too: the test isn't Linux-specific at all). With
UBSAN_OPTIONS=fast_unwind_on_fatal=1, the stack trace shows a duplicate innermost frame:

compiler-rt/test/ubsan/TestCases/Misc/Linux/diag-stacktrace.cpp:14:31: runtime error: execution reached the end of a value-returning function without returning a value
    #<!-- -->0 0x7003a708 in f() compiler-rt/test/ubsan/TestCases/Misc/Linux/diag-stacktrace.cpp:14:35
    #<!-- -->1 0x7003a708 in f() compiler-rt/test/ubsan/TestCases/Misc/Linux/diag-stacktrace.cpp:14:35
    #<!-- -->2 0x7003a714 in g() compiler-rt/test/ubsan/TestCases/Misc/Linux/diag-stacktrace.cpp:17:38

which isn't seen with fast_unwind_on_fatal=0.

This turns out to be another fallout from fixing
__builtin_return_address/__builtin_extract_return_addr on SPARC. In sanitizer_stacktrace_sparc.cpp (BufferedStackTrace::UnwindFast) the pc arg is the return address, while pc1 from the stack frame (fr_savpc) is the address of the call insn, leading to a double entry for the innermost frame in trace_buffer[].

This patch fixes this by moving the adjustment before all uses.

Tested on sparc64-unknown-linux-gnu and sparcv9-sun-solaris2.11 (with the ubsan/TestCases/Misc/Linux tests enabled).

---

Full diff: https://github.com/llvm/llvm-project/pull/101634.diff

1 Files Affected:

• (modified) compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_sparc.cpp (+5-6)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_sparc.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_sparc.cpp
index a2000798a3907..74f435287af3c 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_sparc.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_sparc.cpp
@@ -58,17 +58,16 @@ void BufferedStackTrace::UnwindFast(uptr pc, uptr bp, uptr stack_top,
   // Avoid infinite loop when frame == frame[0] by using frame > prev_frame.
   while (IsValidFrame(bp, stack_top, bottom) && IsAligned(bp, sizeof(uhwptr)) &&
          size < max_depth) {
-    uhwptr pc1 = ((uhwptr *)bp)[15];
+    // %o7 contains the address of the call instruction and not the
+    // return address, so we need to compensate.
+    uhwptr pc1 = GetNextInstructionPc(((uhwptr *)bp)[15]);
     // Let's assume that any pointer in the 0th page is invalid and
     // stop unwinding here.  If we're adding support for a platform
     // where this isn't true, we need to reconsider this check.
     if (pc1 < kPageSize)
       break;
-    if (pc1 != pc) {
-      // %o7 contains the address of the call instruction and not the
-      // return address, so we need to compensate.
-      trace_buffer[size++] = GetNextInstructionPc((uptr)pc1);
-    }
+    if (pc1 != pc)
+      trace_buffer[size++] = pc1;
     bottom = bp;
     bp = (uptr)((uhwptr *)bp)[14] + STACK_BIAS;
   }

[rorth]

@ebotcazou would be the natural reviewer, but for some reason I cannot select him.

[tru]

@ebotcazou would be the natural reviewer, but for some reason I cannot select him.

That's because he's not part of the GitHub organization.

[rorth]

/cherry-pick 3368a32

[llvmbot]

/pull-request #101848