tsan: support inlined frames in external symbolization

New API passes a callback function to the external symbolizer,
allowing it to add multiple frames to the traceback. Note that
the old interface API will be still supported until the clients
migrate to the new one.

Author: asmundak (Alexander Smundak)
Reviewed in: https://reviews.llvm.org/D44714

llvm-svn: 328079

5 files changed, 96 insertions, 14 deletions

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
index 0a92e776898..dc789741224 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
@@ -48,7 +48,7 @@ const uptr kMaxThreadStackSize = 1 << 30;  // 1Gb
 static const uptr kErrorMessageBufferSize = 1 << 16;
 
 // Denotes fake PC values that come from JIT/JAVA/etc.
-// For such PC values __tsan_symbolize_external() will be called.
+// For such PC values __tsan_symbolize_external_ex() will be called.
 const u64 kExternalPCBit = 1ULL << 60;
 
 extern const char *SanitizerToolName;  // Can be changed by the tool.
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc
index cc582ab5019..febb6cef2d3 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc
@@ -649,8 +649,8 @@ void ReportRace(ThreadState *thr) {
     // callback. Most likely, TraceTopPC will now return a EventTypeFuncExit
     // event. Later we subtract -1 from it (in GetPreviousInstructionPc)
     // and the resulting PC has kExternalPCBit set, so we pass it to
-    // __tsan_symbolize_external. __tsan_symbolize_external is within its rights
-    // to crash since the PC is completely bogus.
+    // __tsan_symbolize_external_ex. __tsan_symbolize_external_ex is within its
+    // rights to crash since the PC is completely bogus.
     // test/tsan/double_race.cc contains a test case for this.
     toppc = 0;
   }
diff --git a/compiler-rt/lib/tsan/rtl/tsan_symbolize.cc b/compiler-rt/lib/tsan/rtl/tsan_symbolize.cc
index b2423951795..de50c4eb386 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_symbolize.cc
+++ b/compiler-rt/lib/tsan/rtl/tsan_symbolize.cc
@@ -36,6 +36,7 @@ void ExitSymbolizer() {
   thr->ignore_interceptors--;
 }
 
+// Legacy API.
 // May be overriden by JIT/JAVA/etc,
 // whatever produces PCs marked with kExternalPCBit.
 SANITIZER_WEAK_DEFAULT_IMPL
@@ -45,9 +46,50 @@ bool __tsan_symbolize_external(uptr pc, char *func_buf, uptr func_siz,
   return false;
 }
 
