[Sanitizer] On Darwin `__sanitizer_print_stack_trace` only prints topmost frame

[yln]

Summary

In compiler-rt we have the notion of a fast and a slow stack
unwinder. Darwin currently only supports the fast unwinder.

From reading the code, my understanding is that
BufferedStackTrace::Unwind can be called with `bp=0, stack_top=0,
stack_bottom=0, request_fast_unwind=false`. If
request_fast_unwind=true, then we alos need to supply bp, stack_top,
and stack_bottom.

However, BufferedStackTrace::Unwind uses
StackTrace::WillUseFastUnwind which will adapt request_fast_unwind
if the requested unwinder is not supported. On Darwin, the result is
that we don't pass actual values for bp, stack_top, and stack_bottom,
but end up using the fast unwinder. The tests then fail because we only
print the topmost stack frame.

This patch adds a check to WillUseFastUnwind at the point of usage to
avoid the mismatch between request_fast_unwind and what Unwind
actually does. I am also interested in cleaning up the
request_fast_unwind machinery in general so this patch is just the simplest thing
possible so I can enable the tests.

[vsk]

Thanks, LGTM. Could you wait for another +1? It's been a while since I've looked at this code, and I think another review would help make sure this is safe.

[vitalybuka]

LGTM

compiler-rt/test/sanitizer_common/TestCases/symbolize_stack.cc

6

What is wrong with lsan?

[yln]

compiler-rt/test/sanitizer_common/TestCases/symbolize_stack.cc

6

I don't know. I will take a quick look and provide a separate patch if it's easy to fix.
Internally, we don't support LSan, so this isn't really a priority for us.

[yln]

Follow-up patch for LSan: https://reviews.llvm.org/D58358

compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cc

 #if !defined(__sparc__)
 
 // In GCC on ARM bp points to saved lr, not fp, so we should check the next
 // cell in stack to be a saved frame pointer. GetCanonicFrame returns the
 // pointer to saved frame pointer in any case.
 static inline uhwptr *GetCanonicFrame(uptr bp,
                                       uptr stack_top,
                                       uptr stack_bottom) {
+  CHECK_GT(stack_top, stack_bottom);
 #ifdef __arm__
   if (!IsValidFrame(bp, stack_top, stack_bottom)) return 0;
   uhwptr *bp_prev = (uhwptr *)bp;
   if (IsValidFrame((uptr)bp_prev[0], stack_top, stack_bottom)) return bp_prev;
   // The next frame pointer does not look right. This could be a GCC frame, step
   // back by 1 word and try again.
   if (IsValidFrame((uptr)bp_prev[-1], stack_top, stack_bottom))
     return bp_prev - 1;

compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc

 // 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
+  uptr bp = 0;
+  uptr top = 0;
+  uptr bottom = 0;
+  if (__sanitizer::StackTrace::WillUseFastUnwind(false)) {
+    bp = GET_CURRENT_FRAME();
+    __sanitizer::GetThreadStackTopAndBottom(false, &top, &bottom);
+  }
   BufferedStackTrace *ptrace =
       new(internal_alloc(MBlockStackTrace, sizeof(BufferedStackTrace)))
           BufferedStackTrace();
-  ptrace->Unwind(kStackTraceMax, pc, 0, 0, 0, 0, false);
+  ptrace->Unwind(kStackTraceMax, pc, bp, nullptr, top, bottom, false);
   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
 }

compiler-rt/lib/ubsan/ubsan_diag_standalone.cc

 using namespace __ubsan;
 
 extern "C" {
 SANITIZER_INTERFACE_ATTRIBUTE
 void __sanitizer_print_stack_trace() {
   uptr top = 0;
   uptr bottom = 0;
   bool request_fast_unwind = common_flags()->fast_unwind_on_fatal;
-  if (request_fast_unwind)
+  if (__sanitizer::StackTrace::WillUseFastUnwind(request_fast_unwind))
     __sanitizer::GetThreadStackTopAndBottom(false, &top, &bottom);
 
   GET_CURRENT_PC_BP_SP;
   (void)sp;
   BufferedStackTrace stack;
   stack.Unwind(kStackTraceMax, pc, bp, nullptr, top, bottom,
                request_fast_unwind);
   stack.Print();
 }
 } // extern "C"
 
 #endif  // CAN_SANITIZE_UB

compiler-rt/test/sanitizer_common/TestCases/Darwin/print-stack-trace.cc

 // RUN: %clangxx -O0 %s -o %t && %env_tool_opts=stack_trace_format=DEFAULT %run %t 2>&1 | FileCheck %s
 // RUN: %env_tool_opts=stack_trace_format='"frame:%n lineno:%l"' %run %t 2>&1 | FileCheck %s --check-prefix=CUSTOM
 
-// FIXME(dliew): Make this test work with other sanitizers
-// XFAIL: darwin && (lsan || tsan || ubsan)
+// FIXME(dliew): Make this test work on Darwin with LSan
+// XFAIL: darwin && lsan
 
 #include <sanitizer/common_interface_defs.h>
 
 static inline void FooBarBaz() {
   __sanitizer_print_stack_trace();
 }
 
 int main() {

compiler-rt/test/sanitizer_common/TestCases/symbolize_stack.cc

 // RUN: %clangxx -O0 %s -o %t && %run %t 2>&1 | FileCheck %s
 
 // Test that symbolizer does not crash on frame with large function name.
 
-// FIXME(dliew): Make this test work with the other sanitizers.
-// XFAIL: darwin && (lsan || tsan || ubsan)
+// FIXME(dliew): Make this test work on Darwin with LSan
+// XFAIL: darwin && lsan
 
 #include <sanitizer/common_interface_defs.h>
 #include <vector>
 
 template <int N> struct A {
   template <class T> void RecursiveTemplateFunction(const T &t);
 };