Stack sampling profiler: handle unloaded and unloading modules

base/profiler/native_stack_sampler_win.cc

Index: base/profiler/native_stack_sampler_win.cc
diff --git a/base/profiler/native_stack_sampler_win.cc b/base/profiler/native_stack_sampler_win.cc
index 203ab94053c5094d6ffed6863f586081cb40dbc4..167dc25e0a9364350a7c90c2336f925650382431 100644
--- a/base/profiler/native_stack_sampler_win.cc
+++ b/base/profiler/native_stack_sampler_win.cc
@@ -140,13 +140,14 @@ void RewritePointersToStackMemory(uintptr_t top, uintptr_t bottom,
 // Walks the stack represented by |context| from the current frame downwards,
 // recording the instruction pointers for each frame in |instruction_pointers|.
 int RecordStack(CONTEXT* context, int max_stack_size,
-                const void* instruction_pointers[]) {
+                const void* instruction_pointers[],
+                ScopedModuleHandle modules[]) {

[brucedawson, 2015/10/30 00:15:59]

Having array function parameters is frustrating because, of course, they come in
as pointers rather than arrays, so you have to pass in the max_stack_size value.

This may be out-of-scope for this change but there is a solution which is to use
array references, like this:

    const void* (&instruction_pointers)[max_stack_size],
    ScopedModuleHandle (&modules)[max_stack_size]

where max_stack_size is not a function parameter (that parameter goes away) but
instead is an integral constant. It reduces flexibility (unless you template on
the size) but it means you get to use ARRAYSIZE() on |instruction_pointers| and
|modules|. Calling the function is simpler because you don't have to pass in the
size, and calling the function incorrectly is impossible - a wonderful feature.

Food for thought.

[Mike Wittman, 2015/10/30 17:08:07]

That's a nice alternative, but unfortunately it would violate the style guide
here since mutable reference parameters are prohibited. I suppose the parameters
could be passed by pointer but that might be more confusing than its worth.

In any case, these will probably get changed to vectors in a future CL since
this code no longer is restricted from doing memory allocation.

 #ifdef _WIN64
   Win32StackFrameUnwinder frame_unwinder;
   int i = 0;
   for (; (i < max_stack_size) && context->Rip; ++i) {
     instruction_pointers[i] = reinterpret_cast<const void*>(context->Rip);
-    if (!frame_unwinder.TryUnwind(context))
+    if (!frame_unwinder.TryUnwind(context, &modules[i]))
       return i + 1;
   }
   return i;
@@ -155,35 +156,6 @@ int RecordStack(CONTEXT* context, int max_stack_size,
 #endif
 }
 
-// Fills in |module_handles| corresponding to the pointers to code in
-// |addresses|. The module handles are returned with reference counts
-// incremented and should be freed with FreeModuleHandles. See note in
-// SuspendThreadAndRecordStack for why |addresses| and |module_handles| are
-// arrays.
-void FindModuleHandlesForAddresses(const void* const addresses[],
-                                   HMODULE module_handles[], int stack_depth) {
-  for (int i = 0; i < stack_depth; ++i) {
-    HMODULE module_handle = NULL;
-    if (GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS,
-                          reinterpret_cast<LPCTSTR>(addresses[i]),
-                          &module_handle)) {
-      // HMODULE actually represents the base address of the module, so we can
-      // use it directly as an address.
-      DCHECK_LE(reinterpret_cast<const void*>(module_handle), addresses[i]);
-      module_handles[i] = module_handle;
-    }
-  }
-}
-
-// Frees the modules handles returned by FindModuleHandlesForAddresses. See note
-// in SuspendThreadAndRecordStack for why |module_handles| is an array.
-void FreeModuleHandles(int stack_depth, HMODULE module_handles[]) {
-  for (int i = 0; i < stack_depth; ++i) {
-    if (module_handles[i])
-      ::FreeLibrary(module_handles[i]);
-  }
-}
-
 // Gets the unique build ID for a module. Windows build IDs are created by a
 // concatenation of a GUID and AGE fields found in the headers of a module. The
 // GUID is stored in the first 16 bytes and the AGE is stored in the last 4
@@ -300,7 +272,9 @@ int SuspendThreadAndRecordStack(HANDLE thread_handle,
                                 void* stack_copy_buffer,
                                 size_t stack_copy_buffer_size,
                                 int max_stack_size,
-                                const void* instruction_pointers[]) {
+                                const void* instruction_pointers[],
+                                ScopedModuleHandle modules[],
+                                NativeStackSamplerTestDelegate* test_delegate) {
   CONTEXT thread_context = {0};
   thread_context.ContextFlags = CONTEXT_FULL;
   // The stack bounds are saved to uintptr_ts for use outside
@@ -330,16 +304,21 @@ int SuspendThreadAndRecordStack(HANDLE thread_handle,
                 top - bottom);
   }
 
+  if (test_delegate)
+    test_delegate->OnPreStackWalk();
+
   RewritePointersToStackMemory(top, bottom, &thread_context, stack_copy_buffer);
 
-  return RecordStack(&thread_context, max_stack_size, instruction_pointers);
+  return RecordStack(&thread_context, max_stack_size, instruction_pointers,
+                     modules);
 }
 
 // NativeStackSamplerWin ------------------------------------------------------
 
 class NativeStackSamplerWin : public NativeStackSampler {
  public:
-  explicit NativeStackSamplerWin(win::ScopedHandle thread_handle);
+  NativeStackSamplerWin(win::ScopedHandle thread_handle,
+                        NativeStackSamplerTestDelegate* test_delegate);
   ~NativeStackSamplerWin() override;
 
   // StackSamplingProfiler::NativeStackSampler:
@@ -373,13 +352,15 @@ class NativeStackSamplerWin : public NativeStackSampler {
   // Copies the stack information represented by |instruction_pointers| into
   // |sample| and |modules|.
   void CopyToSample(const void* const instruction_pointers[],
-                    const HMODULE module_handles[],
+                    const ScopedModuleHandle module_handles[],
                     int stack_depth,
                     StackSamplingProfiler::Sample* sample,
                     std::vector<StackSamplingProfiler::Module>* modules);
 
   win::ScopedHandle thread_handle_;
 
+  NativeStackSamplerTestDelegate* const test_delegate_;
+
   // The stack base address corresponding to |thread_handle_|.
   const void* const thread_stack_base_address_;
 
@@ -398,8 +379,10 @@ class NativeStackSamplerWin : public NativeStackSampler {
   DISALLOW_COPY_AND_ASSIGN(NativeStackSamplerWin);
 };
 
-NativeStackSamplerWin::NativeStackSamplerWin(win::ScopedHandle thread_handle)
-    : thread_handle_(thread_handle.Take()),
+NativeStackSamplerWin::NativeStackSamplerWin(
+    win::ScopedHandle thread_handle,
+    NativeStackSamplerTestDelegate* test_delegate)
+    : thread_handle_(thread_handle.Take()), test_delegate_(test_delegate),
       thread_stack_base_address_(
           GetThreadEnvironmentBlock(thread_handle_.Get())->Tib.StackBase),
       stack_copy_buffer_(new unsigned char[kStackCopyBufferSize]) {
@@ -423,19 +406,18 @@ void NativeStackSamplerWin::RecordStackSample(
 
   const int max_stack_size = 64;
   const void* instruction_pointers[max_stack_size] = {0};
-  HMODULE module_handles[max_stack_size] = {0};
+  ScopedModuleHandle modules[max_stack_size];
 
   int stack_depth = SuspendThreadAndRecordStack(thread_handle_.Get(),
                                                 thread_stack_base_address_,
                                                 stack_copy_buffer_.get(),
                                                 kStackCopyBufferSize,
                                                 max_stack_size,
-                                                instruction_pointers);
-  FindModuleHandlesForAddresses(instruction_pointers, module_handles,
-                                stack_depth);
-  CopyToSample(instruction_pointers, module_handles, stack_depth, sample,
+                                                instruction_pointers,
+                                                modules,
+                                                test_delegate_);
+  CopyToSample(instruction_pointers, modules, stack_depth, sample,
                current_modules_);
-  FreeModuleHandles(stack_depth, module_handles);
 }
 
 void NativeStackSamplerWin::ProfileRecordingStopped() {
@@ -484,24 +466,25 @@ size_t NativeStackSamplerWin::GetModuleIndex(
 
 void NativeStackSamplerWin::CopyToSample(
     const void* const instruction_pointers[],
-    const HMODULE module_handles[],
+    const ScopedModuleHandle module_handles[],
     int stack_depth,
     StackSamplingProfiler::Sample* sample,
-    std::vector<StackSamplingProfiler::Module>* module) {
+    std::vector<StackSamplingProfiler::Module>* modules) {
   sample->clear();
   sample->reserve(stack_depth);
 
   for (int i = 0; i < stack_depth; ++i) {
     sample->push_back(StackSamplingProfiler::Frame(
         reinterpret_cast<uintptr_t>(instruction_pointers[i]),
-        GetModuleIndex(module_handles[i], module)));
+        GetModuleIndex(module_handles[i].Get(), modules)));
   }
 }
 
 }  // namespace
 
 scoped_ptr<NativeStackSampler> NativeStackSampler::Create(
-    PlatformThreadId thread_id) {
+    PlatformThreadId thread_id,
+    NativeStackSamplerTestDelegate* test_delegate) {
 #if _WIN64
   // Get the thread's handle.
   HANDLE thread_handle = ::OpenThread(
@@ -511,7 +494,8 @@ scoped_ptr<NativeStackSampler> NativeStackSampler::Create(
 
   if (thread_handle) {
     return scoped_ptr<NativeStackSampler>(new NativeStackSamplerWin(
-        win::ScopedHandle(thread_handle)));
+        win::ScopedHandle(thread_handle),
+        test_delegate));
   }
 #endif
   return scoped_ptr<NativeStackSampler>();