[compiler-rt] Refactor stacktrace and symbolizing functions from TSan into sanitizer_common

[kubamracek]

Summary

This is a refactor patch that moves a couple of stacktrace taking and/or symbolizer-related functions from TSan, makes them generic and moves them into sanitizer_common. This is to prepare the functions for an upcoming patch implementing issue suppression mechanism into ASan. NFC intented.

There is a couple of subtle changes, that I'd like to make sure we're fine with:

• The #ifdef TSAN_GO changes into #ifdef SANITIZER_GO in the appropriate places.
• The internal_alloc(MBlockReportStack, sizeof(ReportStack)); becomes InternalAlloc(sizeof(ReportStack));, not sure if the MBlockReportStack is used for anything important.
• stack->pc to stack->info.address, how could this event work before???
• bool strip_main argument to SymbolizeStack whether it should try to strip the main function from the stack trace

[samsonov]

Sorry for the delay, I've seen the patch (and the following one, about
supressions), but wasn't able to review it yet.

[samsonov]

Probably, I should have started with your patch for suppression support in ASan to see what you actually need there :) Anyway, it's too late now.
I think that this patch is way too gross and should be significantly modified before it can go in.

1. __tsan::SymbolizeStack() used to call __tsan::SymbolizeCode(), which does its kExternalPCBit magic. But now this function is unused, as __sanitizer::SymbolizeStack() will never call it.
2. stack->pc didn't break because DPrintf() arguments are compiled out of regular build. There's a comment which tells why this should be a DPrintf(), not a Printf(). Note that we don't have DPrint() in sanitizer_common. Thanks for pointing at it, I'll fix this right away.
3. StackStripMain is completely TSan-specific, it has hardcoded Linux function names and even names from TSan runtime.
4. ReportStack::suppressable is TSan-specific, and in retrospect looks like a weird idea - ReportStack represents a single frame stack trace, while "suppressable" is the flag that should be attached to the whole stack trace.

ReportStack probably makes sense for TSan, as it used to be a class in its report generation machinery, but if we want to make it generally usable, we should use smth. like SymbolizedStackFrame.

If we want to run a useful refactoring, I *think* we can try something like this: remove the proliferation of AddressInfo class and gradually replace it with SymbolizedStackFrame. In particular, we can make Symbolizer::SymbolizePC() just return a pointer to a newly-allocated SymbolizedStackFrame object, and save the users from the effort of allocating/deallocating arrays of AddressInfo structures. We will be able to generalize the code between __tsan::SymbolizeCode() and StackTrace::Print() (see sanitizer_stacktrace_libcdep.cc).

Also, it's not a coincidence that we used to have a symbolization inside the function that prints the stacktrace, and it's more or less a design choice of ASan: when we print an error report, a program can be in a terrible state, so we're interested in printing the data as soon as we obtain it. We don't want to spend time and resources trying to symbolize 100-frame stack trace, and then print it all. Instead, we want to have a symbolize-print loop to produce at least some information if the program is killed while we're doing it.

[dvyukov]

s/TSAN_GO/SANITIZER_GO/ is perfectly OK. These macros are just a way to detect that we are building Go runtime. There is no difference between using one or another.

lib/sanitizer_common/sanitizer_symbolizer.h

    public:
     explicit SymbolizerScope(const Symbolizer *sym);
     ~SymbolizerScope();
    private:
     const Symbolizer *sym_;
   };
 };
 
+struct ReportStack {
+  ReportStack *next;
+  AddressInfo info;
+  bool suppressable;
+  static ReportStack *New(uptr addr);
+
+ private:
+  ReportStack();
+};
+
+ReportStack *SymbolizeCode(uptr addr);
+
+ReportStack *SymbolizeStack(StackTrace trace, bool strip_main = true);
+
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_SYMBOLIZER_H

lib/sanitizer_common/sanitizer_symbolizer.cc

 //===-- sanitizer_symbolizer.cc -------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file is shared between AddressSanitizer and ThreadSanitizer
 // run-time libraries.
 //===----------------------------------------------------------------------===//
 
 #include "sanitizer_platform.h"
 #include "sanitizer_internal_defs.h"
 #include "sanitizer_placement_new.h"
