[compiler-rt] 6563bb5 - tsan: don't use caller/current PC in Java interfaces

Dmitry Vyukov via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 28 11:25:26 PDT 2021


Author: Dmitry Vyukov
Date: 2021-07-28T20:25:20+02:00
New Revision: 6563bb53b5fd837a2a0368c58de1cf02ec921d27

URL: https://github.com/llvm/llvm-project/commit/6563bb53b5fd837a2a0368c58de1cf02ec921d27
DIFF: https://github.com/llvm/llvm-project/commit/6563bb53b5fd837a2a0368c58de1cf02ec921d27.diff

LOG: tsan: don't use caller/current PC in Java interfaces

Caller PC is plain harmful as native caller PC has nothing to do with Java code.
Current PC is not particularly useful and may be somewhat confusing for Java users
as it makes top frame to point to some magical __tsan function.
But obtaining and using these PCs adds runtime cost for every java event.
Remove these PCs. Rely only on official Java frames.
It makes execution faster, code simpler and reports better.

Depends on D106956.

Reviewed By: vitalybuka

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

Added: 
    

Modified: 
    compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp
    compiler-rt/test/tsan/java_symbolization.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp b/compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp
index 72595527989b..7280ed1f67c7 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp
@@ -34,23 +34,6 @@ struct JavaContext {
   }
 };
 
-class ScopedJavaFunc {
- public:
-  ScopedJavaFunc(ThreadState *thr, uptr pc)
-      : thr_(thr) {
-    Initialize(thr_);
-    FuncEntry(thr, pc);
-  }
-
-  ~ScopedJavaFunc() {
-    FuncExit(thr_);
-    // FIXME(dvyukov): process pending signals.
-  }
-
- private:
-  ThreadState *thr_;
-};
-
 static u64 jctx_buf[sizeof(JavaContext) / sizeof(u64) + 1];
 static JavaContext *jctx;
 
@@ -73,13 +56,10 @@ MBlock *JavaHeapBlock(uptr addr, uptr *start) {
 
 }  // namespace __tsan
 
-#define SCOPED_JAVA_FUNC(func) \
+#define SCOPED_JAVA_FUNC(func)     \
   ThreadState *thr = cur_thread(); \
