Revert "Reland "[SSM] Release memory used by libunwindstack when no p…

…rofiler is active""

This reverts commit fa3c250.

Reason for revert: tests are still failing
(https://crbug.com/1418366)

Original change's description:
> Reland "[SSM] Release memory used by libunwindstack when no profiler is active"
>
> This is a reland of commit 388c13f
>
> The root cause of failing build is that `NativeUnwinderAndroid` holds
> an invalid reference to map delegate as the map delegate ownership
> has been transferred after the initial reference.
>
> This reland fixes this issue by letting the child class
> `NativeUnwinderAndroidForTesting` refresh the reference of map
> delegate in the parent class `NativeUnwinderAndroid` after the
> ownership transfer.
>
> Original change's description:
> > [SSM] Release memory used by libunwindstack when no profiler is active
> >
> > The previous investigation reveals that the libunwindstack info map
> > structures holds in `NativeUnwinderCreator` are holding significant
> > amount of memory.
> >
> > This CL lets `SamplingThread` clear the memory cache used in
> > libunwindstack when no profiler is active.
> >
> > Details on where to add this clear cache action is discussed
> > in the following doc:
> > https://docs.google.com/document/d/1Eell0hfe_U5h8ROf-GYSVfQE2j1_Pe1dbcO0054alZ4/edit?usp=sharing
> >
> > Bug: 1375938
> > Change-Id: Icb230bec2bab85cb0ded678fe12befa563b79d45
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4159607
> > Reviewed-by: Mike Wittman <wittman@chromium.org>
> > Commit-Queue: Charlie Hu <chenleihu@google.com>
> > Cr-Commit-Position: refs/heads/main@{#1108178}
>
> Bug: 1375938, 1418366
> Change-Id: I0159b3744c395288246498c5cec4ff93849925c1
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4283897
> Commit-Queue: Charlie Hu <chenleihu@google.com>
> Reviewed-by: Andrew Grieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1108595}

Bug: 1375938, 1418366
Change-Id: I83bde0de3afa4a02646194f9371257bbaff1a122
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4286558
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Owners-Override: Alex Ilin <alexilin@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1108823}

base/BUILD.gn

