[tracing] Fix mising thread names in heap profiler

base/trace_event/heap_profiler_allocation_context_tracker.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/trace_event/heap_profiler_allocation_context_tracker.h"
 
 #include <algorithm>
 #include <iterator>
 
 #include "base/atomicops.h"
+#include "base/threading/platform_thread.h"
 #include "base/threading/thread_local_storage.h"
 #include "base/trace_event/heap_profiler_allocation_context.h"
 
+#if defined(OS_LINUX) || defined(OS_ANDROID)
+#include <sys/prctl.h>
+#endif
+
 namespace base {
 namespace trace_event {
 
 subtle::Atomic32 AllocationContextTracker::capture_mode_ =
     static_cast<int32_t>(AllocationContextTracker::CaptureMode::DISABLED);
 
 namespace {
 
 const size_t kMaxStackDepth = 128u;
 const size_t kMaxTaskDepth = 16u;
 AllocationContextTracker* const kInitializingSentinel =
     reinterpret_cast<AllocationContextTracker*>(-1);
 const char kTracingOverhead[] = "tracing_overhead";
 
 ThreadLocalStorage::StaticSlot g_tls_alloc_ctx_tracker = TLS_INITIALIZER;
 
 // This function is added to the TLS slot to clean up the instance when the
 // thread exits.
 void DestructAllocationContextTracker(void* alloc_ctx_tracker) {
   delete static_cast<AllocationContextTracker*>(alloc_ctx_tracker);
 }
 
+// Gets the thread name from kernel if available or returns a string with id.
+// This function intenionally leaks the allocated strings since they are used to
+// tag allocations even after the thread dies.
+const char* GetThreadName() {

[Primiano Tucci (use gerrit), 2016/05/17 19:38:43]

can you make this a void GetThreadName(char* out_name, size_t size) and move the
strdup down where you use it?
I can be ok with the strdup but I fear that in future somebody else calls this
and we multiply the leaks. 

[Dmitry Skiba, 2016/05/17 20:05:18]

Let's simply call function LeakThreadName(), and put annotation there? Right now
function knows how much buffer space it needs, by adding name/size arguments you
require callers to care about function's internals  (to justify 16 in your
example below).

[Primiano Tucci (use gerrit), 2016/05/17 20:46:12]

You could pass a std::string* as arugment and leak that.
Anyways LGTM both ways (either with the Leak in the name or leaking a
std::string).
You guys figure out. Just remember the annotation :)

[ssid, 2016/05/18 03:38:05]

Changed the name and added ANNOTATE_LEAKING_OBJECT_PTR.

+  char name[16];
+#if defined(OS_LINUX) || defined(OS_ANDROID)
+  // If the thread name is not set, try to get it from prctl. Thread name might
+  // not be set in cases where the thread started before heap profiling was
+  // enabled.
+  int err = prctl(PR_GET_NAME, name);
+  if (!err) {
+    return strdup(name);

[Primiano Tucci (use gerrit), 2016/05/17 19:38:44]

once you move this, make sure you include leak_annotations.h and use
ANNOTATE_LEAKING_OBJECT_PTR(duped_str_ptr).
Otherwise your patch will be reverted as soon as it hits the asan bots :)

[ssid, 2016/05/18 03:38:05]

Done.

+  }
+#endif  // defined(OS_LINUX) || defined(OS_ANDROID)
+
+  // Use tid if we don't have a thread name.
+  snprintf(name, sizeof(name), "%lu",
+           static_cast<unsigned long>(PlatformThread::CurrentId()));
+  return strdup(name);

[Primiano Tucci (use gerrit), 2016/05/17 19:38:43]

instead of strduping twice what you want to do below is:

if(!thread_name_) {
 thread_name = new char[16];
 ANNOTATE_LEAKING_OBJECT_PTR(thread_name_);
 GetThreadName(&thread_name, sizeof(thread_name))
}

[ssid, 2016/05/18 03:38:05]

Keepin strdup twice.

+}
+
 }  // namespace
 
 // static
 AllocationContextTracker*
 AllocationContextTracker::GetInstanceForCurrentThread() {
   AllocationContextTracker* tracker =
       static_cast<AllocationContextTracker*>(g_tls_alloc_ctx_tracker.Get());
   if (tracker == kInitializingSentinel)
     return nullptr;  // Re-entrancy case.
 
@@ skipping 89 matching lines @@
     ctx.backtrace.frame_count = 1;
     return ctx;
   }
 
   CaptureMode mode = static_cast<CaptureMode>(
       subtle::NoBarrier_Load(&capture_mode_));
 
   auto backtrace = std::begin(ctx.backtrace.frames);
   auto backtrace_end = std::end(ctx.backtrace.frames);
 
-  // Add the thread name as the first entry
+  if (!thread_name_) {
+    // Ignore the string allocation made by GetThreadName to avoid reentrancy.
+    ignore_scope_depth_++;
+    thread_name_ = GetThreadName();
+    DCHECK(thread_name_);
+    ignore_scope_depth_--;
+  }
+
+  // Add the thread name as the first entry in pseudo stack.
   if (thread_name_) {
     *backtrace++ = StackFrame::FromThreadName(thread_name_);
   }
 
   switch (mode) {
     case CaptureMode::DISABLED:
       {
         break;
       }
     case CaptureMode::PSEUDO_STACK:
@@ skipping 42 matching lines @@
 
   // TODO(ssid): Fix crbug.com/594803 to add file name as 3rd dimension
   // (component name) in the heap profiler and not piggy back on the type name.
   ctx.type_name = task_contexts_.empty() ? nullptr : task_contexts_.back();
 
   return ctx;
 }
 
 }  // namespace trace_event
 }  // namespace base