From a46a4885e1417ff1e13b3b480d53634ef20f4203 Mon Sep 17 00:00:00 2001 From: Mike Wittman Date: Wed, 7 Aug 2019 15:14:40 +0000 Subject: [PATCH] Extract StackBuffer to own file In subsequent CLs this class will be used from low-level code where it doesn't make sense to take a dependency StackSamplerImpl. TBR=gab@chromium.org Bug: 988579 Change-Id: I0b04a42c0a4c79157aefaccca8092cf3bdc89eaa Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1738669 Commit-Queue: Mike Wittman Reviewed-by: Charlie Andrews Reviewed-by: oysteine Cr-Commit-Position: refs/heads/master@{#684761} --- base/BUILD.gn | 2 + base/profiler/stack_buffer.cc | 17 ++++++ base/profiler/stack_buffer.h | 58 +++++++++++++++++++ base/profiler/stack_sampler.cc | 11 +--- base/profiler/stack_sampler.h | 40 +------------ base/profiler/stack_sampler_impl.cc | 8 +-- base/profiler/stack_sampler_impl_unittest.cc | 26 ++++----- base/profiler/stack_sampling_profiler.cc | 3 +- .../stack_sampling/stack_sampler_android.cc | 2 +- .../stack_sampling/stack_sampler_android.h | 2 +- .../stack_sampling/stack_unwinder_android.cc | 25 ++++---- .../stack_sampling/stack_unwinder_android.h | 17 +++--- .../stack_unwinder_android_unittest.cc | 1 + 13 files changed, 120 insertions(+), 92 deletions(-) create mode 100644 base/profiler/stack_buffer.cc create mode 100644 base/profiler/stack_buffer.h diff --git a/base/BUILD.gn b/base/BUILD.gn index c99fad581c021f..604fd0f4be0313 100644 --- a/base/BUILD.gn +++ b/base/BUILD.gn @@ -625,6 +625,8 @@ jumbo_component("base") { "profiler/register_context.h", "profiler/sample_metadata.cc", "profiler/sample_metadata.h", + "profiler/stack_buffer.cc", + "profiler/stack_buffer.h", "profiler/stack_sampler.cc", "profiler/stack_sampler.h", "profiler/stack_sampler_android.cc", diff --git a/base/profiler/stack_buffer.cc b/base/profiler/stack_buffer.cc new file mode 100644 index 00000000000000..c85271473300fe --- /dev/null +++ b/base/profiler/stack_buffer.cc @@ -0,0 +1,17 @@ +// 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/stack_buffer.h" + +namespace base { + +constexpr size_t StackBuffer::kPlatformStackAlignment; + +StackBuffer::StackBuffer(size_t buffer_size) + : buffer_(new uint8_t[buffer_size + kPlatformStackAlignment - 1]), + size_(buffer_size) {} + +StackBuffer::~StackBuffer() = default; + +} // namespace base diff --git a/base/profiler/stack_buffer.h b/base/profiler/stack_buffer.h new file mode 100644 index 00000000000000..e678aa286ade23 --- /dev/null +++ b/base/profiler/stack_buffer.h @@ -0,0 +1,58 @@ +// 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. + +#ifndef BASE_PROFILER_STACK_BUFFER_H_ +#define BASE_PROFILER_STACK_BUFFER_H_ + +#include +#include +#include + +#include "base/base_export.h" +#include "base/macros.h" + +namespace base { + +// This class contains a buffer for stack copies that can be shared across +// multiple instances of StackSampler. +class BASE_EXPORT StackBuffer { + public: + // The expected alignment of the stack on the current platform. Windows and + // System V AMD64 ABIs on x86, x64, and ARM require the stack to be aligned + // to twice the pointer size. Excepted from this requirement is code setting + // up the stack during function calls (between pushing the return address + // and the end of the function prologue). The profiler will sometimes + // encounter this exceptional case for leaf frames. + static constexpr size_t kPlatformStackAlignment = 2 * sizeof(uintptr_t); + + StackBuffer(size_t buffer_size); + ~StackBuffer(); + + // Returns a kPlatformStackAlignment-aligned pointer to the stack buffer. + uintptr_t* buffer() const { + // Return the first address in the buffer aligned to + // kPlatformStackAlignment. The buffer is guaranteed to have enough space + // for size() bytes beyond this value. + return reinterpret_cast( + (reinterpret_cast(buffer_.get()) + kPlatformStackAlignment - + 1) & + ~(kPlatformStackAlignment - 1)); + } + + size_t size() const { return size_; } + + private: + // The buffer to store the stack. + const std::unique_ptr buffer_; + + // The size of the requested buffer allocation. The actual allocation is + // larger to accommodate alignment requirements. + const size_t size_; + + DISALLOW_COPY_AND_ASSIGN(StackBuffer); +}; + +} // namespace base + +#endif // BASE_PROFILER_STACK_BUFFER_H_ diff --git a/base/profiler/stack_sampler.cc b/base/profiler/stack_sampler.cc index c191e592405d79..e0704e8caad574 100644 --- a/base/profiler/stack_sampler.cc +++ b/base/profiler/stack_sampler.cc @@ -5,22 +5,15 @@ #include "base/profiler/stack_sampler.h" #include "base/memory/ptr_util.h" +#include "base/profiler/stack_buffer.h" namespace base { -constexpr size_t StackSampler::StackBuffer::kPlatformStackAlignment; - -StackSampler::StackBuffer::StackBuffer(size_t buffer_size) - : buffer_(new uint8_t[buffer_size + kPlatformStackAlignment - 1]), - size_(buffer_size) {} - -StackSampler::StackBuffer::~StackBuffer() = default; - StackSampler::StackSampler() = default; StackSampler::~StackSampler() = default; -std::unique_ptr StackSampler::CreateStackBuffer() { +std::unique_ptr StackSampler::CreateStackBuffer() { size_t size = GetStackBufferSize(); if (size == 0) return nullptr; diff --git a/base/profiler/stack_sampler.h b/base/profiler/stack_sampler.h index f4d7824c674348..42e14efd9e86bc 100644 --- a/base/profiler/stack_sampler.h +++ b/base/profiler/stack_sampler.h @@ -16,6 +16,7 @@ namespace base { class Unwinder; class ModuleCache; class ProfileBuilder; +class StackBuffer; class StackSamplerTestDelegate; // StackSampler is an implementation detail of StackSamplingProfiler. It @@ -23,45 +24,6 @@ class StackSamplerTestDelegate; // for a given thread. class BASE_EXPORT StackSampler { public: - // This class contains a buffer for stack copies that can be shared across - // multiple instances of StackSampler. - class BASE_EXPORT StackBuffer { - public: - // The expected alignment of the stack on the current platform. Windows and - // System V AMD64 ABIs on x86, x64, and ARM require the stack to be aligned - // to twice the pointer size. Excepted from this requirement is code setting - // up the stack during function calls (between pushing the return address - // and the end of the function prologue). The profiler will sometimes - // encounter this exceptional case for leaf frames. - static constexpr size_t kPlatformStackAlignment = 2 * sizeof(uintptr_t); - - StackBuffer(size_t buffer_size); - ~StackBuffer(); - - // Returns a kPlatformStackAlignment-aligned pointer to the stack buffer. - uintptr_t* buffer() const { - // Return the first address in the buffer aligned to - // kPlatformStackAlignment. The buffer is guaranteed to have enough space - // for size() bytes beyond this value. - return reinterpret_cast( - (reinterpret_cast(buffer_.get()) + - kPlatformStackAlignment - 1) & - ~(kPlatformStackAlignment - 1)); - } - - size_t size() const { return size_; } - - private: - // The buffer to store the stack. - const std::unique_ptr buffer_; - - // The size of the requested buffer allocation. The actual allocation is - // larger to accommodate alignment requirements. - const size_t size_; - - DISALLOW_COPY_AND_ASSIGN(StackBuffer); - }; - virtual ~StackSampler(); // Creates a stack sampler that records samples for thread with |thread_id|. diff --git a/base/profiler/stack_sampler_impl.cc b/base/profiler/stack_sampler_impl.cc index 6993664fd7f762..2dbfb7e380d872 100644 --- a/base/profiler/stack_sampler_impl.cc +++ b/base/profiler/stack_sampler_impl.cc @@ -9,6 +9,7 @@ #include "base/logging.h" #include "base/profiler/profile_builder.h" #include "base/profiler/sample_metadata.h" +#include "base/profiler/stack_buffer.h" #include "base/profiler/thread_delegate.h" #include "base/profiler/unwinder.h" @@ -237,10 +238,9 @@ const uint8_t* CopyStackContentsAndRewritePointers( // alignment between values in the original stack and the copy. This uses the // platform stack alignment rather than pointer alignment so that the stack // copy is aligned to platform expectations. - uint8_t* stack_copy_bottom = - reinterpret_cast(stack_buffer_bottom) + - (reinterpret_cast(byte_src) & - (StackSampler::StackBuffer::kPlatformStackAlignment - 1)); + uint8_t* stack_copy_bottom = reinterpret_cast(stack_buffer_bottom) + + (reinterpret_cast(byte_src) & + (StackBuffer::kPlatformStackAlignment - 1)); uint8_t* byte_dst = stack_copy_bottom; // Copy bytes verbatim up to the first aligned address. diff --git a/base/profiler/stack_sampler_impl_unittest.cc b/base/profiler/stack_sampler_impl_unittest.cc index f86d4330e33984..2b373bde65aa11 100644 --- a/base/profiler/stack_sampler_impl_unittest.cc +++ b/base/profiler/stack_sampler_impl_unittest.cc @@ -9,6 +9,7 @@ #include #include "base/profiler/profile_builder.h" +#include "base/profiler/stack_buffer.h" #include "base/profiler/stack_sampler_impl.h" #include "base/profiler/thread_delegate.h" #include "base/profiler/unwinder.h" @@ -229,8 +230,7 @@ class FakeTestUnwinder : public Unwinder { static constexpr size_t kTestStackBufferSize = sizeof(uintptr_t) * 4; -union alignas(StackSampler::StackBuffer::kPlatformStackAlignment) - TestStackBuffer { +union alignas(StackBuffer::kPlatformStackAlignment) TestStackBuffer { uintptr_t as_uintptr[kTestStackBufferSize / sizeof(uintptr_t)]; uint16_t as_uint16[kTestStackBufferSize / sizeof(uint16_t)]; uint8_t as_uint8[kTestStackBufferSize / sizeof(uint8_t)]; @@ -301,7 +301,7 @@ TEST(StackSamplerImplTest, StackCopy_NonAlignedStackPointerCopy) { // Leave extra space within the stack buffer beyond the end of the stack, but // preserve the platform alignment. - const size_t extra_space = StackSampler::StackBuffer::kPlatformStackAlignment; + const size_t extra_space = StackBuffer::kPlatformStackAlignment; uintptr_t* stack_top = &stack_buffer.as_uintptr[size(stack_buffer.as_uintptr) - extra_space / sizeof(uintptr_t)]; @@ -439,9 +439,8 @@ TEST(StackSamplerImplTest, CopyStack) { std::make_unique(stack), std::make_unique(stack.size(), &stack_copy), &module_cache); - std::unique_ptr stack_buffer = - std::make_unique(stack.size() * - sizeof(uintptr_t)); + std::unique_ptr stack_buffer = + std::make_unique(stack.size() * sizeof(uintptr_t)); TestProfileBuilder profile_builder(&module_cache); stack_sampler_impl.RecordStackFrames(stack_buffer.get(), &profile_builder); @@ -457,9 +456,8 @@ TEST(StackSamplerImplTest, CopyStackBufferTooSmall) { std::make_unique(stack), std::make_unique(stack.size(), &stack_copy), &module_cache); - std::unique_ptr stack_buffer = - std::make_unique((stack.size() - 1) * - sizeof(uintptr_t)); + std::unique_ptr stack_buffer = + std::make_unique((stack.size() - 1) * sizeof(uintptr_t)); // Make the buffer different than the input stack. stack_buffer->buffer()[0] = 100; TestProfileBuilder profile_builder(&module_cache); @@ -486,9 +484,8 @@ TEST(StackSamplerImplTest, CopyStackAndRewritePointers) { &stack_copy_bottom), &module_cache); - std::unique_ptr stack_buffer = - std::make_unique(stack.size() * - sizeof(uintptr_t)); + std::unique_ptr stack_buffer = + std::make_unique(stack.size() * sizeof(uintptr_t)); TestProfileBuilder profile_builder(&module_cache); stack_sampler_impl.RecordStackFrames(stack_buffer.get(), &profile_builder); @@ -510,9 +507,8 @@ TEST(StackSamplerImplTest, RewriteRegisters) { std::make_unique(stack.size(), nullptr, &stack_copy_bottom), &module_cache); - std::unique_ptr stack_buffer = - std::make_unique(stack.size() * - sizeof(uintptr_t)); + std::unique_ptr stack_buffer = + std::make_unique(stack.size() * sizeof(uintptr_t)); TestProfileBuilder profile_builder(&module_cache); stack_sampler_impl.RecordStackFrames(stack_buffer.get(), &profile_builder); diff --git a/base/profiler/stack_sampling_profiler.cc b/base/profiler/stack_sampling_profiler.cc index 482a04c0ccc880..a5f5e4b1c8cfdc 100644 --- a/base/profiler/stack_sampling_profiler.cc +++ b/base/profiler/stack_sampling_profiler.cc @@ -17,6 +17,7 @@ #include "base/macros.h" #include "base/memory/ptr_util.h" #include "base/memory/singleton.h" +#include "base/profiler/stack_buffer.h" #include "base/profiler/stack_sampler.h" #include "base/profiler/unwinder.h" #include "base/synchronization/lock.h" @@ -191,7 +192,7 @@ class StackSamplingProfiler::SamplingThread : public Thread { // A stack-buffer used by the sampler for its work. This buffer is re-used // across multiple sampler objects since their execution is serialized on the // sampling thread. - std::unique_ptr stack_buffer_; + std::unique_ptr stack_buffer_; // A map of collection ids to collection contexts. Because this class is a // singleton that is never destroyed, context objects will never be destructed diff --git a/services/tracing/public/cpp/stack_sampling/stack_sampler_android.cc b/services/tracing/public/cpp/stack_sampling/stack_sampler_android.cc index adbcfba6f63ae3..1628ad549ec602 100644 --- a/services/tracing/public/cpp/stack_sampling/stack_sampler_android.cc +++ b/services/tracing/public/cpp/stack_sampling/stack_sampler_android.cc @@ -25,7 +25,7 @@ void StackSamplerAndroid::AddAuxUnwinder( std::unique_ptr unwinder) {} void StackSamplerAndroid::RecordStackFrames( - StackBuffer* stack_buffer, + base::StackBuffer* stack_buffer, base::ProfileBuilder* profile_builder) { if (!unwinder_.is_initialized()) { // May block on disk access. This function is executed on the profiler diff --git a/services/tracing/public/cpp/stack_sampling/stack_sampler_android.h b/services/tracing/public/cpp/stack_sampling/stack_sampler_android.h index 0da1e022ffc828..7eb7bd8cc5b55c 100644 --- a/services/tracing/public/cpp/stack_sampling/stack_sampler_android.h +++ b/services/tracing/public/cpp/stack_sampling/stack_sampler_android.h @@ -28,7 +28,7 @@ class StackSamplerAndroid : public base::StackSampler { // StackSampler: void AddAuxUnwinder(std::unique_ptr unwinder) override; - void RecordStackFrames(StackBuffer* stack_buffer, + void RecordStackFrames(base::StackBuffer* stack_buffer, base::ProfileBuilder* profile_builder) override; private: diff --git a/services/tracing/public/cpp/stack_sampling/stack_unwinder_android.cc b/services/tracing/public/cpp/stack_sampling/stack_unwinder_android.cc index 19290bb2662950..1004ade3706a7d 100644 --- a/services/tracing/public/cpp/stack_sampling/stack_unwinder_android.cc +++ b/services/tracing/public/cpp/stack_sampling/stack_unwinder_android.cc @@ -18,6 +18,7 @@ #include "base/debug/proc_maps_linux.h" #include "base/logging.h" #include "base/metrics/histogram_macros.h" +#include "base/profiler/stack_buffer.h" #include "base/stl_util.h" #include "base/strings/string_util.h" #include "base/trace_event/cfi_backtrace_android.h" @@ -92,7 +93,7 @@ class UnwindHelper { const tracing::StackUnwinderAndroid* unwinder, uintptr_t original_sp, size_t stack_size, - base::StackSampler::StackBuffer* stack_buffer, + base::StackBuffer* stack_buffer, const void** out_trace, size_t max_depth) : use_libunwind_(use_libunwind), @@ -120,7 +121,7 @@ class UnwindHelper { size_t Unwind(uintptr_t original_sp, unw_context_t* context, const ucontext_t& signal_context, - base::StackSampler::StackBuffer* stack_buffer) { + base::StackBuffer* stack_buffer) { const uintptr_t new_stack_top = initial_sp_; // Set the frame to the return frame from signal handler. current_ip_ = signal_context.uc_mcontext.arm_pc; @@ -311,10 +312,9 @@ class UnwindHelper { return ptr_address; } - void RewritePointersAndGetMarkers( - base::StackSampler::StackBuffer* stack_buffer, - uintptr_t original_sp, - size_t stack_size) { + void RewritePointersAndGetMarkers(base::StackBuffer* stack_buffer, + uintptr_t original_sp, + size_t stack_size) { jni_markers_.clear(); uintptr_t* new_stack = stack_buffer->buffer(); constexpr uint32_t marker_l = @@ -398,7 +398,7 @@ struct HandlerParams { // The context of the return function from signal context. ucontext_t* ucontext; // Buffer to copy the stack segment. - base::StackSampler::StackBuffer* stack_buffer; + base::StackBuffer* stack_buffer; size_t* stack_size; }; @@ -534,11 +534,10 @@ size_t StackUnwinderAndroid::TraceStack(const void** out_trace, return helper.Unwind(sp, &context, sigcontext, nullptr); } -size_t StackUnwinderAndroid::TraceStack( - base::PlatformThreadId tid, - base::StackSampler::StackBuffer* stack_buffer, - const void** out_trace, - size_t max_depth) const { +size_t StackUnwinderAndroid::TraceStack(base::PlatformThreadId tid, + base::StackBuffer* stack_buffer, + const void** out_trace, + size_t max_depth) const { // Stops the thread with given tid with a signal handler. The signal handler // copies the stack of the thread and returns. This function tries to unwind // stack frames from the copied stack. @@ -579,7 +578,7 @@ bool StackUnwinderAndroid::IsAddressMapped(uintptr_t pc) const { bool StackUnwinderAndroid::SuspendThreadAndRecordStack( base::PlatformThreadId tid, - base::StackSampler::StackBuffer* stack_buffer, + base::StackBuffer* stack_buffer, uintptr_t* sp, size_t* stack_size, unw_context_t* context, diff --git a/services/tracing/public/cpp/stack_sampling/stack_unwinder_android.h b/services/tracing/public/cpp/stack_sampling/stack_unwinder_android.h index a970d1caea87c8..ae301805900f15 100644 --- a/services/tracing/public/cpp/stack_sampling/stack_unwinder_android.h +++ b/services/tracing/public/cpp/stack_sampling/stack_unwinder_android.h @@ -52,7 +52,7 @@ class COMPONENT_EXPORT(TRACING_CPP) StackUnwinderAndroid { // Same as above function, but pauses the thread with the given |tid| and then // unwinds. |tid| should not be current thread's. size_t TraceStack(base::PlatformThreadId tid, - base::StackSampler::StackBuffer* stack_buffer, + base::StackBuffer* stack_buffer, const void** out_trace, size_t max_depth) const; @@ -69,18 +69,17 @@ class COMPONENT_EXPORT(TRACING_CPP) StackUnwinderAndroid { // Sends a SIGURG signal to the thread with id |tid| and copies the stack // segment of the thread, along with register context. Returns true on // success. - bool SuspendThreadAndRecordStack( - base::PlatformThreadId tid, - base::StackSampler::StackBuffer* stack_buffer, - uintptr_t* sp, - size_t* stack_size, - unw_context_t* context, - ucontext_t* signal_context) const; + bool SuspendThreadAndRecordStack(base::PlatformThreadId tid, + base::StackBuffer* stack_buffer, + uintptr_t* sp, + size_t* stack_size, + unw_context_t* context, + ucontext_t* signal_context) const; // Replaces any pointers to the old stack to point to the new stack segment. // Returns the jni markers found on stack while scanning stack for pointers. std::vector RewritePointersAndGetMarkers( - base::StackSampler::StackBuffer* stack_buffer, + base::StackBuffer* stack_buffer, uintptr_t sp, size_t stack_size) const; diff --git a/services/tracing/public/cpp/stack_sampling/stack_unwinder_android_unittest.cc b/services/tracing/public/cpp/stack_sampling/stack_unwinder_android_unittest.cc index 1a03dfe71486af..7222fab6d9f372 100644 --- a/services/tracing/public/cpp/stack_sampling/stack_unwinder_android_unittest.cc +++ b/services/tracing/public/cpp/stack_sampling/stack_unwinder_android_unittest.cc @@ -6,6 +6,7 @@ #include "base/android/jni_generator/jni_generator_helper.h" #include "base/bind.h" +#include "base/profiler/stack_buffer.h" #include "base/synchronization/waitable_event.h" #include "base/task/post_task.h" #include "base/test/scoped_task_environment.h"