[PATCH] D87120: [HeapProf] Heap profiling runtime support

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: compiler-rt/lib/heapprof/heapprof_allocator.cpp:689
+      return reinterpret_cast<HeapprofChunk *>(alloc_magic[1]);
+    return reinterpret_cast<HeapprofChunk *>(alloc_beg);
+  }
----------------
tejohnson wrote:
> vitalybuka wrote:
> > AsanChunk is used in such way that any garbage works there.
> > Here we return alloc_beg but we have no prove that it's initialized. 
> > There is a data race by design, I guess, when lsan can start using that data when it's not yet fully initialized.
> > lsan has sanity checks for all fields so it should not crash/hang and in the worst case it just misses memory leaks. 
> > However If block is alive for user or it's in quarantine then chunk is valid and lsan will do correct leak checking.
> > I can imagine that author tried to avoid locking between lsan and fast thread-local allocator cache.
> > 
> > Is it true for heapprof?
> > Looks like heapprof_mz_size can get here. Can valid, data race free, program get into GetHeapprofChunk in one thread for a memory block which is being allocated/deallocated by another thread? 
> > 
> > BTW. Here is my another Asan patch to avoid metadata and more importantly make sure header is in consistent state D87359
> > 
> The user_requested_size is set to 0 on deallocation (min size is 1 per Allocate). All of the callers of this function do specifically check for a 0 user_requested_size and handle it specially.
If block is allocated from primary allocator, there is not guaranty that it's zeroed. E.g. allocator may uses that mem to maintain free list.
So user_requested_size may not be 0 just after return from allocator.Allocate
If HeapprofChunk is alloc_beg then we have no kAllocBegMagic and GetHeapprofChunk will return this not yet initialized HeapprofChunk.

For the secondary it's not a problem as it's from mmap so it's zero-ed.
For the primary with Magic also should not be a problem (if we fix Allocate to set Magic after initialization)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87120/new/

https://reviews.llvm.org/D87120