-  const uptr caller_pc = GET_CALLER_PC(); \
-  const uptr pc = StackTrace::GetCurrentPc(); \
-  (void)pc; \
-  ScopedJavaFunc scoped(thr, caller_pc); \
-/**/
+  (void)thr;                       \
+  /**/
 
 void __tsan_java_init(jptr heap_begin, jptr heap_size) {
   SCOPED_JAVA_FUNC(__tsan_java_init);
@@ -113,7 +93,7 @@ void __tsan_java_alloc(jptr ptr, jptr size) {
   CHECK_GE(ptr, jctx->heap_begin);
   CHECK_LE(ptr + size, jctx->heap_begin + jctx->heap_size);
 
-  OnUserAlloc(thr, pc, ptr, size, false);
+  OnUserAlloc(thr, 0, ptr, size, false);
 }
 
 void __tsan_java_free(jptr ptr, jptr size) {
@@ -195,8 +175,9 @@ void __tsan_java_mutex_lock(jptr addr) {
   CHECK_GE(addr, jctx->heap_begin);
   CHECK_LT(addr, jctx->heap_begin + jctx->heap_size);
 
-  MutexPostLock(thr, pc, addr, MutexFlagLinkerInit | MutexFlagWriteReentrant |
-      MutexFlagDoPreLockOnPostLock);
+  MutexPostLock(thr, 0, addr,
+                MutexFlagLinkerInit | MutexFlagWriteReentrant |
+                    MutexFlagDoPreLockOnPostLock);
 }
 
 void __tsan_java_mutex_unlock(jptr addr) {
@@ -206,7 +187,7 @@ void __tsan_java_mutex_unlock(jptr addr) {
   CHECK_GE(addr, jctx->heap_begin);
   CHECK_LT(addr, jctx->heap_begin + jctx->heap_size);
 
-  MutexUnlock(thr, pc, addr);
+  MutexUnlock(thr, 0, addr);
 }
 
 void __tsan_java_mutex_read_lock(jptr addr) {
@@ -216,8 +197,9 @@ void __tsan_java_mutex_read_lock(jptr addr) {
   CHECK_GE(addr, jctx->heap_begin);
   CHECK_LT(addr, jctx->heap_begin + jctx->heap_size);
 
-  MutexPostReadLock(thr, pc, addr, MutexFlagLinkerInit |
-      MutexFlagWriteReentrant | MutexFlagDoPreLockOnPostLock);
+  MutexPostReadLock(thr, 0, addr,
+                    MutexFlagLinkerInit | MutexFlagWriteReentrant |
+                        MutexFlagDoPreLockOnPostLock);
 }
 
 void __tsan_java_mutex_read_unlock(jptr addr) {
@@ -227,7 +209,7 @@ void __tsan_java_mutex_read_unlock(jptr addr) {
   CHECK_GE(addr, jctx->heap_begin);
   CHECK_LT(addr, jctx->heap_begin + jctx->heap_size);
 
-  MutexReadUnlock(thr, pc, addr);
+  MutexReadUnlock(thr, 0, addr);
 }
 
 void __tsan_java_mutex_lock_rec(jptr addr, int rec) {
@@ -238,8 +220,10 @@ void __tsan_java_mutex_lock_rec(jptr addr, int rec) {
   CHECK_LT(addr, jctx->heap_begin + jctx->heap_size);
   CHECK_GT(rec, 0);
 
-  MutexPostLock(thr, pc, addr, MutexFlagLinkerInit | MutexFlagWriteReentrant |
-      MutexFlagDoPreLockOnPostLock | MutexFlagRecursiveLock, rec);
+  MutexPostLock(thr, 0, addr,
+                MutexFlagLinkerInit | MutexFlagWriteReentrant |
+                    MutexFlagDoPreLockOnPostLock | MutexFlagRecursiveLock,
+                rec);
 }
 
 int __tsan_java_mutex_unlock_rec(jptr addr) {
@@ -249,7 +233,7 @@ int __tsan_java_mutex_unlock_rec(jptr addr) {
   CHECK_GE(addr, jctx->heap_begin);
   CHECK_LT(addr, jctx->heap_begin + jctx->heap_size);
 
-  return MutexUnlock(thr, pc, addr, MutexFlagRecursiveUnlock);
+  return MutexUnlock(thr, 0, addr, MutexFlagRecursiveUnlock);
 }
 
 void __tsan_java_acquire(jptr addr) {
@@ -259,7 +243,7 @@ void __tsan_java_acquire(jptr addr) {
   CHECK_GE(addr, jctx->heap_begin);
   CHECK_LT(addr, jctx->heap_begin + jctx->heap_size);
 
-  Acquire(thr, caller_pc, addr);
+  Acquire(thr, 0, addr);
 }
 
 void __tsan_java_release(jptr addr) {
@@ -269,7 +253,7 @@ void __tsan_java_release(jptr addr) {
   CHECK_GE(addr, jctx->heap_begin);
   CHECK_LT(addr, jctx->heap_begin + jctx->heap_size);
 
-  Release(thr, caller_pc, addr);
+  Release(thr, 0, addr);
 }
 
 void __tsan_java_release_store(jptr addr) {
@@ -279,5 +263,5 @@ void __tsan_java_release_store(jptr addr) {
   CHECK_GE(addr, jctx->heap_begin);
   CHECK_LT(addr, jctx->heap_begin + jctx->heap_size);
 
-  ReleaseStore(thr, caller_pc, addr);
+  ReleaseStore(thr, 0, addr);
 }

diff  --git a/compiler-rt/test/tsan/java_symbolization.cpp b/compiler-rt/test/tsan/java_symbolization.cpp
index cdc9782c8ec0..34e18a771b75 100644
--- a/compiler-rt/test/tsan/java_symbolization.cpp
+++ b/compiler-rt/test/tsan/java_symbolization.cpp
@@ -65,10 +65,8 @@ int main() {
 // On Linux/glibc #4 is __libc_start_main, but can be something else elsewhere.
 // CHECK:     #4
 // CHECK:   Location is heap block of size 32 at {{.*}} allocated by main thread:
-// CHECK:     #0 __tsan_java_alloc
-// CHECK:     #1 main
-// CHECK:     #2 Allocer1 Alloc.java:11:222
-// CHECK:     #3 Allocer2 Alloc.java:33:444
-// On Linux/glibc #4 is __libc_start_main, but can be something else elsewhere.
-// CHECK:     #4
+// CHECK:     #0 Allocer1 Alloc.java:11:222
+// CHECK:     #1 Allocer2 Alloc.java:33:444
+// On Linux/glibc #2 is __libc_start_main, but can be something else elsewhere.
+// CHECK:     #2
 // CHECK: DONE


        


More information about the llvm-commits mailing list