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

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

Thanks for taking a look. A couple questions/comments below. I'll address the other ones.



================
Comment at: compiler-rt/lib/heapprof/heapprof_allocator.cpp:62-113
+// The memory chunk allocated from the underlying allocator looks like this:
+// H H U U U U U U
+//   H -- ChunkHeader (32 bytes)
+//   U -- user memory.
+
+// If there is left padding before the ChunkHeader (due to use of memalign),
+// we store a magic value in the first uptr word of the memory block and
----------------
vitalybuka wrote:
> All this ChunkHeader stuff is Asan optimization hack to reuse redzones.
> There is metatada support in underlying allocators.
> Please take a look at hwasan_allocator
Note that this serves a specific purpose in the heap allocator as well (and in fact having a 32-byte header was part of the original design). It allows us to support a 64-byte shadow counter granularity with only a 32-byte alignment, so less overhead to avoid counter aliasing.


================
Comment at: compiler-rt/lib/heapprof/heapprof_allocator.cpp:561
+    }
+    if (using_primary_allocator) {
+      CHECK(size);
----------------
vitalybuka wrote:
> there is also asan specific inconsistency
> primiary/secondary difference is because primiary is reuse redzones and secondary not
> unfortunately asan is the worst example. 
> msan, hwasan and lsan have less unneeded stuff
Can you clarify why this isn't needed here? I do see references to the primary/secondary allocator in both hwasan and lsan. In fact lsan has code that conditionally memsets to 0 if it is from primary (similar to what is further below here in asan).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87120