[Asan] Don't crash if metadata is not initialized

[vitalybuka]

Summary

Fixes https://github.com/google/sanitizers/issues/1193.

AsanChunk can be uninitialized yet just after return from the secondary
allocator. If lsan starts scan just before metadata assignment it can
fail to find corresponding AsanChunk.

It should be safe to ignore this and let lsan to assume that
AsanChunk is in the beginning of the block. This block is from the
secondary allocator and created with mmap, so it should not contain
any pointers and will make lsan to miss some leaks.

Similar already happens for primary allocator. If it can't find real
AsanChunk it falls back and assume that block starts with AsanChunk.
Then if the block is already returned to allocator we have garbage in
AsanChunk and may scan dead memory hiding some leaks.
I'll fix this in D87135.

[vitalybuka]

simplify test

[morehouse]

compiler-rt/lib/asan/asan_allocator.cpp

742

The reason for falling back to alloc_beg is not obvious from the code.

Could you add a comment explaining why we do this, and why it's safe? (both here and below)

750

Since we're here, could you also document why falling back to alloc_beg is safe for the primary?

[vitalybuka]

move nullptr handling closer to lsan

[vitalybuka]

compiler-rt/lib/asan/asan_allocator.cpp

750

Looks like this is not very safe as well, lsan is lucky as LsanMetadata::allocated() will return false and the chunk will be ignored.
Other callers like AsanChunkView can handle nullptr, but will display wrong information in case of the current primary fallback.

[vitalybuka]

rebase

[morehouse]

Please update the commit message.

compiler-rt/lib/asan/asan_allocator.cpp

1111

Do we also need to make these changes in lsan_allocator.cpp?

1113

s/0/nullptr

[vitalybuka]

0 -> nullptr

compiler-rt/lib/asan/asan_allocator.cpp

1111

Yes. LsanMetadata argument is results of GetUserBegin
pure lsan version of GetUserBegin never returns null
asan has to return null as we can get into situations when AsanChunk is not yet initialized.

compiler-rt/lib/asan/asan_allocator.cpp

void CommitBack(AsanThreadLocalMallocStorage *ms, BufferedStackTrace *stack) {
     AllocatorCache *ac = GetAllocatorCache(ms);
     quarantine.Drain(GetQuarantineCache(ms), QuarantineCallback(ac, stack));
     allocator.SwallowCache(ac);
   }
 
   // -------------------------- Chunk lookup ----------------------
 
   // Assumes alloc_beg == allocator.GetBlockBegin(alloc_beg).
+  // Returns nullptr if AsanChunk is not yet initialized just after
+  // get_allocator().Allocate(), or is being destroyed just before
+  // get_allocator().Deallocate().
   AsanChunk *GetAsanChunk(void *alloc_beg) {
     if (!alloc_beg)
       return nullptr;
     if (!allocator.FromPrimary(alloc_beg)) {
       uptr *meta = reinterpret_cast<uptr *>(allocator.GetMetaData(alloc_beg));
       AsanChunk *m = reinterpret_cast<AsanChunk *>(meta[1]);
       return m;
     }
     uptr *alloc_magic = reinterpret_cast<uptr *>(alloc_beg);
     if (alloc_magic[0] == kAllocBegMagic)
       return reinterpret_cast<AsanChunk *>(alloc_magic[1]);
     // FIXME: This is either valid small chunk with tiny redzone or invalid
     // chunk which is beeing allocated/deallocated. The latter case should
     // return nullptr like secondary allocator does.
     return reinterpret_cast<AsanChunk *>(alloc_beg);
   }
 
   AsanChunk *GetAsanChunkDebug(void *alloc_beg) {
     if (!alloc_beg)
       return nullptr;
     if (!allocator.FromPrimary(alloc_beg)) {
       uptr *meta = reinterpret_cast<uptr *>(allocator.GetMetaData(alloc_beg));
       AsanChunk *m = reinterpret_cast<AsanChunk *>(meta[1]);
void GetUserBeginDebug(uptr chunk) {
   Printf("GetUserBeginDebug1 chunk %p\n", chunk);
   __asan::AsanChunk *m =
       __asan::instance.GetAsanChunkByAddrFastLockedDebug(chunk);
   Printf("GetUserBeginDebug2 m     %p\n", m);
 }
 
 uptr GetUserBegin(uptr chunk) {
   __asan::AsanChunk *m = __asan::instance.GetAsanChunkByAddrFastLocked(chunk);
-  if (!m) {
-    Printf(
-        "ASAN is about to crash with a CHECK failure.\n"
-        "The ASAN developers are trying to chase down this bug,\n"
-        "so if you've encountered this bug please let us know.\n"
-        "See also: https://github.com/google/sanitizers/issues/1193\n"
-        "Internal ref b/149237057\n"
-        "chunk: %p caller %p __lsan_current_stage %s\n",
-        chunk, GET_CALLER_PC(), __lsan_current_stage);
-    GetUserBeginDebug(chunk);
-  }
-  CHECK(m);
-  return m->Beg();
+  return m ? m->Beg() : 0;
 }
 
 LsanMetadata::LsanMetadata(uptr chunk) {
-  metadata_ = reinterpret_cast<void *>(chunk - __asan::kChunkHeaderSize);
+  metadata_ = chunk ? reinterpret_cast<void *>(chunk - __asan::kChunkHeaderSize)
+                    : nullptr;
 }
 
 bool LsanMetadata::allocated() const {
+  if (!metadata_)
+    return false;
   __asan::AsanChunk *m = reinterpret_cast<__asan::AsanChunk *>(metadata_);
   return atomic_load(&m->chunk_state, memory_order_relaxed) ==
          __asan::CHUNK_ALLOCATED;
 }
 
 ChunkTag LsanMetadata::tag() const {
   __asan::AsanChunk *m = reinterpret_cast<__asan::AsanChunk *>(metadata_);
   return static_cast<ChunkTag>(m->lsan_tag);

compiler-rt/test/asan/TestCases/lsan_crash.cpp

+// RUN: %clangxx_asan -O2 %s -o %t && %run %t
+
+#include <atomic>
+#include <memory>
+#include <sanitizer/lsan_interface.h>
+#include <thread>
+#include <vector>
+
+std::atomic<bool> done;
+
+void foo() {
+  std::unique_ptr<char[]> mem;
+
+  while (!done)
+    mem.reset(new char[1000000]);
+}
+
+int main() {
+  std::vector<std::thread> threads;
+  for (int i = 0; i < 10; ++i)
+    threads.emplace_back(foo);
+
+  for (int i = 0; i < 100; ++i)
+    __lsan_do_recoverable_leak_check();
+
+  done = true;
+  for (auto &t : threads)
+    t.join();
+
+  return 0;
+}