[Memprof] Fixes memory leak in MemInfoBlock histogram.

[mattweingarten]

MemInfoBlocks (MIB) with empty callstacks are erased prematurely from the CallStackProfileData. This patch frees allocated histogram buffers when the MIB is associated with an empty callstack.

[llvmbot]

@llvm/pr-subscribers-pgo

Author: Matthew Weingarten (mattweingarten)

Changes

MemInfoBlocks (MIB) with empty callstacks are erased prematurely from the CallStackProfileData. This patch frees allocated histogram buffers when the MIB is associated with an empty callstack.

---

Full diff: https://github.com/llvm/llvm-project/pull/96834.diff

1 Files Affected:

• (modified) llvm/lib/ProfileData/MemProfReader.cpp (+2)

diff --git a/llvm/lib/ProfileData/MemProfReader.cpp b/llvm/lib/ProfileData/MemProfReader.cpp
index 85327273d6d7b..5e2d76e24dab7 100644
--- a/llvm/lib/ProfileData/MemProfReader.cpp
+++ b/llvm/lib/ProfileData/MemProfReader.cpp
@@ -633,6 +633,8 @@ Error RawMemProfReader::symbolizeAndFilterStackFrames(
   // Drop the entries where the callstack is empty.
   for (const uint64_t Id : EntriesToErase) {
     StackMap.erase(Id);
+    if(CallstackProfileData[Id].AccessHistogramSize > 0)
+      free((void*) CallstackProfileData[Id].AccessHistogram);
     CallstackProfileData.erase(Id);
   }
 

[snehasish]

This seems like a bug which could happen elsewhere too. What do you think about adding a destructor to the MemInfoBlock which releases any allocated memory if the histogram size > 0?

[mattweingarten]

Yea I was thinking about that too. The problem is you want to keep the two files the same:

llvm/include/llvm/ProfileData/MemProfData.inc and
compiler-rt/include/profile/MemProfData.inc.

If you add the stuff in the destructor, they will have to diverge because they use different allocators (one with malloc, the other with InternalAlloc.

[mattweingarten]

Unless we don't mind that they are not the exact same file?

[snehasish]

No, that makes sense. If we don't free the memory with delete (which is the case in the runtime) then it's not useful. Leaving as it is seems fine.