[tsan] Implement fast unwinding in __sanitizer_print_stack_trace + cleanup.

[Dor1s]

Summary

Please take a look.

[Dor1s]

test/tsan/print_stack_trace.cc

5

Self-nit: no need to recompile with the same options.

[Dor1s]

lib/sanitizer_common/sanitizer_stacktrace.h

33 ↗

(On Diff #97669)

It looks like I got confused and removing this SANITIZER_CAN_SLOW_UNWIND stuff is wrong. Though maybe I can implement it as well?

[eugenis]

lib/sanitizer_common/sanitizer_stacktrace.h

33 ↗

(On Diff #97669)

Yeah, sorry I've missed this. I assume this is here for a reason.

lib/tsan/rtl/tsan_rtl_report.cc

698

why??

700

just use std::swap()

[Dor1s]

This version doesn't break anything.

[Dor1s]

lib/tsan/rtl/tsan_rtl_report.cc

698

Good catch! My previous patchset used this function twice, but now there is not need to have that. Removed.

700

Looks like this code does not use STL intentionally. If I add it, the following error happens, for example:

In file included from <...>/llvm/src/llvm/projects/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc:29:
In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/algorithm:628:
In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/memory:603:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/new:158:40: error: redefinition of 'operator new'
inline _LIBCPP_INLINE_VISIBILITY void* operator new  (std::size_t, void* __p) _NOEXCEPT {return __p;}
                                       ^

[Dor1s]

Could you please take a look or suggest other reviewers?

[eugenis]

lib/tsan/rtl/tsan_rtl_report.cc

741

Why do you need this case at all? PrintStackTrace call below should take care of it. See the implementation of the same function in ubsan.

747

ThreadState::stk_addr
ThreadState::stk_size

lib/tsan/rtl/tsan_rtl_report.cc

 }
 
 void PrintCurrentStack(ThreadState *thr, uptr pc) {
   VarSizeStackTrace trace;
   ObtainCurrentStack(thr, pc, &trace);
   PrintStack(SymbolizeStack(trace));
 }
 
-// Always inlining PrintCurrentStackSlow, because LocatePcInTrace assumes
-// __sanitizer_print_stack_trace exists in the actual unwinded stack, but
-// tail-call to PrintCurrentStackSlow breaks this assumption because
-// __sanitizer_print_stack_trace disappears after tail-call.
-// However, this solution is not reliable enough, please see dvyukov's comment
-// http://reviews.llvm.org/D19148#406208
-// Also see PR27280 comment 2 and 3 for breaking examples and analysis.
-ALWAYS_INLINE
-void PrintCurrentStackSlow(uptr pc) {
 #if !SANITIZER_GO
+static inline void PrintStackTrace(u32 max_depth, uptr pc, uptr bp,
+                                   void *context, uptr stack_top,
+                                   uptr stack_bottom,
+                                   bool request_fast_unwind) {
   BufferedStackTrace *ptrace =
       new(internal_alloc(MBlockStackTrace, sizeof(BufferedStackTrace)))
           BufferedStackTrace();
-  ptrace->Unwind(kStackTraceMax, pc, 0, 0, 0, 0, false);
+  ptrace->Unwind(max_depth, pc, bp, context, stack_top, stack_bottom,
+                 request_fast_unwind);
   for (uptr i = 0; i < ptrace->size / 2; i++) {
     uptr tmp = ptrace->trace_buffer[i];
     ptrace->trace_buffer[i] = ptrace->trace_buffer[ptrace->size - i - 1];
     ptrace->trace_buffer[ptrace->size - i - 1] = tmp;
   }
   PrintStack(SymbolizeStack(*ptrace));
+}
+#endif
+
+// Always inlining PrintCurrentStackSlow, because LocatePcInTrace assumes
+// __sanitizer_print_stack_trace exists in the actual unwinded stack, but
+// tail-call to PrintCurrentStackSlow breaks this assumption because
+// __sanitizer_print_stack_trace disappears after tail-call.
+// However, this solution is not reliable enough, please see dvyukov's comment
+// http://reviews.llvm.org/D19148#406208
+// Also see PR27280 comment 2 and 3 for breaking examples and analysis.
+ALWAYS_INLINE
+void PrintCurrentStackSlow(uptr pc) {
+#if !SANITIZER_GO
+  PrintStackTrace(kStackTraceMax, pc, 0, nullptr, 0, 0, false);
 #endif
 }
 
 }  // namespace __tsan
 
 using namespace __tsan;
 
 extern "C" {
 SANITIZER_INTERFACE_ATTRIBUTE
 void __sanitizer_print_stack_trace() {
-  PrintCurrentStackSlow(StackTrace::GetCurrentPc());
+#if !SANITIZER_GO
+  uptr pc = StackTrace::GetCurrentPc();
+  bool request_fast_unwind = common_flags()->fast_unwind_on_fatal;
+  if (!request_fast_unwind) {
+    PrintCurrentStackSlow(pc);
+    return;
+  }
+
+  uptr top = 0;
+  uptr bottom = 0;
+  __sanitizer::GetThreadStackTopAndBottom(false, &top, &bottom);
+  uptr bp = GET_CURRENT_FRAME();
+  PrintStackTrace(kStackTraceMax, pc, bp, nullptr, top, bottom,
+                  request_fast_unwind);
+#endif
 }
 }  // extern "C"

test/tsan/print_stack_trace.cc

+// Test for __sanitizer_print_stack_trace implementation.
+
+// RUN: %clangxx_tsan -O0 %s -o %t
+// RUN: TSAN_OPTIONS=stack_trace_format=DEFAULT:fast_unwind_on_fatal=0 %run %t 2>&1 | FileCheck %s
+// RUN: TSAN_OPTIONS=stack_trace_format=DEFAULT:fast_unwind_on_fatal=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-FAST-UNWIND
+
+#include <sanitizer/common_interface_defs.h>
+
+static inline void FooBarBaz() {
+  __sanitizer_print_stack_trace();
+}
+
+int main() {
+  FooBarBaz();
+  return 0;
+}
+
+// CHECK: {{.*}} in __sanitizer_print_stack_trace{{.*}}
+// CHECK-FAST-UNWIND: {{.*}} in FooBarBaz{{.*}}print_stack_trace.cc:[[@LINE-9]]
+// CHECK-FAST-UNWIND: {{.*}} in main{{.*}}print_stack_trace.cc:[[@LINE-6]]