@@ -1261,8 +1261,6 @@ component("base") {
       "os_compat_android.cc",
       "os_compat_android.h",
       "process/process_android.cc",
-      "profiler/native_unwinder_android_map_delegate.h",
-      "profiler/native_unwinder_android_memory_regions_map.h",
       "profiler/stack_sampler_android.cc",
       "system/sys_info_android.cc",
       "threading/platform_thread_android.cc",

base/profiler/libunwindstack_unwinder_android.cc

@@ -10,7 +10,6 @@
 #include <vector>
 
 #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 "third_party/libunwindstack/src/libunwindstack/include/unwindstack/Regs.h"
 #include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/Unwinder.h"
@@ -84,9 +83,9 @@ std::unique_ptr<unwindstack::Regs> CreateFromRegisterContext(
 }  // namespace
 
 LibunwindstackUnwinderAndroid::LibunwindstackUnwinderAndroid()
-    : memory_regions_map_(NativeUnwinderAndroid::CreateMemoryRegionsMap()),
+    : memory_regions_map_(NativeUnwinderAndroid::CreateMaps()),
       process_memory_(std::shared_ptr<unwindstack::Memory>(
-          memory_regions_map_->TakeMemory().release())) {
+          NativeUnwinderAndroid::CreateProcessMemory().release())) {
   TRACE_EVENT_INSTANT(
       TRACE_DISABLED_BY_DEFAULT("cpu_profiler"),
       "LibunwindstackUnwinderAndroid::LibunwindstackUnwinderAndroid");
@@ -144,7 +143,7 @@ UnwindResult LibunwindstackUnwinderAndroid::TryUnwind(
     std::unique_ptr<unwindstack::Regs> regs =
         CreateFromRegisterContext(thread_context);
     DCHECK(regs);
-    unwindstack::Unwinder unwinder(kMaxFrames, memory_regions_map_->GetMaps(),
+    unwindstack::Unwinder unwinder(kMaxFrames, memory_regions_map_.get(),
                                    regs.get(), process_memory_);
 
     unwinder.SetJitDebug(GetOrCreateJitDebug(regs->Arch()));
@@ -170,7 +169,7 @@ UnwindResult LibunwindstackUnwinderAndroid::TryUnwind(
     TRACE_EVENT(TRACE_DISABLED_BY_DEFAULT("cpu_profiler.debug"),
                 "TryUnwind Reparsing Maps");
     samples_since_last_maps_parse_ = 0;
-    memory_regions_map_->GetMaps()->Parse();
+    memory_regions_map_->Parse();
     jit_debug_.reset();
     dex_files_.reset();
 

base/profiler/libunwindstack_unwinder_android.h

@@ -8,10 +8,10 @@
 #include <memory>
 #include <vector>
 
-#include "base/profiler/native_unwinder_android_memory_regions_map.h"
 #include "base/profiler/unwinder.h"
 #include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/DexFiles.h"
 #include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/JitDebug.h"
+#include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/Maps.h"
 #include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/Memory.h"
 
 namespace base {
@@ -43,12 +43,11 @@ class LibunwindstackUnwinderAndroid : public Unwinder {
   unwindstack::DexFiles* GetOrCreateDexFiles(unwindstack::ArchEnum arch);
 
   int samples_since_last_maps_parse_ = 0;
-  std::unique_ptr<NativeUnwinderAndroidMemoryRegionsMap> memory_regions_map_;
+  std::unique_ptr<unwindstack::Maps> memory_regions_map_;
   // libunwindstack::Unwinder requires that process_memory be provided as a
   // std::shared_ptr. Since this is a third_party library this exception to
   // normal Chromium conventions of not using shared_ptr has to exist here.
   std::shared_ptr<unwindstack::Memory> process_memory_;
-
   std::unique_ptr<unwindstack::JitDebug> jit_debug_;
   std::unique_ptr<unwindstack::DexFiles> dex_files_;
   // Libraries where to search for dex and jit descriptors.

base/profiler/native_unwinder_android.cc

@@ -10,15 +10,12 @@
 #include <vector>
 
 #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 "third_party/libunwindstack/src/libunwindstack/include/unwindstack/Regs.h"
 
 #include "base/memory/ptr_util.h"
 #include "base/notreached.h"
 #include "base/profiler/module_cache.h"
-#include "base/profiler/native_unwinder_android_map_delegate.h"
-#include "base/profiler/native_unwinder_android_memory_regions_map.h"
 #include "base/profiler/profile_builder.h"
 #include "build/build_config.h"
 
@@ -88,28 +85,6 @@ void CopyToRegisterContext(unwindstack::Regs* regs,
 #endif  // #if defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_32_BITS)
 }
 
-// The wrapper class exists to avoid the reference of concrete libunwindstack
-// types in chrome code. Only code in the stack unwinder DFM has the access to
-// third_party/libunwindstack/src/libunwindstack. Files within the stack
-// unwinder DFM can be found by searching `native_unwinder_android` source set
-// in `base/BUILD.gn`.
-class MemoryRegionsMap : public base::NativeUnwinderAndroidMemoryRegionsMap {
- public:
-  MemoryRegionsMap(std::unique_ptr<unwindstack::Maps> maps,
-                   std::unique_ptr<unwindstack::Memory> memory)
-      : maps_(std::move(maps)), memory_(std::move(memory)) {}
-
-  unwindstack::Maps* GetMaps() override { return maps_.get(); }
-  unwindstack::Memory* GetMemory() override { return memory_.get(); }
-  std::unique_ptr<unwindstack::Memory> TakeMemory() override {
-    return std::move(memory_);
-  }
-
- private:
-  std::unique_ptr<unwindstack::Maps> maps_;
-  std::unique_ptr<unwindstack::Memory> memory_;
-};
-
 }  // namespace
 
 UnwindStackMemoryAndroid::UnwindStackMemoryAndroid(uintptr_t stack_ptr,
@@ -130,42 +105,36 @@ size_t UnwindStackMemoryAndroid::Read(uint64_t addr, void* dst, size_t size) {
 }
 
 // static
-std::unique_ptr<NativeUnwinderAndroidMemoryRegionsMap>
-NativeUnwinderAndroid::CreateMemoryRegionsMap() {
+std::unique_ptr<unwindstack::Maps> NativeUnwinderAndroid::CreateMaps() {
   auto maps = std::make_unique<unwindstack::LocalMaps>();
-  const bool success = maps->Parse();
-  DCHECK(success);
+  if (maps->Parse())
+    return maps;
+  return nullptr;
+}
 
-  return std::make_unique<MemoryRegionsMap>(
-      std::move(maps), unwindstack::Memory::CreateLocalProcessMemory());
+// static
+std::unique_ptr<unwindstack::Memory>
+NativeUnwinderAndroid::CreateProcessMemory() {
+  return unwindstack::Memory::CreateLocalProcessMemory();
 }
 
 NativeUnwinderAndroid::NativeUnwinderAndroid(
-    uintptr_t exclude_module_with_base_address,
-    NativeUnwinderAndroidMapDelegate* map_delegate)
-    : exclude_module_with_base_address_(exclude_module_with_base_address),
-      map_delegate_(map_delegate),
-      memory_regions_map_(map_delegate->GetMapReference()) {
-  DCHECK(map_delegate_);
-  DCHECK(memory_regions_map_);
-}
+    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() {
   if (module_cache())
     module_cache()->UnregisterAuxiliaryModuleProvider(this);
-
-  map_delegate_->ReleaseMapReference();
 }
 
 void NativeUnwinderAndroid::InitializeModules() {
   module_cache()->RegisterAuxiliaryModuleProvider(this);
 }
 
-void NativeUnwinderAndroid::SetMapDelegateForTesting(
-    NativeUnwinderAndroidMapDelegate* map_delegate) {
-  map_delegate_ = map_delegate;
-}
-
 bool NativeUnwinderAndroid::CanUnwindFrom(const Frame& current_frame) const {
   return current_frame.module && current_frame.module->IsNative() &&
          current_frame.module->GetBaseAddress() !=
@@ -182,15 +151,14 @@ UnwindResult NativeUnwinderAndroid::TryUnwind(RegisterContext* thread_context,
   do {
     uint64_t cur_pc = regs->pc();
     uint64_t cur_sp = regs->sp();
-    unwindstack::MapInfo* map_info =
-        memory_regions_map_->GetMaps()->Find(cur_pc).get();
+    unwindstack::MapInfo* map_info = memory_regions_map_->Find(cur_pc).get();
     if (map_info == nullptr ||
         map_info->flags() & unwindstack::MAPS_FLAGS_DEVICE_MAP) {
       break;
     }
 
     unwindstack::Elf* elf = map_info->GetElf(
-        {memory_regions_map_->GetMemory(), [](unwindstack::Memory*) {}}, arch);
+        {process_memory_.get(), [](unwindstack::Memory*) {}}, arch);
     if (!elf->valid())
       break;
 
@@ -253,8 +221,7 @@ UnwindResult NativeUnwinderAndroid::TryUnwind(RegisterContext* thread_context,
 
 std::unique_ptr<const ModuleCache::Module>
 NativeUnwinderAndroid::TryCreateModuleForAddress(uintptr_t address) {
-  unwindstack::MapInfo* map_info =
-      memory_regions_map_->GetMaps()->Find(address).get();
+  unwindstack::MapInfo* map_info = memory_regions_map_->Find(address).get();
   if (map_info == nullptr || !(map_info->flags() & PROT_EXEC) ||
       map_info->flags() & unwindstack::MAPS_FLAGS_DEVICE_MAP) {
     return nullptr;
@@ -271,8 +238,7 @@ void NativeUnwinderAndroid::EmitDexFrame(uintptr_t dex_pc,
     // usually not executable (.dex file). Since non-executable regions
     // are used much less commonly, it's lazily added here instead of from
     // AddInitialModulesFromMaps().
-    unwindstack::MapInfo* map_info =
-        memory_regions_map_->GetMaps()->Find(dex_pc).get();
+    unwindstack::MapInfo* map_info = memory_regions_map_->Find(dex_pc).get();
     if (map_info) {
       auto new_module = std::make_unique<NonElfModule>(map_info);
       module = new_module.get();

base/profiler/native_unwinder_android.h

@@ -10,13 +10,11 @@
 
 #include "base/memory/raw_ptr.h"
 #include "base/profiler/unwinder.h"
+#include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/Maps.h"
 #include "third_party/libunwindstack/src/libunwindstack/include/unwindstack/Memory.h"
 
 namespace base {
 
-class NativeUnwinderAndroidMapDelegate;
-class NativeUnwinderAndroidMemoryRegionsMap;
-
 // 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.
@@ -39,24 +37,22 @@ class NativeUnwinderAndroid : public Unwinder,
   // 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<NativeUnwinderAndroidMemoryRegionsMap>
-  CreateMemoryRegionsMap();
+  static std::unique_ptr<unwindstack::Maps> CreateMaps();
+  static std::unique_ptr<unwindstack::Memory> CreateProcessMemory();
 
+  // |memory_regions_map| and |process_memory| must outlive this unwinder.
   // |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.
-  // |map_delegate| is used to manage memory used by libunwindstack. It must
-  // outlives this object.
-  NativeUnwinderAndroid(uintptr_t exclude_module_with_base_address,
-                        NativeUnwinderAndroidMapDelegate* map_delegate);
+  NativeUnwinderAndroid(unwindstack::Maps* memory_regions_map,
+                        unwindstack::Memory* process_memory,
+                        uintptr_t exclude_module_with_base_address);
   ~NativeUnwinderAndroid() override;
 
   NativeUnwinderAndroid(const NativeUnwinderAndroid&) = delete;
   NativeUnwinderAndroid& operator=(const NativeUnwinderAndroid&) = delete;
 
-  void SetMapDelegateForTesting(NativeUnwinderAndroidMapDelegate* map_delegate);
-
   // Unwinder
   void InitializeModules() override;
   bool CanUnwindFrom(const Frame& current_frame) const override;
@@ -72,9 +68,9 @@ class NativeUnwinderAndroid : public Unwinder,
   void EmitDexFrame(uintptr_t dex_pc,
                     std::vector<Frame>* stack) const;
 
+  const raw_ptr<unwindstack::Maps> memory_regions_map_;
+  const raw_ptr<unwindstack::Memory> process_memory_;
   const uintptr_t exclude_module_with_base_address_;
-  raw_ptr<NativeUnwinderAndroidMapDelegate> map_delegate_;
-  const raw_ptr<NativeUnwinderAndroidMemoryRegionsMap> memory_regions_map_;
 };
 
 }  // namespace base