Updated MallocHooks to collect stack traces when memory is allocated.

runtime/vm/malloc_hooks.cc

 // Copyright (c) 2017, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "platform/globals.h"
 
 #if defined(DART_USE_TCMALLOC) && !defined(PRODUCT)
 
 #include "vm/malloc_hooks.h"
 
 #include "gperftools/malloc_hook.h"
 
 #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 stack_trace_collection_enabled() {
+    return OSThread::DoesCurrentThreadExist();
+  }
+
+  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 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;
+};
+
 // A locker-type class to automatically grab and release the
 // in_malloc_hook_flag_.
 class MallocHookScope {
  public:
   static void InitMallocHookFlag() {
     ASSERT(in_malloc_hook_flag_ == kUnsetThreadLocalKey);
     in_malloc_hook_flag_ = OSThread::CreateThreadLocal();
     OSThread::SetThreadLocal(in_malloc_hook_flag_, 0);
   }
 
   static void DestroyMallocHookFlag() {
-    ASSERT(in_malloc_hook_flag_ != kUnsetThreadLocalKey);
     OSThread::DeleteThreadLocal(in_malloc_hook_flag_);
     in_malloc_hook_flag_ = kUnsetThreadLocalKey;
   }
 
   MallocHookScope() {
     ASSERT(in_malloc_hook_flag_ != kUnsetThreadLocalKey);
     OSThread::SetThreadLocal(in_malloc_hook_flag_, 1);
   }
 
   ~MallocHookScope() {
     ASSERT(in_malloc_hook_flag_ != kUnsetThreadLocalKey);
     OSThread::SetThreadLocal(in_malloc_hook_flag_, 0);
   }
 
   static bool IsInHook() {
     ASSERT(in_malloc_hook_flag_ != kUnsetThreadLocalKey);
     return OSThread::GetThreadLocal(in_malloc_hook_flag_);
   }
 
  private:
   static ThreadLocalKey in_malloc_hook_flag_;
 
   DISALLOW_ALLOCATION();
   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 (OSThread::DoesCurrentThreadExist()) {
+      sample_ = Profiler::NativeSampleAllocation();

[Cutch, 2017/02/08 00:28:53]

Naming nit:

This should be SampleNativeAllocation

Sample is the verb.

[bkonyi, 2017/02/08 01:37:05]

Done.

+    }
+  }
+
+  Sample* sample() const { return sample_; }
+  intptr_t allocation_size() const { return allocation_size_; }
+
+ private:
+  Sample* sample_;
+  intptr_t allocation_size_;
+};
+
 
 // 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) {}
   };
 
   static Key KeyOf(Pair kv) { return kv.key; }
   static Value ValueOf(Pair kv) { return kv.value; }
   static intptr_t Hashcode(Key key) { return reinterpret_cast<intptr_t>(key); }
   static bool IsKeyEqual(Pair kv, Key key) { return kv.key == key; }
 };
 
 
 // Map class that will be used to store mappings between ptr -> allocation size.
 class AddressMap : public MallocDirectChainedHashMap<AddressKeyValueTrait> {
  public:
   typedef AddressKeyValueTrait::Key Key;
   typedef AddressKeyValueTrait::Value Value;
   typedef AddressKeyValueTrait::Pair Pair;
 
+  ~AddressMap() { Clear(); }
+
   inline void Insert(const Key& key, const Value& value) {

[Cutch, 2017/02/08 00:28:53]

These are declared inline in this source file. No need to use the inline keyword
(here or below)

[bkonyi, 2017/02/08 01:37:05]

Acknowledged. Will be fixed in patch 3.

[bkonyi, 2017/02/09 21:22:46]

Done.

     Pair pair(key, value);
     MallocDirectChainedHashMap<AddressKeyValueTrait>::Insert(pair);
   }
 
   inline bool Lookup(const Key& key, Value* value) {
     ASSERT(value != NULL);
     Pair* pair = MallocDirectChainedHashMap<AddressKeyValueTrait>::Lookup(key);
     if (pair == NULL) {
       return false;
     } else {
       *value = pair->value;
       return true;
     }
   }
+
+  inline void Clear() {
+    Iterator iter = GetIterator();
+    Pair* result = iter.Next();
+    while (result != NULL) {
+      delete result->value;
+      result = iter.Next();
+    }
+    MallocDirectChainedHashMap<AddressKeyValueTrait>::Clear();
+  }
 };
 
 
