[compiler-rt] lib/tsan/go: add mechanism to skip frames (PR #174049)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Keith Randall (randall77)

<details>
<summary>Changes</summary>

Some Go frames are wrapper functions that we don't want to report to users. Add a mechanism for the Go runtime to tell the tsan runtime not to report those frames.

For https://github.com/golang/go/issues/73915

@<!-- -->dvyukov 

---
Full diff: https://github.com/llvm/llvm-project/pull/174049.diff


4 Files Affected:

- (modified) compiler-rt/lib/tsan/go/tsan_go.cpp (+35-15) 
- (modified) compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp (+7-1) 
- (modified) compiler-rt/lib/tsan/rtl/tsan_symbolize.cpp (+1-1) 
- (modified) compiler-rt/lib/tsan/rtl/tsan_symbolize.h (+1-1) 


``````````diff
diff --git a/compiler-rt/lib/tsan/go/tsan_go.cpp b/compiler-rt/lib/tsan/go/tsan_go.cpp
index c689a51fb5e1d..c987ceacaaea0 100644
--- a/compiler-rt/lib/tsan/go/tsan_go.cpp
+++ b/compiler-rt/lib/tsan/go/tsan_go.cpp
@@ -49,30 +49,50 @@ struct SymbolizeCodeContext {
   uptr res;
 };
 
-SymbolizedStack *SymbolizeCode(uptr addr) {
-  SymbolizedStack *first = SymbolizedStack::New(addr);
-  SymbolizedStack *s = first;
+SymbolizedStack* SymbolizeCode(uptr addr, bool leaf) {
+  // addr is a single physical pc, and we return a set of
+  // virtual frames corresponding to that pc.
+  // Multiple virtual frames are possible because of inlining.
+  // Returned frames are ordered from newest to oldest.
+  //
+  // Note: stack walk order is weird here. Tsan calls this on physical
+  // frames from root to leaf (oldest to youngest frame).
+  // The loop within this function iterates through multiple virtual
+  // frames for a given physical frame. The loop iterates in the
+  // opposite direction, from youngest virtual frame to oldest
+  // virtual frame for the given single physical frame.
+  SymbolizedStack* first = nullptr;
+  SymbolizedStack* s = nullptr;
   for (;;) {
     SymbolizeCodeContext cbctx;
     internal_memset(&cbctx, 0, sizeof(cbctx));
     cbctx.pc = addr;
     go_runtime_cb(CallbackSymbolizeCode, &cbctx);
-    if (cbctx.res == 0)
+    if (cbctx.res == 0)  // error of some sort
       break;
-    AddressInfo &info = s->info;
-    info.module_offset = cbctx.off;
-    info.function = internal_strdup(cbctx.func ? cbctx.func : "??");
-    info.file = internal_strdup(cbctx.file ? cbctx.file : "-");
-    info.line = cbctx.line;
-    info.column = 0;
-
+    if (cbctx.res != 2 || leaf) {
+      // res == 2 means it is a wrapper function we don't want to
+      // display (unless it is the leaf frame).
+      if (first == nullptr) {
+        first = SymbolizedStack::New(addr);
+        s = first;
+      } else {
+        // Allocate a stack entry for the parent of the inlined function.
+        SymbolizedStack* s2 = SymbolizedStack::New(addr);
+        s->next = s2;
+        s = s2;
+      }
+      AddressInfo& info = s->info;
+      info.module_offset = cbctx.off;
+      info.function = internal_strdup(cbctx.func ? cbctx.func : "??");
+      info.file = internal_strdup(cbctx.file ? cbctx.file : "-");
+      info.line = cbctx.line;
+      info.column = 0;
+    }
     if (cbctx.pc == addr) // outermost (non-inlined) function
       break;
     addr = cbctx.pc;
-    // Allocate a stack entry for the parent of the inlined function.
-    SymbolizedStack *s2 = SymbolizedStack::New(addr);
-    s->next = s2;
-    s = s2;
+    leaf = false;
   }
   return first;
 }
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
index 43aef30d2f3b3..4e58305b582d5 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
@@ -111,7 +111,13 @@ static ReportStack *SymbolizeStack(StackTrace trace) {
     // instruction.
     if ((pc & kExternalPCBit) == 0)
       pc1 = StackTrace::GetPreviousInstructionPc(pc);
-    SymbolizedStack *ent = SymbolizeCode(pc1);
+    SymbolizedStack* ent = SymbolizeCode(pc1, si == trace.size - 1);
+#if SANITIZER_GO
+    if (ent == nullptr) {
+      // Go might have 0 frames for this PC (wrapper frames aren't reported).
+      continue;
+    }
+#endif
     CHECK_NE(ent, 0);
     SymbolizedStack *last = ent;
     while (last->next) {
diff --git a/compiler-rt/lib/tsan/rtl/tsan_symbolize.cpp b/compiler-rt/lib/tsan/rtl/tsan_symbolize.cpp
index 2e2744d2eae78..b382b63247e8e 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_symbolize.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_symbolize.cpp
@@ -79,7 +79,7 @@ static void AddFrame(void *ctx, const char *function_name, const char *file,
   info->column = column;
 }
 
-SymbolizedStack *SymbolizeCode(uptr addr) {
+SymbolizedStack* SymbolizeCode(uptr addr, bool leaf) {
   // Check if PC comes from non-native land.
   if (addr & kExternalPCBit) {
     SymbolizedStackBuilder ssb = {nullptr, nullptr, addr};
diff --git a/compiler-rt/lib/tsan/rtl/tsan_symbolize.h b/compiler-rt/lib/tsan/rtl/tsan_symbolize.h
index 7adaa04dc273e..2fe34f00a32fc 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_symbolize.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_symbolize.h
@@ -19,7 +19,7 @@ namespace __tsan {
 
 void EnterSymbolizer();
 void ExitSymbolizer();
-SymbolizedStack *SymbolizeCode(uptr addr);
+SymbolizedStack* SymbolizeCode(uptr addr, bool leaf);
 ReportLocation *SymbolizeData(uptr addr);
 void SymbolizeFlush();
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/174049