[Sampling profiler] Add integration test for v8 stack unwinds

Checks that walking stacks from C++ through JavaScript and back to C++
works as expected.

Bug: 909957
Change-Id: Ib881d082990df701c7f22da63cff89880b97764b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1590607
Commit-Queue: Mike Wittman <wittman@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Charlie Andrews <charliea@chromium.org>
Auto-Submit: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#657553}

base/profiler/stack_sampling_profiler_test_util.cc

@@ -4,10 +4,13 @@
 
 #include "base/profiler/stack_sampling_profiler_test_util.h"
 
+#include <utility>
+
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/location.h"
 #include "base/profiler/stack_sampling_profiler.h"
+#include "base/profiler/unwinder.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/bind_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -149,7 +152,8 @@ void WithTargetThread(UnwindScenario* scenario,
 }
 
 std::vector<Frame> SampleScenario(UnwindScenario* scenario,
-                                  ModuleCache* module_cache) {
+                                  ModuleCache* module_cache,
+                                  UnwinderFactory aux_unwinder_factory) {
   StackSamplingProfiler::SamplingParams params;
   params.sampling_interval = TimeDelta::FromMilliseconds(0);
   params.samples_per_profile = 1;
@@ -169,6 +173,8 @@ std::vector<Frame> SampleScenario(UnwindScenario* scenario,
                   sample = std::move(result_sample);
                   sampling_thread_completed.Signal();
                 })));
+        if (aux_unwinder_factory)
+          profiler.AddAuxUnwinder(std::move(aux_unwinder_factory).Run());
         profiler.Start();
         sampling_thread_completed.Wait();
       }));

base/profiler/stack_sampling_profiler_test_util.h

@@ -5,6 +5,7 @@
 #ifndef BASE_PROFILER_STACK_SAMPLING_PROFILER_TEST_UTIL_H_
 #define BASE_PROFILER_STACK_SAMPLING_PROFILER_TEST_UTIL_H_
 
+#include <memory>
 #include <vector>
 
 #include "base/callback.h"
