Revert "[Clank SSM]: Implement NativeUnwinderAndroid."

This reverts commit 38e3a62a.

Reason for revert: Official build failure crbug/1071307

Original change's description:
> [Clank SSM]: Implement NativeUnwinderAndroid.
> 
> This CL implements NativeUnwinderAndroid & tests for android
> unwinding support. It also enables StackSamplingProfilerTest
> on Android.
> 
> A new target source_set is added, native_unwinder_android
> that contains NativeUnwinderAndroid.
> StackSamplingProfilerTest depends on it for android.
> 
> Bug: 989102
> Change-Id: Ie38fd99ca5fb053e1881d0977924b70a6fbc1e9b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2055743
> Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Reviewed-by: Mike Wittman <wittman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#759283}

TBR=gab@chromium.org,thakis@chromium.org,wittman@chromium.org,etiennep@chromium.org

Change-Id: If46d65db088434e84e0fce10897669cd4a3739cc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 989102
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2152205Reviewed-by: Ben Mason <benmason@chromium.org>
Commit-Queue: Ben Mason <benmason@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759526}

base/BUILD.gn

@@ -1213,6 +1213,8 @@ jumbo_component("base") {
       "message_loop/message_pump_android.h",
       "os_compat_android.cc",
       "os_compat_android.h",
+      "profiler/native_unwinder_android.cc",
+      "profiler/native_unwinder_android.h",
       "profiler/stack_sampler_android.cc",
       "threading/platform_thread_android.cc",
       "trace_event/cpufreq_monitor_android.cc",
@@ -2383,11 +2385,9 @@ if (is_win) {
   }
 }
 
