From 3f5ad1a98e211216be9612a60ca30644e0f4073d Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Tue, 8 Jul 2014 20:01:12 +0000 Subject: [PATCH] tsan: allow memory overlap in __tsan_java_move JVM actually moves memory between overlapping ranges. llvm-svn: 212560 --- compiler-rt/lib/tsan/rtl/tsan_interface_java.cc | 12 ++++- compiler-rt/lib/tsan/rtl/tsan_interface_java.h | 2 +- compiler-rt/lib/tsan/rtl/tsan_sync.cc | 15 ++++-- compiler-rt/test/tsan/java_finalizer.cc | 10 ++-- compiler-rt/test/tsan/java_lock.cc | 15 +++--- compiler-rt/test/tsan/java_lock_move.cc | 13 ++--- compiler-rt/test/tsan/java_lock_rec.cc | 15 +++--- compiler-rt/test/tsan/java_lock_rec_race.cc | 15 +++--- compiler-rt/test/tsan/java_move_overlap.cc | 72 +++++++++++++++++++++++++ compiler-rt/test/tsan/java_move_overlap_race.cc | 53 ++++++++++++++++++ compiler-rt/test/tsan/java_race.cc | 12 +++-- compiler-rt/test/tsan/java_race_move.cc | 10 ++-- compiler-rt/test/tsan/java_rwlock.cc | 15 +++--- 13 files changed, 205 insertions(+), 54 deletions(-) create mode 100644 compiler-rt/test/tsan/java_move_overlap.cc create mode 100644 compiler-rt/test/tsan/java_move_overlap_race.cc diff --git a/compiler-rt/lib/tsan/rtl/tsan_interface_java.cc b/compiler-rt/lib/tsan/rtl/tsan_interface_java.cc index e63b93f..5dfb476 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_interface_java.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_interface_java.cc @@ -126,7 +126,8 @@ void __tsan_java_move(jptr src, jptr dst, jptr size) { CHECK_LE(src + size, jctx->heap_begin + jctx->heap_size); CHECK_GE(dst, jctx->heap_begin); CHECK_LE(dst + size, jctx->heap_begin + jctx->heap_size); - CHECK(dst >= src + size || src >= dst + size); + CHECK_NE(dst, src); + CHECK_NE(size, 0); // Assuming it's not running concurrently with threads that do // memory accesses and mutex operations (stop-the-world phase). @@ -136,7 +137,14 @@ void __tsan_java_move(jptr src, jptr dst, jptr size) { u64 *s = (u64*)MemToShadow(src); u64 *d = (u64*)MemToShadow(dst); u64 *send = (u64*)MemToShadow(src + size); - for (; s != send; s++, d++) { + uptr inc = 1; + if (dst > src) { + s = (u64*)MemToShadow(src + size) - 1; + d = (u64*)MemToShadow(dst + size) - 1; + send = (u64*)MemToShadow(src) - 1; + inc = -1; + } + for (; s != send; s += inc, d += inc) { *d = *s; *s = 0; } diff --git a/compiler-rt/lib/tsan/rtl/tsan_interface_java.h b/compiler-rt/lib/tsan/rtl/tsan_interface_java.h index 6a83885..1f793df 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_interface_java.h +++ b/compiler-rt/lib/tsan/rtl/tsan_interface_java.h @@ -50,7 +50,7 @@ void __tsan_java_alloc(jptr ptr, jptr size) INTERFACE_ATTRIBUTE; void __tsan_java_free(jptr ptr, jptr size) INTERFACE_ATTRIBUTE; // Callback for memory move by GC. // Can be aggregated for several objects (preferably). -// The ranges must not overlap. +// The ranges can overlap. void __tsan_java_move(jptr src, jptr dst, jptr size) INTERFACE_ATTRIBUTE; // This function must be called on the finalizer thread // before executing a batch of finalizers. diff --git a/compiler-rt/lib/tsan/rtl/tsan_sync.cc b/compiler-rt/lib/tsan/rtl/tsan_sync.cc index 3462b04..15392c9 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_sync.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_sync.cc @@ -180,13 +180,22 @@ SyncVar* MetaMap::GetAndLock(ThreadState *thr, uptr pc, } void MetaMap::MoveMemory(uptr src, uptr dst, uptr sz) { - // Here we assume that src and dst do not overlap, - // and there are no concurrent accesses to the regions (e.g. stop-the-world). + // src and dst can overlap, + // there are no concurrent accesses to the regions (e.g. stop-the-world). + CHECK_NE(src, dst); + CHECK_NE(sz, 0); uptr diff = dst - src; u32 *src_meta = MemToMeta(src); u32 *dst_meta = MemToMeta(dst); u32 *src_meta_end = MemToMeta(src + sz); - for (; src_meta != src_meta_end; src_meta++, dst_meta++) { + uptr inc = 1; + if (dst > src) { + src_meta = MemToMeta(src + sz) - 1; + dst_meta = MemToMeta(dst + sz) - 1; + src_meta_end = MemToMeta(src) - 1; + inc = -1; + } + for (; src_meta != src_meta_end; src_meta += inc, dst_meta += inc) { CHECK_EQ(*dst_meta, 0); u32 idx = *src_meta; *src_meta = 0; diff --git a/compiler-rt/test/tsan/java_finalizer.cc b/compiler-rt/test/tsan/java_finalizer.cc index f1cf989..d5c6a22 100644 --- a/compiler-rt/test/tsan/java_finalizer.cc +++ b/compiler-rt/test/tsan/java_finalizer.cc @@ -10,15 +10,15 @@ void *Thread(void *p) { int main() { int const kHeapSize = 1024 * 1024; - void *jheap = (char*)malloc(kHeapSize + 8) + 8; - __tsan_java_init((jptr)jheap, kHeapSize); + jptr jheap = (jptr)malloc(kHeapSize + 8) + 8; + __tsan_java_init(jheap, kHeapSize); const int kBlockSize = 16; - __tsan_java_alloc((jptr)jheap, kBlockSize); + __tsan_java_alloc(jheap, kBlockSize); pthread_t th; - pthread_create(&th, 0, Thread, jheap); + pthread_create(&th, 0, Thread, (void*)jheap); *(int*)jheap = 43; pthread_join(th, 0); - __tsan_java_free((jptr)jheap, kBlockSize); + __tsan_java_free(jheap, kBlockSize); fprintf(stderr, "DONE\n"); return __tsan_java_fini(); } diff --git a/compiler-rt/test/tsan/java_lock.cc b/compiler-rt/test/tsan/java_lock.cc index e5513cd..36a0f8b 100644 --- a/compiler-rt/test/tsan/java_lock.cc +++ b/compiler-rt/test/tsan/java_lock.cc @@ -15,21 +15,22 @@ void *Thread(void *p) { int main() { int const kHeapSize = 1024 * 1024; - void *jheap = malloc(kHeapSize); - __tsan_java_init((jptr)jheap, kHeapSize); + jptr jheap = (jptr)malloc(kHeapSize + 8) + 8; + __tsan_java_init(jheap, kHeapSize); const int kBlockSize = 16; - __tsan_java_alloc((jptr)jheap, kBlockSize); - varaddr = (jptr)jheap; - lockaddr = (jptr)jheap + 8; + __tsan_java_alloc(jheap, kBlockSize); + varaddr = jheap; + lockaddr = jheap + 8; pthread_t th; pthread_create(&th, 0, Thread, 0); __tsan_java_mutex_lock(lockaddr); *(int*)varaddr = 43; __tsan_java_mutex_unlock(lockaddr); pthread_join(th, 0); - __tsan_java_free((jptr)jheap, kBlockSize); - printf("OK\n"); + __tsan_java_free(jheap, kBlockSize); + fprintf(stderr, "DONE\n"); return __tsan_java_fini(); } // CHECK-NOT: WARNING: ThreadSanitizer: data race +// CHECK: DONE diff --git a/compiler-rt/test/tsan/java_lock_move.cc b/compiler-rt/test/tsan/java_lock_move.cc index 15a72c7..19c3e35 100644 --- a/compiler-rt/test/tsan/java_lock_move.cc +++ b/compiler-rt/test/tsan/java_lock_move.cc @@ -16,13 +16,13 @@ void *Thread(void *p) { int main() { int const kHeapSize = 1024 * 1024; - void *jheap = malloc(kHeapSize); - __tsan_java_init((jptr)jheap, kHeapSize); + jptr jheap = (jptr)malloc(kHeapSize + 8) + 8; + __tsan_java_init(jheap, kHeapSize); const int kBlockSize = 64; int const kMove = 1024; - __tsan_java_alloc((jptr)jheap, kBlockSize); - varaddr = (jptr)jheap; - lockaddr = (jptr)jheap + 46; + __tsan_java_alloc(jheap, kBlockSize); + varaddr = jheap; + lockaddr = jheap + 46; varaddr2 = varaddr + kMove; lockaddr2 = lockaddr + kMove; pthread_t th; @@ -33,8 +33,9 @@ int main() { __tsan_java_move(varaddr, varaddr2, kBlockSize); pthread_join(th, 0); __tsan_java_free(varaddr2, kBlockSize); - printf("OK\n"); + printf("DONE\n"); return __tsan_java_fini(); } // CHECK-NOT: WARNING: ThreadSanitizer: data race +// CHECK: DONE diff --git a/compiler-rt/test/tsan/java_lock_rec.cc b/compiler-rt/test/tsan/java_lock_rec.cc index 9223695..2b0ab0e 100644 --- a/compiler-rt/test/tsan/java_lock_rec.cc +++ b/compiler-rt/test/tsan/java_lock_rec.cc @@ -27,13 +27,13 @@ void *Thread(void *p) { int main() { int const kHeapSize = 1024 * 1024; - void *jheap = malloc(kHeapSize); - __tsan_java_init((jptr)jheap, kHeapSize); + jptr jheap = (jptr)malloc(kHeapSize + 8) + 8; + __tsan_java_init(jheap, kHeapSize); const int kBlockSize = 16; - __tsan_java_alloc((jptr)jheap, kBlockSize); - varaddr = (jptr)jheap; + __tsan_java_alloc(jheap, kBlockSize); + varaddr = jheap; *(int*)varaddr = 0; - lockaddr = (jptr)jheap + 8; + lockaddr = jheap + 8; pthread_t th; pthread_create(&th, 0, Thread, 0); sleep(1); @@ -45,10 +45,11 @@ int main() { *(int*)varaddr = 43; __tsan_java_mutex_unlock(lockaddr); pthread_join(th, 0); - __tsan_java_free((jptr)jheap, kBlockSize); - printf("OK\n"); + __tsan_java_free(jheap, kBlockSize); + printf("DONE\n"); return __tsan_java_fini(); } // CHECK-NOT: WARNING: ThreadSanitizer: data race // CHECK-NOT: FAILED +// CHECK: DONE diff --git a/compiler-rt/test/tsan/java_lock_rec_race.cc b/compiler-rt/test/tsan/java_lock_rec_race.cc index af308d7..841aa39 100644 --- a/compiler-rt/test/tsan/java_lock_rec_race.cc +++ b/compiler-rt/test/tsan/java_lock_rec_race.cc @@ -25,13 +25,13 @@ void *Thread(void *p) { int main() { int const kHeapSize = 1024 * 1024; - void *jheap = (char*)malloc(kHeapSize + 8) + 8; - __tsan_java_init((jptr)jheap, kHeapSize); + jptr jheap = (jptr)malloc(kHeapSize + 8) + 8; + __tsan_java_init(jheap, kHeapSize); const int kBlockSize = 16; - __tsan_java_alloc((jptr)jheap, kBlockSize); - varaddr = (jptr)jheap; + __tsan_java_alloc(jheap, kBlockSize); + varaddr = jheap; *(int*)varaddr = 0; - lockaddr = (jptr)jheap + 8; + lockaddr = jheap + 8; pthread_t th; pthread_create(&th, 0, Thread, 0); sleep(1); @@ -39,10 +39,11 @@ int main() { *(int*)varaddr = 43; __tsan_java_mutex_unlock(lockaddr); pthread_join(th, 0); - __tsan_java_free((jptr)jheap, kBlockSize); - printf("OK\n"); + __tsan_java_free(jheap, kBlockSize); + printf("DONE\n"); return __tsan_java_fini(); } // CHECK: WARNING: ThreadSanitizer: data race // CHECK-NOT: FAILED +// CHECK: DONE diff --git a/compiler-rt/test/tsan/java_move_overlap.cc b/compiler-rt/test/tsan/java_move_overlap.cc new file mode 100644 index 0000000..12955b4 --- /dev/null +++ b/compiler-rt/test/tsan/java_move_overlap.cc @@ -0,0 +1,72 @@ +// RUN: %clangxx_tsan -O1 %s -o %t +// RUN: %run %t 2>&1 | FileCheck %s +// RUN: %run %t arg 2>&1 | FileCheck %s +#include "java.h" + +jptr varaddr1_old; +jptr varaddr2_old; +jptr lockaddr1_old; +jptr lockaddr2_old; +jptr varaddr1_new; +jptr varaddr2_new; +jptr lockaddr1_new; +jptr lockaddr2_new; + +void *Thread(void *p) { + sleep(1); + __tsan_java_mutex_lock(lockaddr1_new); + *(char*)varaddr1_new = 43; + __tsan_java_mutex_unlock(lockaddr1_new); + __tsan_java_mutex_lock(lockaddr2_new); + *(char*)varaddr2_new = 43; + __tsan_java_mutex_unlock(lockaddr2_new); + return 0; +} + +int main(int argc, char **argv) { + int const kHeapSize = 1024 * 1024; + void *jheap = malloc(kHeapSize); + jheap = (char*)jheap + 8; + __tsan_java_init((jptr)jheap, kHeapSize); + const int kBlockSize = 64; + int const kMove = 32; + varaddr1_old = (jptr)jheap; + lockaddr1_old = (jptr)jheap + 1; + varaddr2_old = (jptr)jheap + kBlockSize - 1; + lockaddr2_old = (jptr)jheap + kBlockSize - 16; + varaddr1_new = varaddr1_old + kMove; + lockaddr1_new = lockaddr1_old + kMove; + varaddr2_new = varaddr2_old + kMove; + lockaddr2_new = lockaddr2_old + kMove; + if (argc > 1) { + // Move memory backwards. + varaddr1_old += kMove; + lockaddr1_old += kMove; + varaddr2_old += kMove; + lockaddr2_old += kMove; + varaddr1_new -= kMove; + lockaddr1_new -= kMove; + varaddr2_new -= kMove; + lockaddr2_new -= kMove; + } + __tsan_java_alloc(varaddr1_old, kBlockSize); + + pthread_t th; + pthread_create(&th, 0, Thread, 0); + + __tsan_java_mutex_lock(lockaddr1_old); + *(char*)varaddr1_old = 43; + __tsan_java_mutex_unlock(lockaddr1_old); + __tsan_java_mutex_lock(lockaddr2_old); + *(char*)varaddr2_old = 43; + __tsan_java_mutex_unlock(lockaddr2_old); + + __tsan_java_move(varaddr1_old, varaddr1_new, kBlockSize); + pthread_join(th, 0); + __tsan_java_free(varaddr1_new, kBlockSize); + printf("DONE\n"); + return __tsan_java_fini(); +} + +// CHECK-NOT: WARNING: ThreadSanitizer: data race +// CHECK: DONE diff --git a/compiler-rt/test/tsan/java_move_overlap_race.cc b/compiler-rt/test/tsan/java_move_overlap_race.cc new file mode 100644 index 0000000..2b3769b --- /dev/null +++ b/compiler-rt/test/tsan/java_move_overlap_race.cc @@ -0,0 +1,53 @@ +// RUN: %clangxx_tsan -O1 %s -o %t +// RUN: %deflake %run %t | FileCheck %s +// RUN: %deflake %run %t arg | FileCheck %s +#include "java.h" + +jptr varaddr1_old; +jptr varaddr2_old; +jptr varaddr1_new; +jptr varaddr2_new; + +void *Thread(void *p) { + sleep(1); + *(int*)varaddr1_new = 43; + *(int*)varaddr2_new = 43; + return 0; +} + +int main(int argc, char **argv) { + int const kHeapSize = 1024 * 1024; + void *jheap = malloc(kHeapSize); + jheap = (char*)jheap + 8; + __tsan_java_init((jptr)jheap, kHeapSize); + const int kBlockSize = 64; + int const kMove = 32; + varaddr1_old = (jptr)jheap; + varaddr2_old = (jptr)jheap + kBlockSize - 1; + varaddr1_new = varaddr1_old + kMove; + varaddr2_new = varaddr2_old + kMove; + if (argc > 1) { + // Move memory backwards. + varaddr1_old += kMove; + varaddr2_old += kMove; + varaddr1_new -= kMove; + varaddr2_new -= kMove; + } + __tsan_java_alloc(varaddr1_old, kBlockSize); + + pthread_t th; + pthread_create(&th, 0, Thread, 0); + + *(int*)varaddr1_old = 43; + *(int*)varaddr2_old = 43; + + __tsan_java_move(varaddr1_old, varaddr1_new, kBlockSize); + pthread_join(th, 0); + __tsan_java_free(varaddr1_new, kBlockSize); + printf("DONE\n"); + return __tsan_java_fini(); +} + +// CHECK: WARNING: ThreadSanitizer: data race +// CHECK: WARNING: ThreadSanitizer: data race +// CHECK: DONE diff --git a/compiler-rt/test/tsan/java_race.cc b/compiler-rt/test/tsan/java_race.cc index 10aefdb..ede058e 100644 --- a/compiler-rt/test/tsan/java_race.cc +++ b/compiler-rt/test/tsan/java_race.cc @@ -8,16 +8,18 @@ void *Thread(void *p) { int main() { int const kHeapSize = 1024 * 1024; - void *jheap = (char*)malloc(kHeapSize + 8) + 8; - __tsan_java_init((jptr)jheap, kHeapSize); + jptr jheap = (jptr)malloc(kHeapSize + 8) + 8; + __tsan_java_init(jheap, kHeapSize); const int kBlockSize = 16; - __tsan_java_alloc((jptr)jheap, kBlockSize); + __tsan_java_alloc(jheap, kBlockSize); pthread_t th; - pthread_create(&th, 0, Thread, jheap); + pthread_create(&th, 0, Thread, (void*)jheap); *(int*)jheap = 43; pthread_join(th, 0); - __tsan_java_free((jptr)jheap, kBlockSize); + __tsan_java_free(jheap, kBlockSize); + fprintf(stderr, "DONE\n"); return __tsan_java_fini(); } // CHECK: WARNING: ThreadSanitizer: data race +// CHECK: DONE diff --git a/compiler-rt/test/tsan/java_race_move.cc b/compiler-rt/test/tsan/java_race_move.cc index 45876a0..8a51be92 100644 --- a/compiler-rt/test/tsan/java_race_move.cc +++ b/compiler-rt/test/tsan/java_race_move.cc @@ -12,12 +12,12 @@ void *Thread(void *p) { int main() { int const kHeapSize = 1024 * 1024; - void *jheap = (char*)malloc(kHeapSize + 8) + 8; - __tsan_java_init((jptr)jheap, kHeapSize); + jptr jheap = (jptr)malloc(kHeapSize + 8) + 8; + __tsan_java_init(jheap, kHeapSize); const int kBlockSize = 64; int const kMove = 1024; - __tsan_java_alloc((jptr)jheap, kBlockSize); - varaddr = (jptr)jheap + 16; + __tsan_java_alloc(jheap, kBlockSize); + varaddr = jheap + 16; varaddr2 = varaddr + kMove; pthread_t th; pthread_create(&th, 0, Thread, 0); @@ -25,7 +25,9 @@ int main() { __tsan_java_move(varaddr, varaddr2, kBlockSize); pthread_join(th, 0); __tsan_java_free(varaddr2, kBlockSize); + fprintf(stderr, "DONE\n"); return __tsan_java_fini(); } // CHECK: WARNING: ThreadSanitizer: data race +// CHECK: DONE diff --git a/compiler-rt/test/tsan/java_rwlock.cc b/compiler-rt/test/tsan/java_rwlock.cc index d43dfe1..b03afa6 100644 --- a/compiler-rt/test/tsan/java_rwlock.cc +++ b/compiler-rt/test/tsan/java_rwlock.cc @@ -15,21 +15,22 @@ void *Thread(void *p) { int main() { int const kHeapSize = 1024 * 1024; - void *jheap = malloc(kHeapSize); - __tsan_java_init((jptr)jheap, kHeapSize); + jptr jheap = (jptr)malloc(kHeapSize + 8) + 8; + __tsan_java_init(jheap, kHeapSize); const int kBlockSize = 16; - __tsan_java_alloc((jptr)jheap, kBlockSize); - varaddr = (jptr)jheap; - lockaddr = (jptr)jheap + 8; + __tsan_java_alloc(jheap, kBlockSize); + varaddr = jheap; + lockaddr = jheap + 8; pthread_t th; pthread_create(&th, 0, Thread, 0); __tsan_java_mutex_lock(lockaddr); *(int*)varaddr = 43; __tsan_java_mutex_unlock(lockaddr); pthread_join(th, 0); - __tsan_java_free((jptr)jheap, kBlockSize); - printf("OK\n"); + __tsan_java_free(jheap, kBlockSize); + printf("DONE\n"); return __tsan_java_fini(); } // CHECK-NOT: WARNING: ThreadSanitizer: data race +// CHECK: DONE -- 2.7.4