[PATCH] [asan] Fix a deadlock between asan's allocator and lsan

[Kostya Serebryany]

  - Removed AssertLockHeld from allocateor, instead called mutex_.AssertHeld
  directly in GetBlockBeginFastLocked
  - Deduplicate code in asan/asan_allocator2.cc

Hi samsonov, earthdok,

http://llvm-reviews.chandlerc.com/D1957

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D1957?vs=4986&id=4991#toc

Files:
  lib/sanitizer_common/sanitizer_allocator.h
  lib/sanitizer_common/tests/sanitizer_allocator_test.cc
  lib/sanitizer_common/sanitizer_mutex.h
  lib/asan/lit_tests/TestCases/assign_large_valloc_to_global.cc
  lib/asan/asan_allocator2.cc

Index: lib/sanitizer_common/sanitizer_allocator.h
===================================================================
--- lib/sanitizer_common/sanitizer_allocator.h
+++ lib/sanitizer_common/sanitizer_allocator.h
@@ -1051,6 +1051,7 @@
   // This function does the same as GetBlockBegin, but is much faster.
   // Must be called with the allocator locked.
   void *GetBlockBeginFastLocked(void *ptr) {
+    mutex_.AssertHeld();
     uptr p = reinterpret_cast<uptr>(ptr);
     uptr n = n_chunks_;
     if (!n) return 0;
Index: lib/sanitizer_common/tests/sanitizer_allocator_test.cc
===================================================================
--- lib/sanitizer_common/tests/sanitizer_allocator_test.cc
+++ lib/sanitizer_common/tests/sanitizer_allocator_test.cc
@@ -742,6 +742,7 @@
     allocated[i] = (char *)a.Allocate(&stats, size, 1);
   }
 
+  a.ForceLock();
   for (uptr i = 0; i < kNumAllocs  * kNumAllocs; i++) {
     // if ((i & (i - 1)) == 0) fprintf(stderr, "[%zd]\n", i);
     char *p1 = allocated[i % kNumAllocs];
@@ -757,6 +758,7 @@
     p = reinterpret_cast<void *>(~0L - (i % 1024));
     EXPECT_EQ((void *)0, a.GetBlockBeginFastLocked(p));
   }
+  a.ForceUnlock();
 
   for (uptr i = 0; i < kNumAllocs; i++)
     a.Deallocate(&stats, allocated[i]);
Index: lib/sanitizer_common/sanitizer_mutex.h
===================================================================
--- lib/sanitizer_common/sanitizer_mutex.h
+++ lib/sanitizer_common/sanitizer_mutex.h
@@ -40,6 +40,10 @@
     atomic_store(&state_, 0, memory_order_release);
   }
 
+  void AssertHeld() {
+    CHECK_EQ(atomic_load(&state_, memory_order_relaxed), 1);
+  }
+
  private:
   atomic_uint8_t state_;
 
Index: lib/asan/lit_tests/TestCases/assign_large_valloc_to_global.cc
===================================================================
--- /dev/null
+++ lib/asan/lit_tests/TestCases/assign_large_valloc_to_global.cc
@@ -0,0 +1,5 @@
+// Make sure we don't report a leak nor hang.
+// RUN: %clangxx_asan -O3 %s -o %t && %t
+#include <malloc.h>
+int *p = (int*)valloc(1 << 20);
+int main() { }
Index: lib/asan/asan_allocator2.cc
===================================================================
--- lib/asan/asan_allocator2.cc
+++ lib/asan/asan_allocator2.cc
@@ -186,14 +186,19 @@
 
 struct AsanChunk: ChunkBase {
   uptr Beg() { return reinterpret_cast<uptr>(this) + kChunkHeaderSize; }
-  uptr UsedSize() {
+  uptr UsedSize(bool locked_version = false) {
     if (user_requested_size != SizeClassMap::kMaxSize)
       return user_requested_size;
-    return *reinterpret_cast<uptr *>(allocator.GetMetaData(AllocBeg()));
+    return *reinterpret_cast<uptr *>(
+                allocator.GetMetaData(AllocBeg(locked_version)));
   }
-  void *AllocBeg() {
-    if (from_memalign)
+  void *AllocBeg(bool locked_version = false) {
+    if (from_memalign) {
+      if (locked_version)
+        return allocator.GetBlockBeginFastLocked(
+            reinterpret_cast<void *>(this));
       return allocator.GetBlockBegin(reinterpret_cast<void *>(this));
+    }
     return reinterpret_cast<void*>(Beg() - RZLog2Size(rz_log));
   }
   // If we don't use stack depot, we store the alloc/free stack traces
@@ -213,8 +218,8 @@
     uptr available = RoundUpTo(user_requested_size, SHADOW_GRANULARITY);
     return (available - kChunkHeader2Size) / sizeof(u32);
   }
-  bool AddrIsInside(uptr addr) {
-    return (addr >= Beg()) && (addr < Beg() + UsedSize());
+  bool AddrIsInside(uptr addr, bool locked_version = false) {
+    return (addr >= Beg()) && (addr < Beg() + UsedSize(locked_version));
   }
 };
 
@@ -722,7 +727,8 @@
   __asan::AsanChunk *m = __asan::GetAsanChunkByAddrFastLocked(addr);
   if (!m) return 0;
   uptr chunk = m->Beg();
-  if ((m->chunk_state == __asan::CHUNK_ALLOCATED) && m->AddrIsInside(addr))
+  if ((m->chunk_state == __asan::CHUNK_ALLOCATED) &&
+      m->AddrIsInside(addr, /*locked_version=*/true))
     return chunk;
   return 0;
 }
@@ -755,7 +761,7 @@
 
 uptr LsanMetadata::requested_size() const {
   __asan::AsanChunk *m = reinterpret_cast<__asan::AsanChunk *>(metadata_);
-  return m->UsedSize();
+  return m->UsedSize(/*locked_version=*/true);
 }
 
 u32 LsanMetadata::stack_trace_id() const {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1957.2.patch
Type: text/x-patch
Size: 4200 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131017/ec3fb5b1/attachment.bin>