-if (is_win || is_mac || is_android) {
-  if (current_cpu == "x64" || (current_cpu == "arm64" && is_win) ||
-      is_android) {
-    # Must be a loadable module so that it can be loaded/unloaded at runtime
-    # during testing.
+if (is_win || is_mac) {
+  if (current_cpu == "x64" || (current_cpu == "arm64" && is_win)) {
+    # Must be a shared library so that it can be unloaded during testing.
     loadable_module("base_profiler_test_support_library") {
       testonly = true
       sources = [ "profiler/test_support_library.cc" ]
@@ -2395,22 +2395,6 @@ if (is_win || is_mac || is_android) {
   }
 }
 
-if (is_android) {
-  source_set("native_unwinder_android") {
-    sources = [
-      "profiler/native_unwinder_android.cc",
-      "profiler/native_unwinder_android.h",
-      "profiler/unwindstack_internal_android.cc",
-      "profiler/unwindstack_internal_android.h",
-    ]
-
-    include_dirs = [ "//third_party/libunwindstack/src/libunwindstack/include" ]
-
-    public_deps = [ ":base" ]
-    deps = [ "//third_party/libunwindstack" ]
-  }
-}
-
 source_set("base_stack_sampling_profiler_test_util") {
   testonly = true
   sources = [
@@ -2966,7 +2950,6 @@ test("base_unittests") {
       "android/sys_utils_unittest.cc",
       "android/unguessable_token_android_unittest.cc",
       "os_compat_android_unittest.cc",
-      "profiler/native_unwinder_android_unittest.cc",
       "trace_event/cpufreq_monitor_android_unittest.cc",
       "trace_event/java_heap_dump_provider_android_unittest.cc",
     ]
@@ -2980,13 +2963,8 @@ test("base_unittests") {
     deps += [
       ":base_java",
       ":base_java_unittest_support",
-      ":base_profiler_test_support_java",
-      ":base_profiler_test_support_jni_headers",
-      ":base_profiler_test_support_library",
-      ":native_unwinder_android",
       "//base/test:test_support_java",
     ]
-    include_dirs = [ "//third_party/libunwindstack/src/libunwindstack/include" ]
   }
 
   if (icu_use_data_file) {
@@ -3678,24 +3656,6 @@ if (is_android) {
     ]
   }
 
-  generate_jni("base_profiler_test_support_jni_headers") {
-    testonly = true
-    sources =
-        [ "android/javatests/src/org/chromium/base/profiler/TestSupport.java" ]
-  }
-
-  android_library("base_profiler_test_support_java") {
-    testonly = true
-    sources =
-        [ "android/javatests/src/org/chromium/base/profiler/TestSupport.java" ]
-
-    annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]
-    deps = [
-      "//base:base_java",
-      "//base:jni_java",
-    ]
-  }
-
   generate_build_config_srcjar("base_build_config_gen") {
     use_final_fields = false
   }

base/DEPS

@@ -6,7 +6,6 @@ include_rules = [
   "+third_party/lss",
   "+third_party/modp_b64",
   "+third_party/tcmalloc",
-  "+third_party/libunwindstack/src/libunwindstack/include",
 
   # These are implicitly brought in from the root, and we don't want them.
   "-ipc",

base/android/javatests/src/org/chromium/base/profiler/TestSupport.java

-// Copyright 2020 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.
-
-package org.chromium.base.profiler;
-
-import org.chromium.base.annotations.CalledByNative;
-import org.chromium.base.annotations.JNINamespace;
-import org.chromium.base.annotations.NativeMethods;
-
-/**
- * Helper to run code through JNI layer to test JNI unwinding.
- */
-@JNINamespace("base")
-public final class TestSupport {
-    @CalledByNative
-    public static void callWithJavaFunction(long context) {
-        TestSupportJni.get().invokeCallbackFunction(context);
-    }
-
-    @NativeMethods
-    interface Natives {
-        void invokeCallbackFunction(long context);
-    }
-}

base/profiler/native_unwinder_android.cc

@@ -4,225 +4,25 @@
 
 #include "base/profiler/native_unwinder_android.h"
 
-#include <string>
-#include <vector>
-
-#include <sys/mman.h>
-
-#include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/Elf.h"
-#include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/Maps.h"
-#include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/Memory.h"
-
-#include "base/memory/ptr_util.h"
 #include "base/profiler/module_cache.h"
 #include "base/profiler/native_unwinder.h"
 #include "base/profiler/profile_builder.h"
-#include "base/profiler/unwindstack_internal_android.h"
-#include "build/build_config.h"
-
-#if defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_32_BITS)
-#include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/MachineArm.h"
-#include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/RegsArm.h"
-#elif defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_64_BITS)
-#include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/MachineArm64.h"
-#include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/RegsArm64.h"
-#endif  // #if defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_32_BITS)
 
 namespace base {
-namespace {
-
-class AndroidModule : public ModuleCache::Module {
- public:
-  AndroidModule(unwindstack::MapInfo* map_info)
-      : start_(map_info->start),
-        size_(map_info->end - map_info->start),
-        build_id_(map_info->GetBuildID()),
-        name_(map_info->name) {}
-  ~AndroidModule() override = default;
-
-  uintptr_t GetBaseAddress() const override { return start_; }
-
-  std::string GetId() const override { return build_id_; }
-
-  FilePath GetDebugBasename() const override { return FilePath(name_); }
-
-  // Gets the size of the module.
-  size_t GetSize() const override { return size_; }
-
-  // True if this is a native module.
-  bool IsNative() const override { return true; }
-
-  const uintptr_t start_;
-  const size_t size_;
-  const std::string build_id_;
-  const std::string name_;
-};
-
-std::unique_ptr<unwindstack::Regs> CreateFromRegisterContext(
-    RegisterContext* thread_context) {
-#if defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_32_BITS)
-  return WrapUnique<unwindstack::Regs>(unwindstack::RegsArm::Read(
-      reinterpret_cast<void*>(&thread_context->arm_r0)));
-#elif defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_64_BITS)
-  return WrapUnique<unwindstack::Regs>(unwindstack::RegsArm64::Read(
-      reinterpret_cast<void*>(&thread_context->regs[0])));
-#else   // #if defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_32_BITS)
-  NOTREACHED();
-  return nullptr;
-#endif  // #if defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_32_BITS)
-}
-
-void CopyToRegisterContext(unwindstack::Regs* regs,
-                           RegisterContext* thread_context) {
-#if defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_32_BITS)
-  memcpy(reinterpret_cast<void*>(&thread_context->arm_r0), regs->RawData(),
-         unwindstack::ARM_REG_LAST * sizeof(uint32_t));
-#elif defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_64_BITS)
-  memcpy(reinterpret_cast<void*>(&thread_context->regs[0]), regs->RawData(),
-         unwindstack::ARM64_REG_LAST * sizeof(uint32_t));
-#else   // #if defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_32_BITS)
-  NOTREACHED();
-#endif  // #if defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_32_BITS)
-}
-
-}  // namespace
-
-// static
-std::unique_ptr<unwindstack::Maps> NativeUnwinderAndroid::CreateMaps() {
-  auto maps = std::make_unique<unwindstack::LocalMaps>();
-  if (maps->Parse())
-    return maps;
-  return nullptr;
-}
-
-// static
-std::unique_ptr<unwindstack::Memory>
-NativeUnwinderAndroid::CreateProcessMemory() {
-  return std::make_unique<unwindstack::MemoryLocal>();
-}
-
-void NativeUnwinderAndroid::AddInitialModulesFromMaps(
-    const unwindstack::Maps& memory_regions_map,
-    ModuleCache* module_cache) {
-  for (const auto& region : memory_regions_map) {
-    // Only add executable regions.
-    if (!(region->flags & PROT_EXEC))
-      continue;
-    module_cache->AddCustomNativeModule(
-        std::make_unique<AndroidModule>(region.get()));
-  }
-}
-
-NativeUnwinderAndroid::NativeUnwinderAndroid(
-    unwindstack::Maps* memory_regions_map,
-    unwindstack::Memory* process_memory,
-    uintptr_t exclude_module_with_base_address)
-    : memory_regions_map_(memory_regions_map),
-      process_memory_(process_memory),
-      exclude_module_with_base_address_(exclude_module_with_base_address) {}
-
-NativeUnwinderAndroid::~NativeUnwinderAndroid() = default;
-
-void NativeUnwinderAndroid::AddInitialModules(ModuleCache* module_cache) {
-  AddInitialModulesFromMaps(*memory_regions_map_, module_cache);
-}
 
 bool NativeUnwinderAndroid::CanUnwindFrom(const Frame& current_frame) const {
-  return current_frame.module && current_frame.module->IsNative() &&
-         current_frame.module->GetBaseAddress() !=
-             exclude_module_with_base_address_;
+  return false;
 }
 
 UnwindResult NativeUnwinderAndroid::TryUnwind(RegisterContext* thread_context,
                                               uintptr_t stack_top,
                                               ModuleCache* module_cache,
                                               std::vector<Frame>* stack) const {
-  auto regs = CreateFromRegisterContext(thread_context);
-  DCHECK(regs);
-  unwindstack::ArchEnum arch = regs->Arch();
-
-  do {
-    uint64_t cur_pc = regs->pc();
-    uint64_t cur_sp = regs->sp();
-    unwindstack::MapInfo* map_info = memory_regions_map_->Find(cur_pc);
-    if (map_info == nullptr ||
-        map_info->flags & unwindstack::MAPS_FLAGS_DEVICE_MAP) {
-      break;
-    }
-
-    unwindstack::Elf* elf =
-        map_info->GetElf({process_memory_, [](unwindstack::Memory*) {}}, arch);
-    if (!elf->valid())
-      break;
-
-    UnwindStackMemoryAndroid stack_memory(cur_sp, stack_top);
-    uintptr_t rel_pc = elf->GetRelPc(cur_pc, map_info);
-    bool finished = false;
-    bool stepped =
-        elf->Step(rel_pc, rel_pc, regs.get(), &stack_memory, &finished);
-    if (stepped && finished)
-      return UnwindResult::COMPLETED;
-
-    if (!stepped) {
-      // Stepping failed. Try unwinding using return address.
-      if (stack->size() == 1) {
-        if (!regs->SetPcFromReturnAddress(&stack_memory))
-          return UnwindResult::ABORTED;
-      } else {
-        break;
-      }
-    }
-
-    // If the pc and sp didn't change, then consider everything stopped.
-    if (cur_pc == regs->pc() && cur_sp == regs->sp())
-      return UnwindResult::ABORTED;
-
-    // Exclusive range of expected stack pointer values after the unwind.
-    struct {
-      uintptr_t start;
-      uintptr_t end;
-    } expected_stack_pointer_range = {cur_sp, stack_top};
-    if (regs->sp() < expected_stack_pointer_range.start ||
-        regs->sp() >= expected_stack_pointer_range.end) {
-      return UnwindResult::ABORTED;
-    }
-
-    if (regs->dex_pc() != 0) {
-      // Add a frame to represent the dex file.
-      EmitDexFrame(regs->dex_pc(), module_cache, stack);
-
-      // Clear the dex pc so that we don't repeat this frame later.
-      regs->set_dex_pc(0);
-    }
-
-    // Add the frame to |stack|.
-    const ModuleCache::Module* module =
-        module_cache->GetModuleForAddress(regs->pc());
-    stack->emplace_back(regs->pc(), module);
-  } while (CanUnwindFrom(stack->back()));
-
-  // Restore registers necessary for further unwinding in |thread_context|.
-  CopyToRegisterContext(regs.get(), thread_context);
-  return UnwindResult::UNRECOGNIZED_FRAME;
+  return UnwindResult::ABORTED;
 }
 
-void NativeUnwinderAndroid::EmitDexFrame(uintptr_t dex_pc,
-                                         ModuleCache* module_cache,
-                                         std::vector<Frame>* stack) const {
-  const ModuleCache::Module* module = module_cache->GetModuleForAddress(dex_pc);
-  if (!module) {
-    // The region containing |dex_pc| may not be in |module_cache| since it's
-    // usually not executable (.dex file). Since non-executable regions
-    // are used much less commonly, it's lazily added here instead of from
-    // AddInitialModules().
-    unwindstack::MapInfo* map_info = memory_regions_map_->Find(dex_pc);
-    if (map_info) {
-      auto new_module = std::make_unique<AndroidModule>(map_info);
-      module = new_module.get();
-      module_cache->AddCustomNativeModule(std::move(new_module));
-    }
-  }
-  stack->emplace_back(dex_pc, module);
+std::unique_ptr<Unwinder> CreateNativeUnwinder(ModuleCache* module_cache) {
+  return std::make_unique<NativeUnwinderAndroid>();
 }
 
 }  // namespace base

base/profiler/native_unwinder_android.h

@@ -7,54 +7,25 @@
 
 #include "base/profiler/unwinder.h"
 
-namespace unwindstack {
-class Maps;
-class Memory;
-}  // namespace unwindstack
-
 namespace base {
 
 // Native unwinder implementation for Android, using libunwindstack.
+//
+// TODO(charliea): Implement this class.
+// See: https://crbug.com/989102
 class NativeUnwinderAndroid : public Unwinder {
  public:
-  // Creates maps object from /proc/self/maps for use by NativeUnwinderAndroid.
-  // Since this is an expensive call, the maps object should be re-used across
-  // all profiles in a process.
-  static std::unique_ptr<unwindstack::Maps> CreateMaps();
-  static std::unique_ptr<unwindstack::Memory> CreateProcessMemory();
-  // Adds modules found from executable loaded memory regions to |module_cache|.
-  static void AddInitialModulesFromMaps(
-      const unwindstack::Maps& memory_regions_map,
-      ModuleCache* module_cache);
-
-  // |exclude_module_with_base_address| is used to exclude a specific module
-  // and let another unwinder take control. TryUnwind() will exit with
-  // UNRECOGNIZED_FRAME and CanUnwindFrom() will return false when a frame is
-  // encountered in that module.
-  NativeUnwinderAndroid(unwindstack::Maps* memory_regions_map,
-                        unwindstack::Memory* process_memory,
-                        uintptr_t exclude_module_with_base_address = 0);
-  ~NativeUnwinderAndroid() override;
+  NativeUnwinderAndroid() = default;
 
   NativeUnwinderAndroid(const NativeUnwinderAndroid&) = delete;
   NativeUnwinderAndroid& operator=(const NativeUnwinderAndroid&) = delete;
 
   // Unwinder
-  void AddInitialModules(ModuleCache* module_cache) override;
   bool CanUnwindFrom(const Frame& current_frame) const override;
   UnwindResult TryUnwind(RegisterContext* thread_context,
                          uintptr_t stack_top,
                          ModuleCache* module_cache,
                          std::vector<Frame>* stack) const override;
-
- private:
-  void EmitDexFrame(uintptr_t dex_pc,
-                    ModuleCache* module_cache,
-                    std::vector<Frame>* stack) const;
-
-  unwindstack::Maps* const memory_regions_map_;
-  unwindstack::Memory* const process_memory_;
-  const uintptr_t exclude_module_with_base_address_;
 };
 
 }  // namespace base

base/profiler/register_context.h

@@ -101,7 +101,7 @@ inline uintptr_t& RegisterContextFramePointer(mcontext_t* context) {
 }
 
 inline uintptr_t& RegisterContextInstructionPointer(mcontext_t* context) {
-  return AsUintPtr(&context->arm_pc);
+  return AsUintPtr(&context->arm_ip);
 }
 
 #elif defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_64_BITS)

base/profiler/stack_sampling_profiler_test_util.cc

@@ -9,13 +9,10 @@
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/location.h"
-#include "base/path_service.h"
-#include "base/profiler/stack_buffer.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 "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
@@ -58,17 +55,6 @@ class TestProfileBuilder : public ProfileBuilder {
   std::vector<Frame> sample_;
 };
 
-// The function to be executed by the code in the other library.
-void OtherLibraryCallback(void* arg) {
-  OnceClosure* wait_for_sample = static_cast<OnceClosure*>(arg);
-
-  std::move(*wait_for_sample).Run();
-
-  // Prevent tail call.
-  volatile int i = 0;
-  ALLOW_UNUSED_LOCAL(i);
-}
-
 }  // namespace
 
 TargetThread::TargetThread(OnceClosure to_run) : to_run_(std::move(to_run)) {}
@@ -148,47 +134,6 @@ CallWithPlainFunction(OnceClosure wait_for_sample) {
   return {start_program_counter, end_program_counter};
 }
 
-// Disable inlining for this function so that it gets its own stack frame.
-NOINLINE FunctionAddressRange CallWithAlloca(OnceClosure wait_for_sample) {
-  const void* start_program_counter = GetProgramCounter();
-
-  // Volatile to force a dynamic stack allocation.
-  const volatile size_t alloca_size = 100;
-  // Use the memory via volatile writes to prevent the allocation from being
-  // optimized out.
-  volatile char* const allocation =
-      const_cast<volatile char*>(static_cast<char*>(alloca(alloca_size)));
-  for (volatile char* p = allocation; p < allocation + alloca_size; ++p)
-    *p = '\0';
-
-  if (!wait_for_sample.is_null())
-    std::move(wait_for_sample).Run();
-
-  // Volatile to prevent a tail call to GetProgramCounter().
-  const void* volatile end_program_counter = GetProgramCounter();
-  return {start_program_counter, end_program_counter};
-}
-
-// Disable inlining for this function so that it gets its own stack frame.
-NOINLINE FunctionAddressRange
-CallThroughOtherLibrary(NativeLibrary library, OnceClosure wait_for_sample) {
-  const void* start_program_counter = GetProgramCounter();
-
-  if (!wait_for_sample.is_null()) {
-    // A function whose arguments are a function accepting void*, and a void*.
-    using InvokeCallbackFunction = void (*)(void (*)(void*), void*);
-    EXPECT_TRUE(library);
-    InvokeCallbackFunction function = reinterpret_cast<InvokeCallbackFunction>(
-        GetFunctionPointerFromNativeLibrary(library, "InvokeCallbackFunction"));
-    EXPECT_TRUE(function);
-    (*function)(&OtherLibraryCallback, &wait_for_sample);
-  }
-
-  // Volatile to prevent a tail call to GetProgramCounter().
-  const void* volatile end_program_counter = GetProgramCounter();
-  return {start_program_counter, end_program_counter};
-}
-
 void WithTargetThread(UnwindScenario* scenario,
                       ProfileCallback profile_callback) {
   UnwindScenario::SampleEvents events;
@@ -301,31 +246,4 @@ void ExpectStackDoesNotContain(
   }
 }
 
-NativeLibrary LoadOtherLibrary() {
-  // The lambda gymnastics works around the fact that we can't use ASSERT_*
-  // macros in a function returning non-null.
-  const auto load = [](NativeLibrary* library) {
-    FilePath other_library_path;
-    ASSERT_TRUE(PathService::Get(DIR_MODULE, &other_library_path));
-    other_library_path = other_library_path.AppendASCII(
-        GetLoadableModuleName("base_profiler_test_support_library"));
-    NativeLibraryLoadError load_error;
-    *library = LoadNativeLibrary(other_library_path, &load_error);
-    ASSERT_TRUE(*library) << "error loading " << other_library_path.value()
-                          << ": " << load_error.ToString();
-  };
-
-  NativeLibrary library = nullptr;
-  load(&library);
-  return library;
-}
-
-uintptr_t GetAddressInOtherLibrary(NativeLibrary library) {
-  EXPECT_TRUE(library);
-  uintptr_t address = reinterpret_cast<uintptr_t>(
-      GetFunctionPointerFromNativeLibrary(library, "InvokeCallbackFunction"));
-  EXPECT_NE(address, 0u);
-  return address;
-}
-
 }  // namespace base

base/profiler/stack_sampling_profiler_test_util.h

@@ -9,10 +9,8 @@
 #include <vector>
 
 #include "base/callback.h"
-#include "base/native_library.h"
 #include "base/profiler/frame.h"
 #include "base/profiler/sampling_profiler_thread_token.h"
-#include "base/profiler/stack_sampler.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/platform_thread.h"
 
@@ -93,16 +91,6 @@ class UnwindScenario {
 // any special unwinding setup, to exercise the "normal" unwind scenario.
 FunctionAddressRange CallWithPlainFunction(OnceClosure wait_for_sample);
 
-// Calls into |wait_for_sample| after using alloca(), to test unwinding with a
-// frame pointer.
-FunctionAddressRange CallWithAlloca(OnceClosure wait_for_sample);
-
-// Calls into |wait_for_sample| through a function within another library, to
-// test unwinding through multiple modules and scenarios involving unloaded
-// modules.
-FunctionAddressRange CallThroughOtherLibrary(NativeLibrary library,
-                                             OnceClosure wait_for_sample);
-
 // The callback to perform profiling on the provided thread.
 using ProfileCallback = OnceCallback<void(SamplingProfilerThreadToken)>;
 
@@ -134,12 +122,6 @@ void ExpectStackDoesNotContain(
     const std::vector<Frame>& stack,
     const std::vector<FunctionAddressRange>& functions);
 
-// Loads the other library, which defines a function to be called in the
-// WITH_OTHER_LIBRARY configuration.
-NativeLibrary LoadOtherLibrary();
-
-uintptr_t GetAddressInOtherLibrary(NativeLibrary library);
-
 }  // namespace base
 
 #endif  // BASE_PROFILER_STACK_SAMPLING_PROFILER_TEST_UTIL_H_

base/profiler/stack_sampling_profiler_unittest.cc

@@ -20,6 +20,8 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/metrics_hashes.h"
+#include "base/native_library.h"
+#include "base/path_service.h"
 #include "base/profiler/sample_metadata.h"
 #include "base/profiler/stack_sampler.h"
 #include "base/profiler/stack_sampling_profiler.h"
@@ -64,6 +66,64 @@ using SamplingParams = StackSamplingProfiler::SamplingParams;
 
 namespace {
 
+// Calls into |wait_for_sample| after using alloca(), to test unwinding with a
+// frame pointer.
+// Disable inlining for this function so that it gets its own stack frame.
+NOINLINE FunctionAddressRange CallWithAlloca(OnceClosure wait_for_sample) {
+  const void* start_program_counter = GetProgramCounter();
+
+  // Volatile to force a dynamic stack allocation.
+  const volatile size_t alloca_size = 100;
+  // Use the memory via volatile writes to prevent the allocation from being
+  // optimized out.
+  volatile char* const allocation =
+      const_cast<volatile char*>(static_cast<char*>(alloca(alloca_size)));
+  for (volatile char* p = allocation; p < allocation + alloca_size; ++p)
+    *p = '\0';
+
+  if (!wait_for_sample.is_null())
+    std::move(wait_for_sample).Run();
+
+  // Volatile to prevent a tail call to GetProgramCounter().
+  const void* volatile end_program_counter = GetProgramCounter();
+  return {start_program_counter, end_program_counter};
+}
+
+// The function to be executed by the code in the other library.
+void OtherLibraryCallback(void* arg) {
+  OnceClosure* wait_for_sample = static_cast<OnceClosure*>(arg);
+
+  std::move(*wait_for_sample).Run();
+
+  // Prevent tail call.
+  volatile int i = 0;
+  ALLOW_UNUSED_LOCAL(i);
+}
+
+// Calls into |wait_for_sample| through a function within another library, to
+// test unwinding through multiple modules and scenarios involving unloaded
+// modules.
+// Disable inlining for this function so that it gets its own stack frame.
+NOINLINE FunctionAddressRange
+CallThroughOtherLibrary(NativeLibrary library, OnceClosure wait_for_sample) {
+  const void* start_program_counter = GetProgramCounter();
+
+  if (!wait_for_sample.is_null()) {
+    // A function whose arguments are a function accepting void*, and a void*.
+    using InvokeCallbackFunction = void (*)(void (*)(void*), void*);
+    EXPECT_TRUE(library);
+    InvokeCallbackFunction function = reinterpret_cast<InvokeCallbackFunction>(
+        GetFunctionPointerFromNativeLibrary(library, "InvokeCallbackFunction"));
+    EXPECT_TRUE(function);
+
+    (*function)(&OtherLibraryCallback, &wait_for_sample);
+  }
+
+  // Volatile to prevent a tail call to GetProgramCounter().
+  const void* volatile end_program_counter = GetProgramCounter();
+  return {start_program_counter, end_program_counter};
+}
+
 // State provided to the ProfileBuilder's ApplyMetadataRetrospectively function.
 struct RetrospectiveMetadata {
   TimeTicks period_start;
@@ -167,6 +227,27 @@ void TestProfileBuilder::OnProfileCompleted(TimeDelta profile_duration,
                                    sampling_period});
 }
 
+// Loads the other library, which defines a function to be called in the
+// WITH_OTHER_LIBRARY configuration.
+NativeLibrary LoadOtherLibrary() {
+  // The lambda gymnastics works around the fact that we can't use ASSERT_*
+  // macros in a function returning non-null.
+  const auto load = [](NativeLibrary* library) {
+    FilePath other_library_path;
+    ASSERT_TRUE(PathService::Get(DIR_MODULE, &other_library_path));
+    other_library_path = other_library_path.AppendASCII(
+        GetLoadableModuleName("base_profiler_test_support_library"));
+    NativeLibraryLoadError load_error;
+    *library = LoadNativeLibrary(other_library_path, &load_error);
+    ASSERT_TRUE(*library) << "error loading " << other_library_path.value()
+                          << ": " << load_error.ToString();
+  };
+
+  NativeLibrary library = nullptr;
+  load(&library);
+  return library;
+}
+
 // Unloads |library| and returns when it has completed unloading. Unloading a
 // library is asynchronous on Windows, so simply calling UnloadNativeLibrary()
 // is insufficient to ensure it's been unloaded.

base/profiler/unwindstack_internal_android.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 "base/profiler/unwindstack_internal_android.h"
-
-#include <string.h>
-
-#include "base/logging.h"
-
-namespace base {
-
-UnwindStackMemoryAndroid::UnwindStackMemoryAndroid(uintptr_t stack_ptr,
-                                                   uintptr_t stack_top)
-    : stack_ptr_(stack_ptr), stack_top_(stack_top) {
-  DCHECK_LE(stack_ptr_, stack_top_);
-}
-
-UnwindStackMemoryAndroid::~UnwindStackMemoryAndroid() = default;
-
-size_t UnwindStackMemoryAndroid::Read(uint64_t addr, void* dst, size_t size) {
-  if (addr < stack_ptr_)
-    return 0;
-  if (size >= stack_top_ || addr > stack_top_ - size)
-    return 0;
-  memcpy(dst, reinterpret_cast<void*>(addr), size);
-  return size;
-}
-
-}  // namespace base
\ No newline at end of file

base/profiler/unwindstack_internal_android.h

-// Copyright 2020 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.
-
-#ifndef BASE_PROFILER_UNWINDSTACK_INTERNAL_ANDROID_H_
-#define BASE_PROFILER_UNWINDSTACK_INTERNAL_ANDROID_H_
-
-#include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/Maps.h"
-#include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/Memory.h"
-
-// Avoid including this file directly in a header as it leaks headers from
-// libunwindstack. In particular, it's not to be included directly or
-// transitively from native_unwinder_android.h
-
-namespace base {
-
-// Implementation of unwindstack::Memory that restricts memory access to a stack
-// buffer, used by NativeUnwinderAndroid. While unwinding, only memory accesses
-// within the stack should be performed to restore registers.
-class UnwindStackMemoryAndroid : public unwindstack::Memory {
- public:
-  UnwindStackMemoryAndroid(uintptr_t stack_ptr, uintptr_t stack_top);
-  ~UnwindStackMemoryAndroid() override;
-
-  size_t Read(uint64_t addr, void* dst, size_t size) override;
-
- private:
-  const uintptr_t stack_ptr_;
-  const uintptr_t stack_top_;
-};
-
-}  // namespace base
-
-#endif  // BASE_PROFILER_UNWINDSTACK_INTERNAL_ANDROID_H_
\ No newline at end of file