tsan: allow memory overlap in __tsan_java_move
authorDmitry Vyukov <dvyukov@google.com>
Tue, 8 Jul 2014 20:01:12 +0000 (20:01 +0000)
committerDmitry Vyukov <dvyukov@google.com>
Tue, 8 Jul 2014 20:01:12 +0000 (20:01 +0000)
JVM actually moves memory between overlapping ranges.

llvm-svn: 212560

13 files changed:
compiler-rt/lib/tsan/rtl/tsan_interface_java.cc
compiler-rt/lib/tsan/rtl/tsan_interface_java.h
compiler-rt/lib/tsan/rtl/tsan_sync.cc
compiler-rt/test/tsan/java_finalizer.cc
compiler-rt/test/tsan/java_lock.cc
compiler-rt/test/tsan/java_lock_move.cc
compiler-rt/test/tsan/java_lock_rec.cc
compiler-rt/test/tsan/java_lock_rec_race.cc
compiler-rt/test/tsan/java_move_overlap.cc [new file with mode: 0644]
compiler-rt/test/tsan/java_move_overlap_race.cc [new file with mode: 0644]
compiler-rt/test/tsan/java_race.cc
compiler-rt/test/tsan/java_race_move.cc
compiler-rt/test/tsan/java_rwlock.cc

index e63b93f..5dfb476 100644 (file)
@@ -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;
   }
index 6a83885..1f793df 100644 (file)
@@ -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.
index 3462b04..15392c9 100644 (file)
@@ -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;
index f1cf989..d5c6a22 100644 (file)
@@ -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();
 }
index e5513cd..36a0f8b 100644 (file)
@@ -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
index 15a72c7..19c3e35 100644 (file)
@@ -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
index 9223695..2b0ab0e 100644 (file)
@@ -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
index af308d7..841aa39 100644 (file)
@@ -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 (file)
index 0000000..12955b4
--- /dev/null
@@ -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 (file)
index 0000000..2b3769b
--- /dev/null
@@ -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
index 10aefdb..ede058e 100644 (file)
@@ -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
index 45876a0..8a51be9 100644 (file)
@@ -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
index d43dfe1..b03afa6 100644 (file)
@@ -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