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



git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@328079 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/sanitizer_common/sanitizer_common.h b/lib/sanitizer_common/sanitizer_common.h
index 0a92e77..dc78974 100644
--- a/lib/sanitizer_common/sanitizer_common.h
+++ b/lib/sanitizer_common/sanitizer_common.h
@@ -48,7 +48,7 @@
 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/lib/tsan/rtl/tsan_rtl_report.cc b/lib/tsan/rtl/tsan_rtl_report.cc
index cc582ab..febb6ce 100644
--- a/lib/tsan/rtl/tsan_rtl_report.cc
+++ b/lib/tsan/rtl/tsan_rtl_report.cc
@@ -649,8 +649,8 @@
     // 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/lib/tsan/rtl/tsan_symbolize.cc b/lib/tsan/rtl/tsan_symbolize.cc
index b242395..de50c4e 100644
--- a/lib/tsan/rtl/tsan_symbolize.cc
+++ b/lib/tsan/rtl/tsan_symbolize.cc
@@ -36,6 +36,7 @@
   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 @@
   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/test/tsan/java_symbolization.cc b/test/tsan/java_symbolization.cc
index aa5ec0c..f82bd5e 100644
--- a/test/tsan/java_symbolization.cc
+++ b/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 @@
 }
 
 // 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/test/tsan/java_symbolization_legacy.cc b/test/tsan/java_symbolization_legacy.cc
new file mode 100644
index 0000000..aa5ec0c
--- /dev/null
+++ b/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