[Sampling profiler] Add Unwinder::UpdateModules()

Adds an interface to allow unwinders to update the modules they're
responsible for in the ModuleCache at each sample, prior to unwinding. To
be used by the V8 unwinder after updating to the new V8 async unwinding
API, to update its current set of modules after they are recorded in
OnStackCapture().

Bug: 1035855
Change-Id: If6fb08fb91323d748facab0557e02a8f2a971c5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2086297
Commit-Queue: Mike Wittman <wittman@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747521}

base/profiler/stack_sampler_impl.cc

@@ -31,10 +31,12 @@ namespace {
 // the thread is suspended.
 class StackCopierDelegate : public StackCopier::Delegate {
  public:
-  StackCopierDelegate(Unwinder* native_unwinder,
+  StackCopierDelegate(ModuleCache* module_cache,
+                      Unwinder* native_unwinder,
                       Unwinder* aux_unwinder,
                       ProfileBuilder* profile_builder)
-      : native_unwinder_(native_unwinder),
+      : module_cache_(module_cache),
+        native_unwinder_(native_unwinder),
         aux_unwinder_(aux_unwinder),
         profile_builder_(profile_builder),
         metadata_provider_(
@@ -66,9 +68,14 @@ class StackCopierDelegate : public StackCopier::Delegate {
     // Reset this as soon as possible because it may hold a lock on the
     // metadata.
     metadata_provider_.reset();
+
+    native_unwinder_->UpdateModules(module_cache_);
+    if (aux_unwinder_)
+      aux_unwinder_->UpdateModules(module_cache_);
   }
 
  private:
+  ModuleCache* const module_cache_;
   Unwinder* const native_unwinder_;
   Unwinder* const aux_unwinder_;
   ProfileBuilder* const profile_builder_;
@@ -100,8 +107,8 @@ void StackSamplerImpl::RecordStackFrames(StackBuffer* stack_buffer,
   RegisterContext thread_context;
   uintptr_t stack_top;
   TimeTicks timestamp;
-  StackCopierDelegate delegate(native_unwinder_.get(), aux_unwinder_.get(),
-                               profile_builder);
+  StackCopierDelegate delegate(module_cache_, native_unwinder_.get(),
+                               aux_unwinder_.get(), profile_builder);
   bool success = stack_copier_->CopyStack(stack_buffer, &stack_top, &timestamp,
                                           &thread_context, &delegate);
   if (!success)

base/profiler/stack_sampler_impl_unittest.cc

@@ -88,6 +88,20 @@ class TestStackCopier : public StackCopier {
   const TimeTicks timestamp_;
 };
 
+// A StackCopier that just invokes the expected functions on the delegate.
+class DelegateInvokingStackCopier : public StackCopier {
+ public:
+  bool CopyStack(StackBuffer* stack_buffer,
+                 uintptr_t* stack_top,
+                 TimeTicks* timestamp,
+                 RegisterContext* thread_context,
+                 Delegate* delegate) override {
+    delegate->OnStackCopy();
+    delegate->OnThreadResume();
+    return true;
+  }
+};
+
 // Trivial unwinder implementation for testing.
 class TestUnwinder : public Unwinder {
  public:
@@ -125,6 +139,37 @@ class TestUnwinder : public Unwinder {
   uintptr_t* stack_copy_bottom_;
 };
 
+// Records invocations of calls to OnStackCapture()/UpdateModules().
+class CallRecordingUnwinder : public Unwinder {
+ public:
+  void OnStackCapture() override { on_stack_capture_was_invoked_ = true; }
+
+  void UpdateModules(ModuleCache*) override {
+    update_modules_was_invoked_ = true;
+  }
+
+  bool CanUnwindFrom(const Frame* current_frame) const override { return true; }
+
+  UnwindResult TryUnwind(RegisterContext* thread_context,
+                         uintptr_t stack_top,
+                         ModuleCache* module_cache,
+                         std::vector<Frame>* stack) const override {
+    return UnwindResult::UNRECOGNIZED_FRAME;
+  }
+
+  bool on_stack_capture_was_invoked() const {
+    return on_stack_capture_was_invoked_;
+  }
+
+  bool update_modules_was_invoked() const {
+    return update_modules_was_invoked_;
+  }
+
+ private:
+  bool on_stack_capture_was_invoked_ = false;
+  bool update_modules_was_invoked_ = false;
+};
+
 class TestModule : public ModuleCache::Module {
  public:
   TestModule(uintptr_t base_address, size_t size, bool is_native = true)
@@ -266,6 +311,40 @@ TEST(StackSamplerImplTest, CopyStackTimestamp) {
   EXPECT_EQ(timestamp, profile_builder.last_timestamp());
 }
 
+TEST(StackSamplerImplTest, UnwinderInvokedWhileRecordingStackFrames) {
+  std::unique_ptr<StackBuffer> stack_buffer = std::make_unique<StackBuffer>(10);
+  auto owned_unwinder = std::make_unique<CallRecordingUnwinder>();
+  CallRecordingUnwinder* unwinder = owned_unwinder.get();
+  ModuleCache module_cache;
+  TestProfileBuilder profile_builder(&module_cache);
+  StackSamplerImpl stack_sampler_impl(
+      std::make_unique<DelegateInvokingStackCopier>(),
+      std::move(owned_unwinder), &module_cache);
+
+  stack_sampler_impl.RecordStackFrames(stack_buffer.get(), &profile_builder);
+
+  EXPECT_TRUE(unwinder->on_stack_capture_was_invoked());
+  EXPECT_TRUE(unwinder->update_modules_was_invoked());
+}
+
+TEST(StackSamplerImplTest, AuxUnwinderInvokedWhileRecordingStackFrames) {
+  std::unique_ptr<StackBuffer> stack_buffer = std::make_unique<StackBuffer>(10);
+  ModuleCache module_cache;
+  TestProfileBuilder profile_builder(&module_cache);
+  StackSamplerImpl stack_sampler_impl(
+      std::make_unique<DelegateInvokingStackCopier>(),
+      std::make_unique<CallRecordingUnwinder>(), &module_cache);
+
+  auto owned_aux_unwinder = std::make_unique<CallRecordingUnwinder>();
+  CallRecordingUnwinder* aux_unwinder = owned_aux_unwinder.get();
+  stack_sampler_impl.AddAuxUnwinder(std::move(owned_aux_unwinder));
+
+  stack_sampler_impl.RecordStackFrames(stack_buffer.get(), &profile_builder);
+
+  EXPECT_TRUE(aux_unwinder->on_stack_capture_was_invoked());
+  EXPECT_TRUE(aux_unwinder->update_modules_was_invoked());
+}
+
 TEST(StackSamplerImplTest, WalkStack_Completed) {
   ModuleCache module_cache;
   RegisterContext thread_context;

base/profiler/unwinder.h

@@ -47,6 +47,11 @@ class Unwinder {
   // including indirectly via use of DCHECK/CHECK or other logging statements.
   virtual void OnStackCapture() {}
 
+  // Allows the unwinder to update ModuleCache with any modules it's responsible
+  // for. Invoked for each sample between OnStackCapture() and the initial
+  // invocations of CanUnwindFrom()/TryUnwind().
+  virtual void UpdateModules(ModuleCache* module_cache) {}
+
   // Returns true if the unwinder recognizes the code referenced by
   // |current_frame| as code from which it should be able to unwind. When
   // multiple unwinders are in use, each should return true for a disjoint set