+#include "sanitizer_stacktrace.h"
 #include "sanitizer_symbolizer.h"
 
 namespace __sanitizer {
 
 Symbolizer *Symbolizer::symbolizer_;
 StaticSpinMutex Symbolizer::init_mu_;
 LowLevelAllocator Symbolizer::symbolizer_allocator_;
 
if (sym_->start_hook_)
     sym_->start_hook_();
 }
 
 Symbolizer::SymbolizerScope::~SymbolizerScope() {
   if (sym_->end_hook_)
     sym_->end_hook_();
 }
 
+ReportStack::ReportStack() : next(nullptr), info(), suppressable(false) {}
+
+ReportStack *ReportStack::New(uptr addr) {
+  void *mem = InternalAlloc(sizeof(ReportStack));
+  ReportStack *res = new(mem) ReportStack();
+  res->info.address = addr;
+  return res;
+}
+
+ReportStack *SymbolizeCode(uptr addr) {
+  static const uptr kMaxAddrFrames = 16;
+  InternalScopedBuffer<AddressInfo> addr_frames(kMaxAddrFrames);
+  for (uptr i = 0; i < kMaxAddrFrames; i++)
+    new(&addr_frames[i]) AddressInfo();
+  uptr addr_frames_num = Symbolizer::GetOrInit()->SymbolizePC(
+      addr, addr_frames.data(), kMaxAddrFrames);
+  if (addr_frames_num == 0)
+    return ReportStack::New(addr);
+  ReportStack *top = 0;
+  ReportStack *bottom = 0;
+  for (uptr i = 0; i < addr_frames_num; i++) {
+    ReportStack *cur_entry = ReportStack::New(addr);
+    cur_entry->info = addr_frames[i];
+    if (i == 0)
+      top = cur_entry;
+    else
+      bottom->next = cur_entry;
+    bottom = cur_entry;
+  }
+  return top;
+}
+
+static void StackStripMain(ReportStack *stack) {
+  ReportStack *last_frame = 0;
+  ReportStack *last_frame2 = 0;
+  for (ReportStack *ent = stack; ent; ent = ent->next) {
+    last_frame2 = last_frame;
+    last_frame = ent;
+  }
+
+  if (last_frame2 == 0)
+    return;
+  const char *last = last_frame->info.function;
+#ifndef SANITIZER_GO
+  const char *last2 = last_frame2->info.function;
+  // Strip frame above 'main'
+  if (last2 && 0 == internal_strcmp(last2, "main")) {
+    last_frame2->next = 0;
+  // Strip our internal thread start routine.
+  } else if (last && 0 == internal_strcmp(last, "__tsan_thread_start_func")) {
+    last_frame2->next = 0;
+  // Strip global ctors init.
+  } else if (last && 0 == internal_strcmp(last, "__do_global_ctors_aux")) {
+    last_frame2->next = 0;
+  // If both are 0, then we probably just failed to symbolize.
+  } else if (last || last2) {
+    // Ensure that we recovered stack completely. Trimmed stack
+    // can actually happen if we do not instrument some code,
+    // so it's only a debug print. However we must try hard to not miss it
+    // due to our fault.
+    Printf("Bottom stack frame of stack %zx is missed\n", stack->info.address);
+  }
+#else
+  // The last frame always point into runtime (gosched0, goexit0, runtime.main).
+  last_frame2->next = 0;
+  (void)last;
+#endif
+}
+
+ReportStack *SymbolizeStack(StackTrace trace, bool strip_main) {
+  if (trace.size == 0)
+    return 0;
+  ReportStack *stack = 0;
+  for (uptr si = 0; si < trace.size; si++) {
+    const uptr pc = trace.trace[si];
+#ifndef SANITIZER_GO
+    // We obtain the return address, that is, address of the next instruction,
+    // so offset it by 1 byte.
+    const uptr pc1 = StackTrace::GetPreviousInstructionPc(pc);
+#else
+    // FIXME(dvyukov): Go sometimes uses address of a function as top pc.
+    uptr pc1 = pc;
+    if (si != trace.size - 1)
+      pc1 -= 1;
+#endif
+    ReportStack *ent = SymbolizeCode(pc1);
+    CHECK_NE(ent, 0);
+    ReportStack *last = ent;
+    while (last->next) {
+      last->info.address = pc;  // restore original pc for report
+      last = last->next;
+    }
+    last->info.address = pc;  // restore original pc for report
+    last->next = stack;
+    stack = ent;
+  }
+  if (strip_main)
+    StackStripMain(stack);
+  return stack;
+}
+
 }  // namespace __sanitizer