+// New API: call __tsan_symbolize_external_ex only when it exists.
+// Once old clients are gone, provide dummy implementation.
+extern "C" SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
+void __tsan_symbolize_external_ex(uptr pc,
+                                  void (*add_frame)(void *, const char *,
+                                                    const char *, int, int),
+                                  void *ctx);
+
+struct SymbolizedStackBuilder {
+  SymbolizedStack *head;
+  SymbolizedStack *tail;
+  uptr addr;
+};
+
+static void AddFrame(void *ctx, const char *function_name, const char *file,
+                     int line, int column) {
+  SymbolizedStackBuilder *ssb = (struct SymbolizedStackBuilder *)ctx;
+  if (ssb->tail) {
+    ssb->tail->next = SymbolizedStack::New(ssb->addr);
+    ssb->tail = ssb->tail->next;
+  } else {
+    ssb->head = ssb->tail = SymbolizedStack::New(ssb->addr);
+  }
+  AddressInfo *info = &ssb->tail->info;
+  if (function_name) {
+    info->function = internal_strdup(function_name);
+  }
+  if (file) {
+    info->file = internal_strdup(file);
+  }
+  info->line = line;
+  info->column = column;
+}
+
 SymbolizedStack *SymbolizeCode(uptr addr) {
   // Check if PC comes from non-native land.
   if (addr & kExternalPCBit) {
+    if (__tsan_symbolize_external_ex) {
+      SymbolizedStackBuilder ssb = {nullptr, nullptr, addr};
+      __tsan_symbolize_external_ex(addr, AddFrame, &ssb);
+      return ssb.head ? ssb.head : SymbolizedStack::New(addr);
+    }
+    // Legacy code: remove along with the declaration above
+    // once all clients using this API are gone.
     // Declare static to not consume too much stack space.
     // We symbolize reports in a single thread, so this is fine.
     static char func_buf[1024];
diff --git a/compiler-rt/test/tsan/java_symbolization.cc b/compiler-rt/test/tsan/java_symbolization.cc
index aa5ec0c3755..f82bd5ead48 100644
--- a/compiler-rt/test/tsan/java_symbolization.cc
+++ b/compiler-rt/test/tsan/java_symbolization.cc
@@ -2,18 +2,13 @@
 #include "java.h"
 #include <memory.h>
 
-extern "C" bool __tsan_symbolize_external(jptr pc,
-                                          char *func_buf, jptr func_siz,
-                                          char *file_buf, jptr file_siz,
-                                          int *line, int *col) {
+extern "C" void __tsan_symbolize_external_ex(
+    jptr pc, void (*add_frame)(void *, const char *, const char *, int, int),
+    void *ctx) {
   if (pc == (1234 | kExternalPCBit)) {
-    memcpy(func_buf, "MyFunc", sizeof("MyFunc"));
-    memcpy(file_buf, "MyFile.java", sizeof("MyFile.java"));
-    *line = 1234;
-    *col = 56;
-    return true;
+    add_frame(ctx, "MyInnerFunc", "MyInnerFile.java", 1234, 56);
+    add_frame(ctx, "MyOuterFunc", "MyOuterFile.java", 4321, 65);
   }
-  return false;
 }
 
 void *Thread(void *p) {
@@ -40,5 +35,6 @@ int main() {
 }
 
 // CHECK: WARNING: ThreadSanitizer: data race
-// CHECK:     #0 MyFunc MyFile.java:1234:56
+// CHECK:     #0 MyInnerFunc MyInnerFile.java:1234:56
+// CHECK:     #1 MyOuterFunc MyOuterFile.java:4321:65
 // CHECK: DONE
diff --git a/compiler-rt/test/tsan/java_symbolization_legacy.cc b/compiler-rt/test/tsan/java_symbolization_legacy.cc
new file mode 100644
index 00000000000..aa5ec0c3755
--- /dev/null
+++ b/compiler-rt/test/tsan/java_symbolization_legacy.cc
@@ -0,0 +1,44 @@
+// RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t | FileCheck %s
+#include "java.h"
+#include <memory.h>
+
+extern "C" bool __tsan_symbolize_external(jptr pc,
+                                          char *func_buf, jptr func_siz,
+                                          char *file_buf, jptr file_siz,
+                                          int *line, int *col) {
+  if (pc == (1234 | kExternalPCBit)) {
+    memcpy(func_buf, "MyFunc", sizeof("MyFunc"));
+    memcpy(file_buf, "MyFile.java", sizeof("MyFile.java"));
+    *line = 1234;
+    *col = 56;
+    return true;
+  }
+  return false;
+}
+
+void *Thread(void *p) {
+  barrier_wait(&barrier);
+  __tsan_write1_pc((jptr)p, 1234 | kExternalPCBit);
+  return 0;
+}
+
+int main() {
+  barrier_init(&barrier, 2);
+  int const kHeapSize = 1024 * 1024;
+  jptr jheap = (jptr)malloc(kHeapSize + 8) + 8;
+  __tsan_java_init(jheap, kHeapSize);
+  const int kBlockSize = 16;
+  __tsan_java_alloc(jheap, kBlockSize);
+  pthread_t th;
+  pthread_create(&th, 0, Thread, (void*)jheap);
+  __tsan_write1_pc((jptr)jheap, 1234 | kExternalPCBit);
+  barrier_wait(&barrier);
+  pthread_join(th, 0);
+  __tsan_java_free(jheap, kBlockSize);
+  fprintf(stderr, "DONE\n");
+  return __tsan_java_fini();
+}
+
+// CHECK: WARNING: ThreadSanitizer: data race
+// CHECK:     #0 MyFunc MyFile.java:1234:56
+// CHECK: DONE