@@ -14,6 +15,8 @@
 
 namespace base {
 
+class Unwinder;
+
 // A thread to target for profiling that will run the supplied closure.
 class TargetThread : public PlatformThread::Delegate {
  public:
@@ -96,9 +99,13 @@ using ProfileCallback = OnceCallback<void(PlatformThreadId)>;
 void WithTargetThread(UnwindScenario* scenario,
                       ProfileCallback profile_callback);
 
+using UnwinderFactory = OnceCallback<std::unique_ptr<Unwinder>()>;
+
 // Returns the sample seen when taking one sample of |scenario|.
-std::vector<Frame> SampleScenario(UnwindScenario* scenario,
-                                  ModuleCache* module_cache);
+std::vector<Frame> SampleScenario(
+    UnwindScenario* scenario,
+    ModuleCache* module_cache,
+    UnwinderFactory aux_unwinder_factory = UnwinderFactory());
 
 // Formats a sample into a string that can be output for test diagnostics.
 std::string FormatSampleForDiagnosticOutput(const std::vector<Frame>& sample);

chrome/renderer/v8_unwinder.cc

@@ -11,15 +11,6 @@
 
 namespace {
 
-// Synthetic build ids to use for V8 modules. The difference is in the digit
-// after the leading 5's.
-// clang-format off
-const char kV8EmbeddedCodeRangeBuildId[] =
-    "5555555507284E1E874EFA4EB754964B999";
-const char kV8CodeRangeBuildId[] =
-    "5555555517284E1E874EFA4EB754964B999";
-// clang-format on
-
 class V8Module : public base::ModuleCache::Module {
  public:
   V8Module(const v8::MemoryRange& memory_range,
@@ -110,3 +101,12 @@ base::UnwindResult V8Unwinder::TryUnwind(
 
   return base::UnwindResult::UNRECOGNIZED_FRAME;
 }
+
+// Synthetic build ids to use for V8 modules. The difference is in the digit
+// after the leading 5's.
+// clang-format off
+const char V8Unwinder::kV8EmbeddedCodeRangeBuildId[] =
+    "5555555507284E1E874EFA4EB754964B999";
+const char V8Unwinder::kV8CodeRangeBuildId[] =
+    "5555555517284E1E874EFA4EB754964B999";
+// clang-format on

chrome/renderer/v8_unwinder.h

@@ -27,6 +27,10 @@ class V8Unwinder : public base::Unwinder {
                                base::ModuleCache* module_cache,
                                std::vector<base::Frame>* stack) const override;
 
+  // Build ids generated by the unwinder. Exposed for test use.
+  static const char kV8EmbeddedCodeRangeBuildId[];
+  static const char kV8CodeRangeBuildId[];
+
  private:
   const v8::UnwindState unwind_state_;
   base::flat_set<const base::ModuleCache::Module*> v8_modules_;

chrome/renderer/v8_unwinder_unittest.cc

+// Copyright 2019 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 "chrome/renderer/v8_unwinder.h"
+
+#include <algorithm>
+#include <memory>
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/location.h"
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/profiler/stack_sampling_profiler_test_util.h"
+#include "base/stl_util.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/test/bind_test_util.h"
+#include "base/test/scoped_task_environment.h"
+#include "build/build_config.h"
+#include "gin/public/isolate_holder.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "v8/include/v8.h"
+
+namespace {
+
+v8::Local<v8::String> ToV8String(const char* str) {
+  return v8::String::NewFromUtf8(v8::Isolate::GetCurrent(), str,
+                                 v8::NewStringType::kNormal)
+      .ToLocalChecked();
+}
+
+v8::Local<v8::Object> CreatePointerHolder(const void* ptr) {
+  v8::Local<v8::ObjectTemplate> object_template =
+      v8::ObjectTemplate::New(v8::Isolate::GetCurrent());
+  object_template->SetInternalFieldCount(1);
+  v8::Local<v8::Object> holder =
+      object_template
+          ->NewInstance(v8::Isolate::GetCurrent()->GetCurrentContext())
+          .ToLocalChecked();
+  holder->SetAlignedPointerInInternalField(0, const_cast<void*>(ptr));
+  return holder;
+}
+
+template <typename T>
+T* GetPointerFromHolder(v8::Local<v8::Object> holder) {
+  return reinterpret_cast<T*>(holder->GetAlignedPointerFromInternalField(0));
+}
+
+// Sets up the environment necessary to execute V8 code.
+class ScopedV8Environment {
+ public:
+  ScopedV8Environment()
+      : isolate_holder_(scoped_task_environment_.GetMainThreadTaskRunner(),
+                        gin::IsolateHolder::IsolateType::kBlinkMainThread) {
+    isolate()->Enter();
+    v8::HandleScope handle_scope(isolate());
+    context_.Reset(isolate(), v8::Context::New(isolate()));
+    v8::Local<v8::Context>::New(isolate(), context_)->Enter();
+  }
+
+  ~ScopedV8Environment() {
+    {
+      v8::HandleScope handle_scope(isolate());
+      v8::Local<v8::Context>::New(isolate(), context_)->Exit();
+      context_.Reset();
+    }
+    isolate()->Exit();
+  }
+
+  v8::Isolate* isolate() { return isolate_holder_.isolate(); }
+
+ private:
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+  gin::IsolateHolder isolate_holder_;
+  v8::Persistent<v8::Context> context_;
+};
+
+// C++ function to be invoked from V8 which calls back into the provided closure
+// pointer (passed via a holder object) to wait for a stack sample to be taken.
+void WaitForSampleNative(const v8::FunctionCallbackInfo<v8::Value>& info) {
+  const base::RepeatingClosure* wait_for_sample =
+      GetPointerFromHolder<const base::RepeatingClosure>(
+          info[0].As<v8::Object>());
+  if (wait_for_sample)
+    wait_for_sample->Run();
+}
+
+// Causes a stack sample to be taken after setting up a call stack from C++ to
+// JavaScript and back into C++.
+base::FunctionAddressRange CallThroughV8(
+    const base::RepeatingCallback<void(const v8::UnwindState&)>&
+        report_unwind_state,
+    const base::RepeatingClosure& wait_for_sample) {
+  const void* start_program_counter = base::GetProgramCounter();
+
+  if (wait_for_sample) {
+    // Set up V8 runtime environment.
+    // Allows use of natives (functions starting with '%') within JavaScript
+    // code, which allows us to control compilation of the JavaScript function
+    // we define.
+    // TODO(wittman): The flag should be set only for the duration of this test
+    // but the V8 API currently doesn't support this. http://crbug.com/v8/9210
+    // covers adding the necessary functionality to V8.
+    v8::V8::SetFlagsFromString("--allow-natives-syntax");
+    ScopedV8Environment v8_environment;
+    v8::Isolate* isolate = v8_environment.isolate();
+    report_unwind_state.Run(isolate->GetUnwindState());
+    v8::HandleScope handle_scope(isolate);
+    v8::Local<v8::Context> context = isolate->GetCurrentContext();
+
+    // Define a V8 function WaitForSampleNative() backed by the C++ function
+    // WaitForSampleNative().
+    v8::Local<v8::FunctionTemplate> js_wait_for_sample_native_template =
+        v8::FunctionTemplate::New(isolate, WaitForSampleNative);
+    v8::Local<v8::Function> js_wait_for_sample_native =
+        js_wait_for_sample_native_template->GetFunction(context)
+            .ToLocalChecked();
+    js_wait_for_sample_native->SetName(ToV8String("WaitForSampleNative"));
+    context->Global()
+        ->Set(context, ToV8String("WaitForSampleNative"),
+              js_wait_for_sample_native)
+        .FromJust();
+
+    // Run a script to create the V8 function waitForSample() that invokes
+    // WaitForSampleNative(), and a function that ensures that waitForSample()
+    // gets compiled. waitForSample() just passes the holder object for the
+    // pointer to the wait_for_sample Closure back into the C++ code. We ensure
+    // that the function is compiled to test walking through both builtin and
+    // runtime-generated code.
+    const char kWaitForSampleJs[] = R"(
+        function waitForSample(closure_pointer_holder) {
+          if (closure_pointer_holder)
+            WaitForSampleNative(closure_pointer_holder);
+        }
+
+        // Set up the function to be compiled rather than interpreted.
+        function compileWaitForSample(closure_pointer_holder) {
+          %PrepareFunctionForOptimization(waitForSample);
+          waitForSample(closure_pointer_holder);
+          waitForSample(closure_pointer_holder);
+          %OptimizeFunctionOnNextCall(waitForSample);
+        }
+        )";
+    v8::Local<v8::Script> script =
+        v8::Script::Compile(context, ToV8String(kWaitForSampleJs))
+            .ToLocalChecked();
+    script->Run(context).ToLocalChecked();
+
+    // Run compileWaitForSample(), using a null closure pointer to avoid
+    // actually waiting.
+    v8::Local<v8::Function> js_compile_wait_for_sample =
+        v8::Local<v8::Function>::Cast(
+            context->Global()
+                ->Get(context, ToV8String("compileWaitForSample"))
+                .ToLocalChecked());
+    v8::Local<v8::Value> argv[] = {CreatePointerHolder(nullptr)};
+    js_compile_wait_for_sample
+        ->Call(context, v8::Undefined(isolate), base::size(argv), argv)
+        .ToLocalChecked();
+
+    // Run waitForSample() with the real closure pointer.
+    argv[0] = CreatePointerHolder(&wait_for_sample);
+    v8::Local<v8::Function> js_wait_for_sample = v8::Local<v8::Function>::Cast(
+        context->Global()
+            ->Get(context, ToV8String("waitForSample"))
+            .ToLocalChecked());
+    js_wait_for_sample
+        ->Call(context, v8::Undefined(isolate), base::size(argv), argv)
+        .ToLocalChecked();
+  }
+
+  // Volatile to prevent a tail call to GetProgramCounter().
+  const void* volatile end_program_counter = base::GetProgramCounter();
+  return {start_program_counter, end_program_counter};
+}
+
+}  // namespace
+
+// Checks that unwinding from C++ through JavaScript and back into C++ succeeds.
+// NB: unwinding is only supported for 64 bit Windows and OS X.
+#if (defined(OS_WIN) && defined(ARCH_CPU_64_BITS)) || \
+    (defined(OS_MACOSX) && !defined(OS_IOS))
+#define MAYBE_UnwindThroughV8Frames UnwindThroughV8Frames
+#else
+#define MAYBE_UnwindThroughV8Frames DISABLED_UnwindThroughV8Frames
+#endif
+TEST(V8UnwinderTest, MAYBE_UnwindThroughV8Frames) {
+  v8::UnwindState unwind_state;
+  base::WaitableEvent unwind_state_available;
+
+  const auto set_unwind_state = [&](const v8::UnwindState& state) {
+    unwind_state = state;
+    unwind_state_available.Signal();
+  };
+
+  const auto create_v8_unwinder = [&]() -> std::unique_ptr<base::Unwinder> {
+    unwind_state_available.Wait();
+    return std::make_unique<V8Unwinder>(unwind_state);
+  };
+
+  base::UnwindScenario scenario(base::BindRepeating(
+      &CallThroughV8, base::BindLambdaForTesting(set_unwind_state)));
+  base::ModuleCache module_cache;
+
+  std::vector<base::Frame> sample = SampleScenario(
+      &scenario, &module_cache, base::BindLambdaForTesting(create_v8_unwinder));
+
+  // The stack should contain a full unwind.
+  ExpectStackContains(sample, {scenario.GetWaitForSampleAddressRange(),
+                               scenario.GetSetupFunctionAddressRange(),
+                               scenario.GetOuterFunctionAddressRange()});
+
+  // The stack should contain a frame from a JavaScript module.
+  auto loc =
+      std::find_if(sample.begin(), sample.end(), [&](const base::Frame& frame) {
+        return frame.module &&
+               (frame.module->GetId() ==
+                    V8Unwinder::kV8EmbeddedCodeRangeBuildId ||
+                frame.module->GetId() == V8Unwinder::kV8CodeRangeBuildId);
+      });
+  EXPECT_NE(sample.end(), loc);
+}

chrome/test/BUILD.gn

@@ -3039,6 +3039,7 @@ test("unit_tests") {
     "../renderer/page_load_metrics/page_timing_metrics_sender_unittest.cc",
     "../renderer/plugins/plugin_uma_unittest.cc",
     "../renderer/prerender/prerender_dispatcher_unittest.cc",
+    "../renderer/v8_unwinder_unittest.cc",
     "../test/base/chrome_render_view_test.cc",
     "../test/base/chrome_render_view_test.h",
     "../test/base/menu_model_test.cc",
@@ -3137,6 +3138,7 @@ test("unit_tests") {
   deps = [
     ":test_support",
     ":test_support_unit",
+    "//base:base_stack_sampling_profiler_test_util",
     "//base/test:test_support",
     "//chrome:browser_dependencies",
     "//chrome:child_dependencies",