From db2f870fe3dcecc43c874ef571757d5aeac0569c Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Tue, 21 Sep 2021 21:07:43 +0200 Subject: [PATCH] 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 --- compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp | 19 +++++-------------- compiler-rt/test/tsan/java_move_overlap_race.cpp | 9 +++++---- compiler-rt/test/tsan/java_race_move.cpp | 6 ++++-- 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp b/compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp index 8ea5343..c090c1f 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 fbbcf2c..efe90b6 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 6d1b092..eb4dba8 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 -- 2.7.4