[compiler-rt] db2f870 - tsan: reset destination range in Java heap move

[Dmitry Vyukov via llvm-commits]

Author: Dmitry Vyukov
Date: 2021-09-22T07:23:21+02:00
New Revision: db2f870fe3dcecc43c874ef571757d5aeac0569c

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

LOG: tsan: reset destination range in Java heap move

Switch Java heap move to the new scheme required for the new tsan runtime.
Instead of copying the shadow we reset the destination range.
The new v3 trace contains addresses of accesses, so we cannot simply copy the shadow.
This can lead to false negatives, but cannot lead to false positives.

Depends on D110159.

Reviewed By: vitalybuka

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

Added: 
    

Modified: 
    compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp
    compiler-rt/test/tsan/java_move_overlap_race.cpp
    compiler-rt/test/tsan/java_race_move.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 8ea53431d9b75..c090c1f08cbeb 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp
@@ -128,21 +128,12 @@ void __tsan_java_move(jptr src, jptr dst, jptr size) {
   // memory accesses and mutex operations (stop-the-world phase).
   ctx->metamap.MoveMemory(src, dst, size);
 
-  // Move shadow.
-  RawShadow *s = MemToShadow(src);
+  // Clear the destination shadow range.
+  // We used to move shadow from src to dst, but the trace format does not
+  // support that anymore as it contains addresses of accesses.
   RawShadow *d = MemToShadow(dst);
-  RawShadow *send = MemToShadow(src + size);
-  uptr inc = 1;
-  if (dst > src) {
-    s = MemToShadow(src + size) - 1;
-    d = MemToShadow(dst + size) - 1;
-    send = MemToShadow(src) - 1;
-    inc = -1;
-  }
-  for (; s != send; s += inc, d += inc) {
-    *d = *s;
-    *s = 0;
-  }
+  RawShadow *dend = MemToShadow(dst + size);
+  internal_memset(d, 0, (dend - d) * sizeof(*d));
 }
 
 jptr __tsan_java_find(jptr *from_ptr, jptr to) {

diff  --git a/compiler-rt/test/tsan/java_move_overlap_race.cpp b/compiler-rt/test/tsan/java_move_overlap_race.cpp
index fbbcf2c8a33fa..efe90b6fa7e9c 100644
--- a/compiler-rt/test/tsan/java_move_overlap_race.cpp
+++ b/compiler-rt/test/tsan/java_move_overlap_race.cpp
@@ -1,6 +1,6 @@
 // RUN: %clangxx_tsan -O1 %s -o %t
-// RUN: %deflake %run %t 2>&1 | FileCheck %s
-// RUN: %deflake %run %t arg 2>&1 | FileCheck %s
+// RUN: %run %t 2>&1 | FileCheck %s
+// RUN: %run %t arg 2>&1 | FileCheck %s
 #include "java.h"
 
 jptr varaddr1_old;
@@ -50,6 +50,7 @@ int main(int argc, char **argv) {
   return __tsan_java_fini();
 }
 
-// CHECK: WARNING: ThreadSanitizer: data race
-// CHECK: WARNING: ThreadSanitizer: data race
+// Note: there is a race on the moved object (which we used to detect),
+// but now __tsan_java_move resets the object shadow, so we don't detect it anymore.
+// CHECK-NOT: WARNING: ThreadSanitizer: data race
 // CHECK: DONE

diff  --git a/compiler-rt/test/tsan/java_race_move.cpp b/compiler-rt/test/tsan/java_race_move.cpp
index 6d1b092160e07..eb4dba872a832 100644
--- a/compiler-rt/test/tsan/java_race_move.cpp
+++ b/compiler-rt/test/tsan/java_race_move.cpp
@@ -1,4 +1,4 @@
-// RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t | FileCheck %s
+// RUN: %clangxx_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
 #include "java.h"
 
 jptr varaddr;
@@ -31,5 +31,7 @@ int main() {
   return __tsan_java_fini();
 }
 
-// CHECK: WARNING: ThreadSanitizer: data race
+// Note: there is a race on the moved object (which we used to detect),
+// but now __tsan_java_move resets the object shadow, so we don't detect it anymore.
+// CHECK-NOT: WARNING: ThreadSanitizer: data race
 // CHECK: DONE