lib/tsan/rtl/tsan_report.h

enum ReportType {
   ReportTypeMutexBadUnlock,
   ReportTypeMutexBadReadLock,
   ReportTypeMutexBadReadUnlock,
   ReportTypeSignalUnsafe,
   ReportTypeErrnoInSignal,
   ReportTypeDeadlock
 };
 
-struct ReportStack {
-  ReportStack *next;
-  AddressInfo info;
-  bool suppressable;
-  static ReportStack *New(uptr addr);
-
- private:
-  ReportStack();
-};
-
 struct ReportMopMutex {
   u64 id;
   bool write;
 };
 
 struct ReportMop {
   int tid;
   uptr addr;

lib/tsan/rtl/tsan_report.cc

 #include "tsan_platform.h"
 #include "tsan_rtl.h"
 #include "sanitizer_common/sanitizer_placement_new.h"
 #include "sanitizer_common/sanitizer_report_decorator.h"
 #include "sanitizer_common/sanitizer_stacktrace_printer.h"
 
 namespace __tsan {
 
-ReportStack::ReportStack() : next(nullptr), info(), suppressable(false) {}
-
-ReportStack *ReportStack::New(uptr addr) {
-  void *mem = internal_alloc(MBlockReportStack, sizeof(ReportStack));
-  ReportStack *res = new(mem) ReportStack();
-  res->info.address = addr;
-  return res;
-}
-
 ReportLocation::ReportLocation(ReportLocationType type)
     : type(type), global(), heap_chunk_start(0), heap_chunk_size(0), tid(0),
       fd(0), suppressable(false), stack(nullptr) {}
 
 ReportLocation *ReportLocation::New(ReportLocationType type) {
   void *mem = internal_alloc(MBlockReportStack, sizeof(ReportLocation));
   return new(mem) ReportLocation(type);
 }

lib/tsan/rtl/tsan_rtl_report.cc

 #include "tsan_mman.h"
 #include "tsan_flags.h"
 #include "tsan_fd.h"
 
 namespace __tsan {
 
 using namespace __sanitizer;  // NOLINT
 
-static ReportStack *SymbolizeStack(StackTrace trace);
-
 void TsanCheckFailed(const char *file, int line, const char *cond,
                      u64 v1, u64 v2) {
   // There is high probability that interceptors will check-fail as well,
   // on the other hand there is no sense in processing interceptors
   // since we are going to die soon.
   ScopedIgnoreInterceptors ignore;
   Printf("FATAL: ThreadSanitizer CHECK failed: "
          "%s:%d \"%s\" (0x%zx, 0x%zx)\n",
          file, line, cond, (uptr)v1, (uptr)v2);
   PrintCurrentStackSlow(StackTrace::GetCurrentPc());
   Die();
 }
 
 // Can be overriden by an application/test to intercept reports.
 #ifdef TSAN_EXTERNAL_HOOKS
 bool OnReport(const ReportDesc *rep, bool suppressed);
 #else
 SANITIZER_INTERFACE_ATTRIBUTE
 bool WEAK OnReport(const ReportDesc *rep, bool suppressed) {
   (void)rep;
   return suppressed;
 }
 #endif
 
-static void StackStripMain(ReportStack *stack) {
-  ReportStack *last_frame = 0;
-  ReportStack *last_frame2 = 0;
-  for (ReportStack *ent = stack; ent; ent = ent->next) {
-    last_frame2 = last_frame;
-    last_frame = ent;
-  }
-
-  if (last_frame2 == 0)
-    return;
-  const char *last = last_frame->info.function;
-#ifndef TSAN_GO
-  const char *last2 = last_frame2->info.function;
-  // Strip frame above 'main'
-  if (last2 && 0 == internal_strcmp(last2, "main")) {
-    last_frame2->next = 0;
-  // Strip our internal thread start routine.
-  } else if (last && 0 == internal_strcmp(last, "__tsan_thread_start_func")) {
-    last_frame2->next = 0;
-  // Strip global ctors init.
-  } else if (last && 0 == internal_strcmp(last, "__do_global_ctors_aux")) {
-    last_frame2->next = 0;
-  // If both are 0, then we probably just failed to symbolize.
-  } else if (last || last2) {
-    // Ensure that we recovered stack completely. Trimmed stack
-    // can actually happen if we do not instrument some code,
-    // so it's only a debug print. However we must try hard to not miss it
-    // due to our fault.
-    DPrintf("Bottom stack frame of stack %zx is missed\n", stack->pc);
-  }
-#else
-  // The last frame always point into runtime (gosched0, goexit0, runtime.main).
-  last_frame2->next = 0;
-  (void)last;
-#endif
-}
-
 ReportStack *SymbolizeStackId(u32 stack_id) {
   if (stack_id == 0)
     return 0;
   StackTrace stack = StackDepotGet(stack_id);
   if (stack.trace == nullptr)
     return nullptr;
   return SymbolizeStack(stack);
 }
 
-static ReportStack *SymbolizeStack(StackTrace trace) {
-  if (trace.size == 0)
-    return 0;
-  ReportStack *stack = 0;
-  for (uptr si = 0; si < trace.size; si++) {
-    const uptr pc = trace.trace[si];
-#ifndef TSAN_GO
-    // We obtain the return address, that is, address of the next instruction,
-    // so offset it by 1 byte.
-    const uptr pc1 = StackTrace::GetPreviousInstructionPc(pc);
-#else
-    // FIXME(dvyukov): Go sometimes uses address of a function as top pc.
-    uptr pc1 = pc;
-    if (si != trace.size - 1)
-      pc1 -= 1;
-#endif
-    ReportStack *ent = SymbolizeCode(pc1);
-    CHECK_NE(ent, 0);
-    ReportStack *last = ent;
-    while (last->next) {
-      last->info.address = pc;  // restore original pc for report
-      last = last->next;
-    }
-    last->info.address = pc;  // restore original pc for report
-    last->next = stack;
-    stack = ent;
-  }
-  StackStripMain(stack);
-  return stack;
-}
-
 ScopedReport::ScopedReport(ReportType typ) {
   ctx->thread_registry->CheckLocked();
   void *mem = internal_alloc(MBlockReport, sizeof(ReportDesc));
   rep_ = new(mem) ReportDesc;
   rep_->typ = typ;
   ctx->report_mtx.Lock();
   CommonSanitizerReportMutex.Lock();
 }

lib/tsan/rtl/tsan_symbolize.cc

if (!__tsan_symbolize_external(addr, func_buf, sizeof(func_buf),
                                   file_buf, sizeof(file_buf), &line, &col))
       return ent;
     ent->info.function = internal_strdup(func_buf);
     ent->info.file = internal_strdup(file_buf);
     ent->info.line = line;
     ent->info.column = col;
     return ent;
   }
-  static const uptr kMaxAddrFrames = 16;
-  InternalScopedBuffer<AddressInfo> addr_frames(kMaxAddrFrames);
-  for (uptr i = 0; i < kMaxAddrFrames; i++)
-    new(&addr_frames[i]) AddressInfo();
-  uptr addr_frames_num = Symbolizer::GetOrInit()->SymbolizePC(
-      addr, addr_frames.data(), kMaxAddrFrames);
-  if (addr_frames_num == 0)
-    return ReportStack::New(addr);
-  ReportStack *top = 0;
-  ReportStack *bottom = 0;
-  for (uptr i = 0; i < addr_frames_num; i++) {
-    ReportStack *cur_entry = ReportStack::New(addr);
-    cur_entry->info = addr_frames[i];
-    if (i == 0)
-      top = cur_entry;
-    else
-      bottom->next = cur_entry;
-    bottom = cur_entry;
-  }
-  return top;
+  return __sanitizer::SymbolizeCode(addr);
 }
 
 ReportLocation *SymbolizeData(uptr addr) {
   DataInfo info;
   if (!Symbolizer::GetOrInit()->SymbolizeData(addr, &info))
     return 0;
   ReportLocation *ent = ReportLocation::New(ReportLocationGlobal);
   ent->global = info;
   return ent;
 }
 
 void SymbolizeFlush() {
   Symbolizer::GetOrInit()->Flush();
 }
 
 }  // namespace __tsan