From 61ba7bbd60d879ff3da1cb46d24c195f2442987f Mon Sep 17 00:00:00 2001 From: Boris Sazonov Date: Fri, 17 Apr 2020 11:45:21 +0000 Subject: [PATCH] Revert "Reland "[Clank SSM]: Implement NativeUnwinderAndroid."" This reverts commit 4eee5285b15b696b85b3f8434833138239941bd3. Reason for revert: base_unittests failures (https://ci.chromium.org/p/chromium/builders/ci/Lollipop%20Phone%20Tester/25027). Original change's description: > Reland "[Clank SSM]: Implement NativeUnwinderAndroid." > > This is a reland of 38e3a62ae16a3e73108209d584f4ae4e98bcc661 > > Reason for revert: Official build failure crbug/1071307 > Fix: Compile NativeUnwinderAndroid + tests only on arm/arm64 > > 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 > > Reviewed-by: Nico Weber > > Reviewed-by: Mike Wittman > > Cr-Commit-Position: refs/heads/master@{#759283} > > Bug: 989102, 1071307 > Change-Id: I03345dc46b205f3c1b211ffa9aeb6d51fdacaacf > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2151727 > Commit-Queue: Etienne Pierre-Doray > Reviewed-by: Mike Wittman > Reviewed-by: Nico Weber > Cr-Commit-Position: refs/heads/master@{#759777} TBR=gab@chromium.org,thakis@chromium.org,wittman@chromium.org,etiennep@chromium.org Bug: 989102, 1071768 Change-Id: I90411b354f4f357de7a1dffcd194051eac2eb820 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2154186 Reviewed-by: Boris Sazonov Commit-Queue: Boris Sazonov Cr-Commit-Position: refs/heads/master@{#760019} --- base/BUILD.gn | 61 +-- base/DEPS | 1 - .../chromium/base/profiler/TestSupport.java | 25 -- base/profiler/native_unwinder_android.cc | 209 +--------- base/profiler/native_unwinder_android.h | 37 +- .../native_unwinder_android_unittest.cc | 362 ------------------ base/profiler/register_context.h | 2 +- .../stack_sampling_profiler_test_util.cc | 82 ---- .../stack_sampling_profiler_test_util.h | 18 - .../stack_sampling_profiler_unittest.cc | 81 ++++ base/profiler/unwindstack_internal_android.cc | 30 -- base/profiler/unwindstack_internal_android.h | 34 -- 12 files changed, 99 insertions(+), 843 deletions(-) delete mode 100644 base/android/javatests/src/org/chromium/base/profiler/TestSupport.java delete mode 100644 base/profiler/native_unwinder_android_unittest.cc delete mode 100644 base/profiler/unwindstack_internal_android.cc delete mode 100644 base/profiler/unwindstack_internal_android.h diff --git a/base/BUILD.gn b/base/BUILD.gn index 80777f58258dae..9e54bbe696cf04 100644 --- a/base/BUILD.gn +++ b/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,29 +2385,13 @@ if (is_win) { } } -if ((is_win && current_cpu != "x86") || is_mac || - (is_android && (current_cpu == "arm" || current_cpu == "arm64"))) { - # Must be a loadable module so that it can be loaded/unloaded at runtime - # during testing. - loadable_module("base_profiler_test_support_library") { - testonly = true - sources = [ "profiler/test_support_library.cc" ] - } -} - -if (is_android && (current_cpu == "arm" || current_cpu == "arm64")) { - 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" ] +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" ] + } } } @@ -2947,17 +2933,6 @@ test("base_unittests") { if (current_cpu == "arm") { sources += [ "profiler/chrome_unwinder_android_unittest.cc" ] } - if (current_cpu == "arm" || current_cpu == "arm64") { - sources += [ "profiler/native_unwinder_android_unittest.cc" ] - include_dirs = - [ "//third_party/libunwindstack/src/libunwindstack/include" ] - deps += [ - ":base_profiler_test_support_java", - ":base_profiler_test_support_jni_headers", - ":base_profiler_test_support_library", - ":native_unwinder_android", - ] - } sources += [ "android/android_image_reader_compat_unittest.cc", @@ -3679,24 +3654,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 } diff --git a/base/DEPS b/base/DEPS index 477a867eafb8f0..1c8911d04302b1 100644 --- a/base/DEPS +++ b/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", diff --git a/base/android/javatests/src/org/chromium/base/profiler/TestSupport.java b/base/android/javatests/src/org/chromium/base/profiler/TestSupport.java deleted file mode 100644 index ea5f268c392dc2..00000000000000 --- a/base/android/javatests/src/org/chromium/base/profiler/TestSupport.java +++ /dev/null @@ -1,25 +0,0 @@ -// 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); - } -} diff --git a/base/profiler/native_unwinder_android.cc b/base/profiler/native_unwinder_android.cc index fa06494aee232e..d5f9cfeec4ece0 100644 --- a/base/profiler/native_unwinder_android.cc +++ b/base/profiler/native_unwinder_android.cc @@ -4,226 +4,25 @@ #include "base/profiler/native_unwinder_android.h" -#include -#include - -#include - -#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/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 CreateFromRegisterContext( - RegisterContext* thread_context) { -#if defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_32_BITS) - return WrapUnique(unwindstack::RegsArm::Read( - reinterpret_cast(&thread_context->arm_r0))); -#elif defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_64_BITS) - return WrapUnique(unwindstack::RegsArm64::Read( - reinterpret_cast(&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(&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(&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 NativeUnwinderAndroid::CreateMaps() { - auto maps = std::make_unique(); - if (maps->Parse()) - return maps; - return nullptr; -} - -// static -std::unique_ptr -NativeUnwinderAndroid::CreateProcessMemory() { - return std::make_unique(); -} - -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(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* 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* 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(map_info); - module = new_module.get(); - module_cache->AddCustomNativeModule(std::move(new_module)); - } - } - stack->emplace_back(dex_pc, module); +std::unique_ptr CreateNativeUnwinder(ModuleCache* module_cache) { + return std::make_unique(); } } // namespace base diff --git a/base/profiler/native_unwinder_android.h b/base/profiler/native_unwinder_android.h index 926a581b32a604..16f1b7b39aa71a 100644 --- a/base/profiler/native_unwinder_android.h +++ b/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 CreateMaps(); - static std::unique_ptr 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* stack) const override; - - private: - void EmitDexFrame(uintptr_t dex_pc, - ModuleCache* module_cache, - std::vector* stack) const; - - unwindstack::Maps* const memory_regions_map_; - unwindstack::Memory* const process_memory_; - const uintptr_t exclude_module_with_base_address_; }; } // namespace base diff --git a/base/profiler/native_unwinder_android_unittest.cc b/base/profiler/native_unwinder_android_unittest.cc deleted file mode 100644 index 2e02b08d1fe28b..00000000000000 --- a/base/profiler/native_unwinder_android_unittest.cc +++ /dev/null @@ -1,362 +0,0 @@ -// 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. - -#include "base/profiler/native_unwinder_android.h" - -#include - -#include "base/android/jni_android.h" -#include "base/base_profiler_test_support_jni_headers/TestSupport_jni.h" -#include "base/bind.h" -#include "base/profiler/register_context.h" -#include "base/profiler/stack_buffer.h" -#include "base/profiler/stack_copier_signal.h" -#include "base/profiler/stack_sampling_profiler_test_util.h" -#include "base/profiler/thread_delegate_posix.h" -#include "base/profiler/unwindstack_internal_android.h" -#include "base/test/bind_test_util.h" -#include "build/build_config.h" -#include "testing/gtest/include/gtest/gtest.h" - -extern char __executable_start; - -namespace base { - -class TestStackCopierDelegate : public StackCopier::Delegate { - public: - void OnStackCopy() override {} - void OnThreadResume() override {} -}; - -std::vector CaptureScenario( - UnwindScenario* scenario, - ModuleCache* module_cache, - OnceCallback*)> - unwind_callback) { - std::vector sample; - WithTargetThread( - scenario, - BindLambdaForTesting( - [&](SamplingProfilerThreadToken target_thread_token) { - auto stack_copier = std::make_unique( - std::make_unique(target_thread_token)); - std::unique_ptr stack_buffer = - StackSampler::CreateStackBuffer(); - - RegisterContext thread_context; - uintptr_t stack_top; - TimeTicks timestamp; - TestStackCopierDelegate delegate; - bool success = - stack_copier->CopyStack(stack_buffer.get(), &stack_top, - ×tamp, &thread_context, &delegate); - ASSERT_TRUE(success); - - sample.emplace_back( - RegisterContextInstructionPointer(&thread_context), - module_cache->GetModuleForAddress( - RegisterContextInstructionPointer(&thread_context))); - - std::move(unwind_callback).Run(&thread_context, stack_top, &sample); - })); - - return sample; -} - -// Checks that the expected information is present in sampled frames. -TEST(NativeUnwinderAndroidTest, PlainFunction) { - UnwindScenario scenario(BindRepeating(&CallWithPlainFunction)); - - std::unique_ptr maps = NativeUnwinderAndroid::CreateMaps(); - std::unique_ptr memory = - NativeUnwinderAndroid::CreateProcessMemory(); - auto unwinder = - std::make_unique(maps.get(), memory.get(), 0); - - ModuleCache module_cache; - unwinder->AddInitialModules(&module_cache); - std::vector sample = - CaptureScenario(&scenario, &module_cache, - BindLambdaForTesting([&](RegisterContext* thread_context, - uintptr_t stack_top, - std::vector* sample) { - ASSERT_TRUE(unwinder->CanUnwindFrom(sample->back())); - UnwindResult result = unwinder->TryUnwind( - thread_context, stack_top, &module_cache, sample); - EXPECT_EQ(UnwindResult::COMPLETED, result); - })); - - // Check that all the modules are valid. - for (const auto& frame : sample) - EXPECT_NE(nullptr, frame.module); - - // The stack should contain a full unwind. - ExpectStackContains(sample, {scenario.GetWaitForSampleAddressRange(), - scenario.GetSetupFunctionAddressRange(), - scenario.GetOuterFunctionAddressRange()}); -} - -// Checks that the unwinder handles stacks containing dynamically-allocated -// stack memory. -TEST(NativeUnwinderAndroidTest, Alloca) { - UnwindScenario scenario(BindRepeating(&CallWithAlloca)); - - std::unique_ptr maps = NativeUnwinderAndroid::CreateMaps(); - std::unique_ptr memory = - NativeUnwinderAndroid::CreateProcessMemory(); - auto unwinder = - std::make_unique(maps.get(), memory.get(), 0); - - ModuleCache module_cache; - unwinder->AddInitialModules(&module_cache); - std::vector sample = - CaptureScenario(&scenario, &module_cache, - BindLambdaForTesting([&](RegisterContext* thread_context, - uintptr_t stack_top, - std::vector* sample) { - ASSERT_TRUE(unwinder->CanUnwindFrom(sample->back())); - UnwindResult result = unwinder->TryUnwind( - thread_context, stack_top, &module_cache, sample); - EXPECT_EQ(UnwindResult::COMPLETED, result); - })); - - // Check that all the modules are valid. - for (const auto& frame : sample) - EXPECT_NE(nullptr, frame.module); - - // The stack should contain a full unwind. - ExpectStackContains(sample, {scenario.GetWaitForSampleAddressRange(), - scenario.GetSetupFunctionAddressRange(), - scenario.GetOuterFunctionAddressRange()}); -} - -// Checks that a stack that runs through another library produces a stack with -// the expected functions. -TEST(NativeUnwinderAndroidTest, OtherLibrary) { - NativeLibrary other_library = LoadOtherLibrary(); - UnwindScenario scenario( - BindRepeating(&CallThroughOtherLibrary, Unretained(other_library))); - - std::unique_ptr maps = NativeUnwinderAndroid::CreateMaps(); - std::unique_ptr memory = - NativeUnwinderAndroid::CreateProcessMemory(); - auto unwinder = - std::make_unique(maps.get(), memory.get(), 0); - - ModuleCache module_cache; - unwinder->AddInitialModules(&module_cache); - std::vector sample = - CaptureScenario(&scenario, &module_cache, - BindLambdaForTesting([&](RegisterContext* thread_context, - uintptr_t stack_top, - std::vector* sample) { - ASSERT_TRUE(unwinder->CanUnwindFrom(sample->back())); - UnwindResult result = unwinder->TryUnwind( - thread_context, stack_top, &module_cache, sample); - EXPECT_EQ(UnwindResult::COMPLETED, result); - })); - - // The stack should contain a full unwind. - ExpectStackContains(sample, {scenario.GetWaitForSampleAddressRange(), - scenario.GetSetupFunctionAddressRange(), - scenario.GetOuterFunctionAddressRange()}); -} - -// Check that unwinding is interrupted for excluded modules. -TEST(NativeUnwinderAndroidTest, ExcludeOtherLibrary) { - NativeLibrary other_library = LoadOtherLibrary(); - UnwindScenario scenario( - BindRepeating(&CallThroughOtherLibrary, Unretained(other_library))); - - std::unique_ptr maps = NativeUnwinderAndroid::CreateMaps(); - std::unique_ptr memory = - NativeUnwinderAndroid::CreateProcessMemory(); - ModuleCache module_cache; - NativeUnwinderAndroid::AddInitialModulesFromMaps(*maps, &module_cache); - - auto unwinder = std::make_unique( - maps.get(), memory.get(), - module_cache.GetModuleForAddress(GetAddressInOtherLibrary(other_library)) - ->GetBaseAddress()); - std::vector sample = - CaptureScenario(&scenario, &module_cache, - BindLambdaForTesting([&](RegisterContext* thread_context, - uintptr_t stack_top, - std::vector* sample) { - ASSERT_TRUE(unwinder->CanUnwindFrom(sample->back())); - EXPECT_EQ(UnwindResult::UNRECOGNIZED_FRAME, - unwinder->TryUnwind(thread_context, stack_top, - &module_cache, sample)); - EXPECT_FALSE(unwinder->CanUnwindFrom(sample->back())); - })); - - ExpectStackContains(sample, {scenario.GetWaitForSampleAddressRange()}); - ExpectStackDoesNotContain(sample, {scenario.GetSetupFunctionAddressRange(), - scenario.GetOuterFunctionAddressRange()}); -} - -// Check that unwinding can be resumed after an incomplete unwind. -TEST(NativeUnwinderAndroidTest, ResumeUnwinding) { - NativeLibrary other_library = LoadOtherLibrary(); - UnwindScenario scenario( - BindRepeating(&CallThroughOtherLibrary, Unretained(other_library))); - - std::unique_ptr maps = NativeUnwinderAndroid::CreateMaps(); - std::unique_ptr memory = - NativeUnwinderAndroid::CreateProcessMemory(); - ModuleCache module_cache; - NativeUnwinderAndroid::AddInitialModulesFromMaps(*maps, &module_cache); - - // Several unwinders are used to unwind different portion of the stack. This - // tests that NativeUnwinderAndroid can pick up from a state in the middle of - // the stack. This emulates having NativeUnwinderAndroid work with other - // unwinders, but doesn't reproduce what happens in production. - auto unwinder_for_all = - std::make_unique(maps.get(), memory.get(), 0); - auto unwinder_for_native = std::make_unique( - maps.get(), memory.get(), - reinterpret_cast(&__executable_start)); - auto unwinder_for_chrome = std::make_unique( - maps.get(), memory.get(), - module_cache.GetModuleForAddress(GetAddressInOtherLibrary(other_library)) - ->GetBaseAddress()); - - std::vector sample = CaptureScenario( - &scenario, &module_cache, - BindLambdaForTesting([&](RegisterContext* thread_context, - uintptr_t stack_top, - std::vector* sample) { - // |unwinder_for_native| unwinds through native frames, but stops at - // chrome frames. It might not contain SampleAddressRange. - ASSERT_TRUE(unwinder_for_native->CanUnwindFrom(sample->back())); - EXPECT_EQ(UnwindResult::UNRECOGNIZED_FRAME, - unwinder_for_native->TryUnwind(thread_context, stack_top, - &module_cache, sample)); - EXPECT_FALSE(unwinder_for_native->CanUnwindFrom(sample->back())); - - ExpectStackDoesNotContain(*sample, - {scenario.GetSetupFunctionAddressRange(), - scenario.GetOuterFunctionAddressRange()}); - size_t prior_stack_size = sample->size(); - - // |unwinder_for_chrome| unwinds through Chrome frames, but stops at - // |other_library|. It won't contain SetupFunctionAddressRange. - ASSERT_TRUE(unwinder_for_chrome->CanUnwindFrom(sample->back())); - EXPECT_EQ(UnwindResult::UNRECOGNIZED_FRAME, - unwinder_for_chrome->TryUnwind(thread_context, stack_top, - &module_cache, sample)); - EXPECT_FALSE(unwinder_for_chrome->CanUnwindFrom(sample->back())); - EXPECT_LT(prior_stack_size, sample->size()); - ExpectStackContains(*sample, {scenario.GetWaitForSampleAddressRange()}); - ExpectStackDoesNotContain(*sample, - {scenario.GetSetupFunctionAddressRange(), - scenario.GetOuterFunctionAddressRange()}); - - // |unwinder_for_all| should complete unwinding through all frames. - ASSERT_TRUE(unwinder_for_all->CanUnwindFrom(sample->back())); - EXPECT_EQ(UnwindResult::COMPLETED, - unwinder_for_all->TryUnwind(thread_context, stack_top, - &module_cache, sample)); - })); - - // The stack should contain a full unwind. - ExpectStackContains(sample, {scenario.GetWaitForSampleAddressRange(), - scenario.GetSetupFunctionAddressRange(), - scenario.GetOuterFunctionAddressRange()}); -} - -struct JavaTestSupportParams { - OnceClosure wait_for_sample; - FunctionAddressRange range; -}; - -void JNI_TestSupport_InvokeCallbackFunction(JNIEnv* env, jlong context) { - const void* start_program_counter = GetProgramCounter(); - - JavaTestSupportParams* params = - reinterpret_cast(context); - if (!params->wait_for_sample.is_null()) - std::move(params->wait_for_sample).Run(); - - // Volatile to prevent a tail call to GetProgramCounter(). - const void* volatile end_program_counter = GetProgramCounter(); - - params->range = {start_program_counter, end_program_counter}; -} - -// Checks that java frames can be unwound through. -// Due to varying availability of compiled java unwind tables, the test is only -// enabled on arm 32-bits. -#if defined(ARCH_CPU_ARM_FAMILY) && defined(ARCH_CPU_32_BITS) -#define MAYBE_JavaFunction JavaFunction -#else -#define MAYBE_JavaFunction DISABLED_JavaFunction -#endif -TEST(NativeUnwinderAndroidTest, MAYBE_JavaFunction) { - UnwindScenario scenario(BindLambdaForTesting([](OnceClosure wait_for_sample) { - JNIEnv* env = base::android::AttachCurrentThread(); - JavaTestSupportParams params{std::move(wait_for_sample), {}}; - base::Java_TestSupport_callWithJavaFunction( - env, reinterpret_cast(¶ms)); - return params.range; - })); - - std::unique_ptr maps = NativeUnwinderAndroid::CreateMaps(); - std::unique_ptr memory = - NativeUnwinderAndroid::CreateProcessMemory(); - auto unwinder = - std::make_unique(maps.get(), memory.get(), 0); - - ModuleCache module_cache; - unwinder->AddInitialModules(&module_cache); - std::vector sample = - CaptureScenario(&scenario, &module_cache, - BindLambdaForTesting([&](RegisterContext* thread_context, - uintptr_t stack_top, - std::vector* sample) { - ASSERT_TRUE(unwinder->CanUnwindFrom(sample->back())); - EXPECT_EQ(UnwindResult::COMPLETED, - unwinder->TryUnwind(thread_context, stack_top, - &module_cache, sample)); - })); - - // Check that all the modules are valid. - for (const auto& frame : sample) - EXPECT_NE(nullptr, frame.module); - - // The stack should contain a full unwind. - ExpectStackContains(sample, {scenario.GetWaitForSampleAddressRange(), - scenario.GetSetupFunctionAddressRange(), - scenario.GetOuterFunctionAddressRange()}); -} - -TEST(NativeUnwinderAndroidTest, UnwindStackMemoryTest) { - std::vector input = {1, 2, 3, 4, 5}; - uintptr_t begin = reinterpret_cast(input.data()); - uintptr_t end = reinterpret_cast(input.data() + input.size()); - UnwindStackMemoryAndroid memory(begin, end); - - const auto check_read_fails = [&](uintptr_t addr, size_t size) { - std::vector output(size); - EXPECT_EQ(0U, memory.Read(addr, output.data(), size)); - }; - const auto check_read_succeeds = [&](uintptr_t addr, size_t size) { - std::vector output(size); - EXPECT_EQ(size, memory.Read(addr, output.data(), size)); - EXPECT_EQ( - 0, memcmp(reinterpret_cast(addr), output.data(), size)); - }; - - check_read_fails(begin - 1, 1); - check_read_fails(begin - 1, 2); - check_read_fails(end, 1); - check_read_fails(end, 2); - check_read_fails(end - 1, 2); - - check_read_succeeds(begin, 1); - check_read_succeeds(begin, 5); - check_read_succeeds(end - 1, 1); -} - -} // namespace base diff --git a/base/profiler/register_context.h b/base/profiler/register_context.h index 14380a527848c2..8ced8dae433ce9 100644 --- a/base/profiler/register_context.h +++ b/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) diff --git a/base/profiler/stack_sampling_profiler_test_util.cc b/base/profiler/stack_sampling_profiler_test_util.cc index e9574a15a9916a..f9ad0921a3914e 100644 --- a/base/profiler/stack_sampling_profiler_test_util.cc +++ b/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 sample_; }; -// The function to be executed by the code in the other library. -void OtherLibraryCallback(void* arg) { - OnceClosure* wait_for_sample = static_cast(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(static_cast(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( - 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( - GetFunctionPointerFromNativeLibrary(library, "InvokeCallbackFunction")); - EXPECT_NE(address, 0u); - return address; -} - } // namespace base diff --git a/base/profiler/stack_sampling_profiler_test_util.h b/base/profiler/stack_sampling_profiler_test_util.h index ff2daebfe14b42..2e0c2b9c9c1b01 100644 --- a/base/profiler/stack_sampling_profiler_test_util.h +++ b/base/profiler/stack_sampling_profiler_test_util.h @@ -9,10 +9,8 @@ #include #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; @@ -134,12 +122,6 @@ void ExpectStackDoesNotContain( const std::vector& stack, const std::vector& 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_ diff --git a/base/profiler/stack_sampling_profiler_unittest.cc b/base/profiler/stack_sampling_profiler_unittest.cc index b7a06f9929ac23..071874700f561a 100644 --- a/base/profiler/stack_sampling_profiler_unittest.cc +++ b/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(static_cast(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(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( + 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. diff --git a/base/profiler/unwindstack_internal_android.cc b/base/profiler/unwindstack_internal_android.cc deleted file mode 100644 index 92328590aa455c..00000000000000 --- a/base/profiler/unwindstack_internal_android.cc +++ /dev/null @@ -1,30 +0,0 @@ -// 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 - -#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(addr), size); - return size; -} - -} // namespace base \ No newline at end of file diff --git a/base/profiler/unwindstack_internal_android.h b/base/profiler/unwindstack_internal_android.h deleted file mode 100644 index 75058613fc7e04..00000000000000 --- a/base/profiler/unwindstack_internal_android.h +++ /dev/null @@ -1,34 +0,0 @@ -// 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