Updated MallocHooks to collect stack traces when memory is allocated.

runtime/vm/malloc_hooks.cc

Index: runtime/vm/malloc_hooks.cc
diff --git a/runtime/vm/malloc_hooks.cc b/runtime/vm/malloc_hooks.cc
index 849f8d5d56928ed4bb812cf0128808426c730578..b2fb5420a4ea5428434924094c871a7672c67bbf 100644
--- a/runtime/vm/malloc_hooks.cc
+++ b/runtime/vm/malloc_hooks.cc
@@ -13,9 +13,70 @@
 #include "platform/assert.h"
 #include "vm/hash_map.h"
 #include "vm/lockers.h"
+#include "vm/profiler.h"
 
 namespace dart {
 
+class AddressMap;
+
+// MallocHooksState contains all of the state related to the configuration of
+// the malloc hooks, allocation information, and locks.
+class MallocHooksState : public AllStatic {
+ public:
+  static void RecordAllocHook(const void* ptr, size_t size);
+  static void RecordFreeHook(const void* ptr);
+
+  static bool initialized() { return initialized_; }
+  static void Init();
+
+  static bool ProfilingEnabled() { return (OSThread::TryCurrent() != NULL); }
+
+  static bool IsOriginalProcess() {
+    ASSERT(original_pid_ != kInvalidPid);
+    return original_pid_ == OS::ProcessId();
+  }
+
+  static Mutex* malloc_hook_mutex() { return malloc_hook_mutex_; }
+
+  static intptr_t allocation_count() { return allocation_count_; }
+
+  static intptr_t heap_allocated_memory_in_bytes() {
+    return heap_allocated_memory_in_bytes_;
+  }
+
+  static void IncrementHeapAllocatedMemoryInBytes(intptr_t size) {
+    ASSERT(size >= 0);
+    heap_allocated_memory_in_bytes_ += size;
+    ++allocation_count_;
+  }
+
+  static void DecrementHeapAllocatedMemoryInBytes(intptr_t size) {
+    ASSERT(size >= 0);
+    ASSERT(heap_allocated_memory_in_bytes_ >= size);
+    heap_allocated_memory_in_bytes_ -= size;
+    --allocation_count_;
+    ASSERT(allocation_count_ >= 0);
+  }
+
+  static AddressMap* address_map() { return address_map_; }
+
+  static void ResetStats();
+  static void TearDown();
+
+ private:
+  static Mutex* malloc_hook_mutex_;
+
+  // Variables protected by malloc_hook_mutex_.
+  static bool initialized_;
+  static intptr_t allocation_count_;
+  static intptr_t heap_allocated_memory_in_bytes_;
+  static AddressMap* address_map_;
+  // End protected variables.
+
+  static intptr_t original_pid_;
+  static const intptr_t kInvalidPid = -1;
+};
+
 // A locker-type class to automatically grab and release the
 // in_malloc_hook_flag_.
 class MallocHookScope {
@@ -54,18 +115,46 @@ class MallocHookScope {
   DISALLOW_COPY_AND_ASSIGN(MallocHookScope);
 };
 
+// AllocationInfo contains all information related to a given allocation
+// including:
+//   -Allocation size in bytes
+//   -Stack trace corresponding to the location of allocation, if applicable
+class AllocationInfo {
+ public:
+  explicit AllocationInfo(intptr_t allocation_size)
+      : sample_(NULL), allocation_size_(allocation_size) {
+    // Stack trace collection is disabled when we are in the process of creating
+    // the first OSThread in order to prevent deadlocks.
+    if (MallocHooksState::ProfilingEnabled()) {
+      sample_ = Profiler::SampleNativeAllocation(kSkipCount);
+    }
+  }
+
+  Sample* sample() const { return sample_; }
+  intptr_t allocation_size() const { return allocation_size_; }
+
+ private:
+  Sample* sample_;
+  intptr_t allocation_size_;
+
+  // The number of frames that are generated by the malloc hooks and collection
+  // of the stack trace. These frames are ignored when collecting the stack
+  // trace for a memory allocation.
+  static const intptr_t kSkipCount = 5;

[zra, 2017/02/15 05:50:46]

Is this the right value for Debug and Release? ia32 and x64? gcc and clang?

[bkonyi, 2017/02/16 00:28:28]

I've just tested and this is the right value for debug and release, ia32 and
x64. I'm pretty sure we only define DART_USE_TCMALLOC in the .gn files, so I
don't think we install the malloc hooks when using gcc.

[zra, 2017/02/16 21:45:27]

ia32 is built with gcc on Linux, so we do use the malloc hooks with gcc.

Is there any assert we can place in the code to give an indication if this value
is wrong on some configuration? Could you set it to 4, for example, and then
check for an expected value in the first slot?

+};
+
 
 // Custom key/value trait specifically for address/size pairs. Unlike
 // RawPointerKeyValueTrait, the default value is -1 as 0 can be a valid entry.
-class AddressKeyValueTrait {
+class AddressKeyValueTrait : public AllStatic {
  public:
   typedef const void* Key;
-  typedef intptr_t Value;
+  typedef AllocationInfo* Value;
 
   struct Pair {
     Key key;
     Value value;
-    Pair() : key(NULL), value(-1) {}
+    Pair() : key(NULL), value(NULL) {}
     Pair(const Key key, const Value& value) : key(key), value(value) {}
     Pair(const Pair& other) : key(other.key), value(other.value) {}
   };
@@ -84,12 +173,14 @@ class AddressMap : public MallocDirectChainedHashMap<AddressKeyValueTrait> {
   typedef AddressKeyValueTrait::Value Value;
   typedef AddressKeyValueTrait::Pair Pair;
 
-  inline void Insert(const Key& key, const Value& value) {
+  virtual ~AddressMap() { Clear(); }
+
+  void Insert(const Key& key, const Value& value) {
     Pair pair(key, value);
     MallocDirectChainedHashMap<AddressKeyValueTrait>::Insert(pair);
   }
 
-  inline bool Lookup(const Key& key, Value* value) {
+  bool Lookup(const Key& key, Value* value) {
     ASSERT(value != NULL);
     Pair* pair = MallocDirectChainedHashMap<AddressKeyValueTrait>::Lookup(key);
     if (pair == NULL) {
@@ -99,75 +190,16 @@ class AddressMap : public MallocDirectChainedHashMap<AddressKeyValueTrait> {
       return true;
     }
   }
-};
-
-
-class MallocHooksState {
- public:
-  static void RecordAllocHook(const void* ptr, size_t size);
-  static void RecordFreeHook(const void* ptr);
 
-  static bool initialized() { return initialized_; }
-  static void Init() {
-    address_map_ = new AddressMap();
-    initialized_ = true;
-    original_pid_ = OS::ProcessId();
-  }
-
-  static bool IsOriginalProcess() {
-    ASSERT(original_pid_ != kInvalidPid);
-    return original_pid_ == OS::ProcessId();
-  }
-
-  static Mutex* malloc_hook_mutex() { return malloc_hook_mutex_; }
-
-  static intptr_t allocation_count() { return allocation_count_; }
-
-  static intptr_t heap_allocated_memory_in_bytes() {
-    return heap_allocated_memory_in_bytes_;
-  }
-
-  static void IncrementHeapAllocatedMemoryInBytes(intptr_t size) {
-    ASSERT(size >= 0);
-    heap_allocated_memory_in_bytes_ += size;
-    ++allocation_count_;
-  }
-
-  static void DecrementHeapAllocatedMemoryInBytes(intptr_t size) {
-    ASSERT(size >= 0);
-    ASSERT(heap_allocated_memory_in_bytes_ >= size);
-    heap_allocated_memory_in_bytes_ -= size;
-    --allocation_count_;
-    ASSERT(allocation_count_ >= 0);
-  }
-
-  static AddressMap* address_map() { return address_map_; }
-
-  static void ResetStats() {
-    allocation_count_ = 0;
-    heap_allocated_memory_in_bytes_ = 0;
-    address_map_->Clear();
-  }
-
-  static void TearDown() {
-    initialized_ = false;
-    original_pid_ = kInvalidPid;
-    ResetStats();
-    delete address_map_;
+  void Clear() {
+    Iterator iter = GetIterator();
+    Pair* result = iter.Next();
+    while (result != NULL) {
+      delete result->value;

[zra, 2017/02/15 05:50:46]

result->value = NULL;

[bkonyi, 2017/02/16 00:28:28]

Done.

+      result = iter.Next();
+    }
+    MallocDirectChainedHashMap<AddressKeyValueTrait>::Clear();
   }
-
- private:
-  static bool initialized_;
-  static intptr_t original_pid_;
-  static Mutex* malloc_hook_mutex_;
-  static intptr_t allocation_count_;
-  static intptr_t heap_allocated_memory_in_bytes_;
-  static AddressMap* address_map_;
-
-  static const intptr_t kInvalidPid = -1;
-
-  DISALLOW_ALLOCATION();
-  DISALLOW_COPY_AND_ASSIGN(MallocHooksState);
 };
 
 
@@ -215,7 +247,15 @@ void MallocHooks::TearDown() {
 }
 
 
+bool MallocHooks::ProfilingEnabled() {
+  return MallocHooksState::ProfilingEnabled();
+}
+
+
 void MallocHooks::ResetStats() {
+  // Set the malloc hook flag before completing the reset since ResetStats()
+  // frees memory.
+  MallocHookScope mhs;
   MutexLocker ml(MallocHooksState::malloc_hook_mutex());
   ASSERT(MallocHooksState::initialized());
 
@@ -240,6 +280,43 @@ intptr_t MallocHooks::heap_allocated_memory_in_bytes() {
 }
 
 
+Sample* MallocHooks::GetSample(const void* ptr) {
+  MutexLocker ml(MallocHooksState::malloc_hook_mutex());
+  ASSERT(MallocHooksState::initialized());
+
+  if (ptr != NULL) {
+    AllocationInfo* allocation_info = NULL;
+    if (MallocHooksState::address_map()->Lookup(ptr, &allocation_info)) {
+      ASSERT(allocation_info != NULL);
+      return allocation_info->sample();
+    }
+  }
+  return NULL;
+}
+
+
+void MallocHooksState::Init() {
+  address_map_ = new AddressMap();
+  initialized_ = true;
+  original_pid_ = OS::ProcessId();
+}
+
+
+void MallocHooksState::ResetStats() {
+  allocation_count_ = 0;
+  heap_allocated_memory_in_bytes_ = 0;
+  address_map_->Clear();
+}
+
+
+void MallocHooksState::TearDown() {
+  initialized_ = false;
+  original_pid_ = kInvalidPid;
+  ResetStats();
+  delete address_map_;

[zra, 2017/02/15 05:50:46]

address_map_ = NULL;

[bkonyi, 2017/02/16 00:28:28]

Done.

+}
+
+
 void MallocHooksState::RecordAllocHook(const void* ptr, size_t size) {
   if (MallocHookScope::IsInHook() || !MallocHooksState::IsOriginalProcess()) {
     return;
@@ -253,7 +330,7 @@ void MallocHooksState::RecordAllocHook(const void* ptr, size_t size) {
 
   if (ptr != NULL) {
     MallocHooksState::IncrementHeapAllocatedMemoryInBytes(size);
-    MallocHooksState::address_map()->Insert(ptr, size);
+    MallocHooksState::address_map()->Insert(ptr, new AllocationInfo(size));
   }
 }
 
@@ -270,10 +347,12 @@ void MallocHooksState::RecordFreeHook(const void* ptr) {
   ASSERT(MallocHooksState::initialized());
 
   if (ptr != NULL) {
-    intptr_t size = 0;
-    if (MallocHooksState::address_map()->Lookup(ptr, &size)) {
-      MallocHooksState::DecrementHeapAllocatedMemoryInBytes(size);
+    AllocationInfo* allocation_info = NULL;
+    if (MallocHooksState::address_map()->Lookup(ptr, &allocation_info)) {
+      MallocHooksState::DecrementHeapAllocatedMemoryInBytes(
+          allocation_info->allocation_size());
       MallocHooksState::address_map()->Remove(ptr);
+      delete allocation_info;
     }
   }
 }