-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_;
-  }
-
- 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);
-};
-
-
 // MallocHooks state / locks.
 ThreadLocalKey MallocHookScope::in_malloc_hook_flag_ = kUnsetThreadLocalKey;
 bool MallocHooksState::initialized_ = false;
 intptr_t MallocHooksState::original_pid_ = MallocHooksState::kInvalidPid;
 Mutex* MallocHooksState::malloc_hook_mutex_ = new Mutex();
 
 // Memory allocation state information.
 intptr_t MallocHooksState::allocation_count_ = 0;
 intptr_t MallocHooksState::heap_allocated_memory_in_bytes_ = 0;
 AddressMap* MallocHooksState::address_map_ = NULL;
@@ skipping 24 matching lines @@
   success = MallocHook::RemoveNewHook(&MallocHooksState::RecordAllocHook);
   ASSERT(success);
   success = MallocHook::RemoveDeleteHook(&MallocHooksState::RecordFreeHook);
   ASSERT(success);
 
   MallocHooksState::TearDown();
   MallocHookScope::DestroyMallocHookFlag();
 }
 
 
+bool MallocHooks::stack_trace_collection_enabled() {
+  return MallocHooksState::stack_trace_collection_enabled();
+}
+
+
 void MallocHooks::ResetStats() {
+  // Set the malloc hook flag before completing the reset since ResetStats()
+  // frees memory.
+  MallocHookScope mhs;

[zra, 2017/02/08 17:42:55]

If this is a fix for an existing bug, we should pull it out into its own CL and
land it first.

[bkonyi, 2017/02/09 21:22:46]

This isn't a fix for an existing bug since ResetStats() didn't free memory
before this change.

   MutexLocker ml(MallocHooksState::malloc_hook_mutex());
   ASSERT(MallocHooksState::initialized());
 
   MallocHooksState::ResetStats();
 }
 
 
 bool MallocHooks::Initialized() {
   return MallocHooksState::initialized();
 }
 
 
 intptr_t MallocHooks::allocation_count() {
   MutexLocker ml(MallocHooksState::malloc_hook_mutex());
   return MallocHooksState::allocation_count();
 }
 
 
 intptr_t MallocHooks::heap_allocated_memory_in_bytes() {
   MutexLocker ml(MallocHooksState::malloc_hook_mutex());
   return MallocHooksState::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_;
+}
+
+
 void MallocHooksState::RecordAllocHook(const void* ptr, size_t size) {
   if (MallocHookScope::IsInHook() || !MallocHooksState::IsOriginalProcess()) {
     return;
   }
 
   // Set the malloc hook flag before grabbing the mutex to avoid calling hooks
   // again.
   MallocHookScope mhs;
   MutexLocker ml(MallocHooksState::malloc_hook_mutex());
   ASSERT(MallocHooksState::initialized());
 
   if (ptr != NULL) {
     MallocHooksState::IncrementHeapAllocatedMemoryInBytes(size);
-    MallocHooksState::address_map()->Insert(ptr, size);
+    MallocHooksState::address_map()->Insert(ptr, new AllocationInfo(size));
   }
 }
 
 
 void MallocHooksState::RecordFreeHook(const void* ptr) {
   if (MallocHookScope::IsInHook() || !MallocHooksState::IsOriginalProcess()) {
     return;
   }
 
   // Set the malloc hook flag before grabbing the mutex to avoid calling hooks
   // again.
   MallocHookScope mhs;
   MutexLocker ml(MallocHooksState::malloc_hook_mutex());
   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;
     }
   }
 }
 
 }  // namespace dart
 
 #endif  // defined(DART_USE_TCMALLOC) && !defined(PRODUCT)