[compiler-rt] 711ff37 - [TSan] Make sure we only collect non-TSan frames for memory operations r=dvyukov,rsundahl,thetruestblue,wrotki,kubamracek!

[Julian Lettner via llvm-commits]

Author: Julian Lettner
Date: 2023-03-17T17:40:14-07:00
New Revision: 711ff37b554b2819f5fad02bc263c8044d762a3c

URL: https://github.com/llvm/llvm-project/commit/711ff37b554b2819f5fad02bc263c8044d762a3c
DIFF: https://github.com/llvm/llvm-project/commit/711ff37b554b2819f5fad02bc263c8044d762a3c.diff

LOG: [TSan] Make sure we only collect non-TSan frames for memory operations r=dvyukov,rsundahl,thetruestblue,wrotki,kubamracek!

A previous change [1] moved retrieval of the caller PC
(`__builtin_return_address(0)` via `CALLERPC`) from an
interface-boundary function into a shared helper function
`ExternalAccess`.  If this function does not get inlined, we fail to
collect the appropriate caller PC for the "TSan interface boundary".

[1] https://reviews.llvm.org/D32360

rdar://78489600

Differential Revision: https://reviews.llvm.org/D146264

Added: 
    

Modified: 
    compiler-rt/lib/tsan/rtl/tsan_external.cpp
    compiler-rt/test/tsan/Darwin/external-swift-debugging.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/tsan/rtl/tsan_external.cpp b/compiler-rt/lib/tsan/rtl/tsan_external.cpp
index 19ae174f20a59..463f32d7fdc31 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_external.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_external.cpp
@@ -60,14 +60,15 @@ uptr TagFromShadowStackFrame(uptr pc) {
 
 #if !SANITIZER_GO
 
-void ExternalAccess(void *addr, uptr caller_pc, void *tag, AccessType typ) {
+void ExternalAccess(void *addr, uptr caller_pc, uptr tsan_caller_pc, void *tag,
+                    AccessType typ) {
   CHECK_LT(tag, atomic_load(&used_tags, memory_order_relaxed));
   ThreadState *thr = cur_thread();
   if (caller_pc) FuncEntry(thr, caller_pc);
   InsertShadowStackFrameForTag(thr, (uptr)tag);
   bool in_ignored_lib;
   if (!caller_pc || !libignore()->IsIgnored(caller_pc, &in_ignored_lib))
-    MemoryAccess(thr, CALLERPC, (uptr)addr, 1, typ);
+    MemoryAccess(thr, tsan_caller_pc, (uptr)addr, 1, typ);
   FuncExit(thr);
   if (caller_pc) FuncExit(thr);
 }
@@ -112,12 +113,12 @@ void __tsan_external_assign_tag(void *addr, void *tag) {
 
 SANITIZER_INTERFACE_ATTRIBUTE
 void __tsan_external_read(void *addr, void *caller_pc, void *tag) {
-  ExternalAccess(addr, STRIP_PAC_PC(caller_pc), tag, kAccessRead);
+  ExternalAccess(addr, STRIP_PAC_PC(caller_pc), CALLERPC, tag, kAccessRead);
 }
 
 SANITIZER_INTERFACE_ATTRIBUTE
 void __tsan_external_write(void *addr, void *caller_pc, void *tag) {
-  ExternalAccess(addr, STRIP_PAC_PC(caller_pc), tag, kAccessWrite);
+  ExternalAccess(addr, STRIP_PAC_PC(caller_pc), CALLERPC, tag, kAccessWrite);
 }
 }  // extern "C"
 

diff  --git a/compiler-rt/test/tsan/Darwin/external-swift-debugging.cpp b/compiler-rt/test/tsan/Darwin/external-swift-debugging.cpp
index 72335edf17896..64475a3e97373 100644
--- a/compiler-rt/test/tsan/Darwin/external-swift-debugging.cpp
+++ b/compiler-rt/test/tsan/Darwin/external-swift-debugging.cpp
@@ -1,6 +1,7 @@
 // RUN: %clangxx_tsan %s -o %t
 // RUN: %deflake %run %t 2>&1 | FileCheck %s
 
+#include <dlfcn.h>
 #include <thread>
 
 #import "../test.h"
@@ -13,6 +14,9 @@ int __tsan_get_report_data(void *report, const char **description, int *count,
                            int *unique_tid_count, void **sleep_trace,
                            unsigned long trace_size);
 int __tsan_get_report_tag(void *report, unsigned long *tag);
+int __tsan_get_report_mop(void *report, unsigned long idx, int *tid, void **addr,
+                          int *size, int *write, int *atomic, void **trace,
+                          unsigned long trace_size);
 }
 
 __attribute__((no_sanitize("thread"), noinline))
@@ -25,7 +29,10 @@ int main(int argc, char *argv[]) {
   barrier_init(&barrier, 2);
   fprintf(stderr, "Start.\n");
   // CHECK: Start.
-  
+
+  fprintf(stderr, "ExternalWrite function address: %p\n", &ExternalWrite);
+  // CHECK: ExternalWrite function address: [[ExternalWrite_addr:0x[0-9a-z]+]]
+
   void *opaque_object = malloc(16);
   std::thread t1([opaque_object] {
     ExternalWrite(opaque_object);
@@ -37,7 +44,9 @@ int main(int argc, char *argv[]) {
   });
   // CHECK: WARNING: ThreadSanitizer: Swift access race
   // CHECK: Modifying access of Swift variable at {{.*}} by thread {{.*}}
+  // CHECK:   #0 ExternalWrite
   // CHECK: Previous modifying access of Swift variable at {{.*}} by thread {{.*}}
+  // CHECK:   #0 ExternalWrite
   // CHECK: SUMMARY: ThreadSanitizer: Swift access race
   t1.join();
   t2.join();
@@ -55,13 +64,28 @@ __tsan_on_report(void *report) {
   __tsan_get_report_data(report, &description, &count, &stack_count, &mop_count,
                          &loc_count, &mutex_count, &thread_count,
                          &unique_tid_count, sleep_trace, 16);
-  fprintf(stderr, "report type = '%s', count = %d\n", description, count);
-  // CHECK: report type = 'external-race', count = 0
+  fprintf(stderr, "report type = '%s', count = %d, mop_count = %d\n", description, count, mop_count);
+  // CHECK: report type = 'external-race', count = 0, mop_count = 2
 
   unsigned long tag;
   __tsan_get_report_tag(report, &tag);
   fprintf(stderr, "tag = %ld\n", tag);
   // CHECK: tag = 1
+
+  int tid, size, write, atomic;
+  void *addr;
+  void *trace[16] = {0};
+  __tsan_get_report_mop(report, /*idx=*/0, &tid, &addr, &size, &write, &atomic,
+                        trace, 16);
+  fprintf(stderr, "Racy write trace (1 of 2):\n");
+  for (int i = 0; i < 16 && trace[i]; i++) {
+    Dl_info info;
+    dladdr(trace[i], &info);
+    fprintf(stderr, "  %d: frame: %p, function: %p %s\n", i, trace[i],
+            info.dli_saddr, info.dli_sname);
+  }
+  // Ensure ExternalWrite() function is top of trace
+  // CHECK: 0: frame: 0x{{[0-9a-z]+}}, function: [[ExternalWrite_addr]] _Z13ExternalWritePv
 }
 
 // CHECK: Done.