From e5cab0e926965e329f4172dd2d86b0d7bccdb4a1 Mon Sep 17 00:00:00 2001 From: Leonard Grey Date: Fri, 18 Feb 2022 08:44:15 +0000 Subject: [PATCH] Reland "Reland "Mac: use frame-pointer unwind for macOS 10.14+ and ARM"" This is a reland of 62de5d0245ae02c08093d0c1f32ef894f1b19d22 Previous reland didn't include https://chromium-review.googlesource.com/c/chromium/src/+/3465714 Original change's description: > Reland "Mac: use frame-pointer unwind for macOS 10.14+ and ARM" > > This is a reland of 1fe207f6139d538cfe0055631c02ec658a22847d > > Fix in patchset 2 > > Original change's description: > > Mac: use frame-pointer unwind for macOS 10.14+ and ARM > > > > This change keeps NativeMacUnwinder for now. In a future change > > (possibly after this bakes), we can backport pthread_stack_frame_decode_np > > and remove it. > > > > For convenience, since I renamed the file, here's a separate diff > > of TryUnwind: https://gist.github.com/speednoisemovement/5545f236ebe674360528895d1ae5c0de > > > > Bug: 1101399, 1102197 > > Change-Id: Id2ca3f8e34d5f91d9adc668831a3f1ea7b46b1ad > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3336376 > > Reviewed-by: David Jean > > Reviewed-by: Mike Wittman > > Reviewed-by: Eric Seckler > > Commit-Queue: Leonard Grey > > Cr-Commit-Position: refs/heads/main@{#971214} > > Bug: 1101399, 1102197 > Change-Id: If8446743a5b82cd54ca620f575c4d0ea4efeab42 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3466814 > Reviewed-by: Mike Wittman > Reviewed-by: David Jean > Reviewed-by: Eric Seckler > Commit-Queue: Leonard Grey > Cr-Commit-Position: refs/heads/main@{#972478} Bug: 1101399, 1102197 Change-Id: Ie40d25a9e2ac67cfe5f314b8d2718f4fd1d23c7f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3472124 Auto-Submit: Leonard Grey Reviewed-by: Mike Wittman Reviewed-by: Eric Seckler Commit-Queue: Eric Seckler Cr-Commit-Position: refs/heads/main@{#972861} --- base/BUILD.gn | 9 +- base/profiler/native_unwinder_apple.cc | 92 +++++ base/profiler/native_unwinder_apple.h | 35 ++ .../native_unwinder_apple_unittest.cc | 335 ++++++++++++++++++ base/profiler/native_unwinder_ios.cc | 68 ---- base/profiler/native_unwinder_ios.h | 31 -- base/profiler/native_unwinder_mac.cc | 5 + base/profiler/native_unwinder_mac.h | 3 +- base/profiler/stack_sampler_ios.cc | 6 +- base/profiler/stack_sampler_mac.cc | 4 +- base/profiler/stack_sampling_profiler.cc | 16 +- .../profiler/thread_profiler_browsertest.cc | 32 -- ...rofiler_platform_configuration_unittest.cc | 24 +- chrome/renderer/v8_unwinder_unittest.cc | 3 +- .../tracing_sampler_profiler_unittest.cc | 35 -- 15 files changed, 491 insertions(+), 207 deletions(-) create mode 100644 base/profiler/native_unwinder_apple.cc create mode 100644 base/profiler/native_unwinder_apple.h create mode 100644 base/profiler/native_unwinder_apple_unittest.cc delete mode 100644 base/profiler/native_unwinder_ios.cc delete mode 100644 base/profiler/native_unwinder_ios.h diff --git a/base/BUILD.gn b/base/BUILD.gn index 8243e7ad765290..f5c62c5168e012 100644 --- a/base/BUILD.gn +++ b/base/BUILD.gn @@ -1263,6 +1263,8 @@ mixed_component("base") { "process/process_mac.cc", "process/process_metrics_mac.cc", "profiler/module_cache_mac.cc", + "profiler/native_unwinder_apple.cc", + "profiler/native_unwinder_apple.h", "profiler/native_unwinder_mac.cc", "profiler/native_unwinder_mac.h", "profiler/stack_sampler_mac.cc", @@ -1310,8 +1312,8 @@ mixed_component("base") { if (ios_stack_profiler_enabled) { sources += [ - "profiler/native_unwinder_ios.cc", - "profiler/native_unwinder_ios.h", + "profiler/native_unwinder_apple.cc", + "profiler/native_unwinder_apple.h", "profiler/suspendable_thread_delegate_mac.cc", "profiler/suspendable_thread_delegate_mac.h", ] @@ -3817,6 +3819,9 @@ test("base_unittests") { } sources += [ "files/os_validation_win_unittest.cc" ] } + if (is_apple) { + sources += [ "profiler/native_unwinder_apple_unittest.cc" ] + } if (use_allocator_shim) { sources += [ diff --git a/base/profiler/native_unwinder_apple.cc b/base/profiler/native_unwinder_apple.cc new file mode 100644 index 00000000000000..01716e6c343998 --- /dev/null +++ b/base/profiler/native_unwinder_apple.cc @@ -0,0 +1,92 @@ +// Copyright 2021 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_apple.h" + +#include + +#include "base/check_op.h" +#include "base/notreached.h" +#include "base/numerics/clamped_math.h" +#include "base/profiler/module_cache.h" +#include "base/profiler/native_unwinder.h" +#include "build/build_config.h" + +namespace base { + +NativeUnwinderApple::NativeUnwinderApple() = default; + +bool NativeUnwinderApple::CanUnwindFrom(const Frame& current_frame) const { + return current_frame.module && current_frame.module->IsNative(); +} + +UnwindResult NativeUnwinderApple::TryUnwind(RegisterContext* thread_context, + uintptr_t stack_top, + std::vector* stack) const { + // We expect the frame corresponding to the |thread_context| register state to + // exist within |stack|. + DCHECK_GT(stack->size(), 0u); +#if defined(ARCH_CPU_ARM64) + constexpr uintptr_t align_mask = 0x1; +#elif defined(ARCH_CPU_X86_64) + constexpr uintptr_t align_mask = 0xf; +#endif + + uintptr_t next_frame = RegisterContextFramePointer(thread_context); + uintptr_t frame_lower_bound = RegisterContextStackPointer(thread_context); + const auto is_fp_valid = [&](uintptr_t fp) { + // Ensure there's space on the stack to read two values: the caller's + // frame pointer and the return address. + return next_frame >= frame_lower_bound && + ClampAdd(next_frame, sizeof(uintptr_t) * 2) <= stack_top && + (next_frame & align_mask) == 0; + }; + if (!is_fp_valid(next_frame)) + return UnwindResult::kAborted; + + for (;;) { + if (!stack->back().module) { + return UnwindResult::kAborted; + } + if (!stack->back().module->IsNative()) { + // This is a non-native module associated with the auxiliary unwinder + // (e.g. corresponding to a frame in V8 generated code). Report as + // UNRECOGNIZED_FRAME to allow that unwinder to unwind the frame. + return UnwindResult::kUnrecognizedFrame; + } + uintptr_t retaddr; + uintptr_t frame = next_frame; + next_frame = pthread_stack_frame_decode_np(frame, &retaddr); + frame_lower_bound = frame + 1; + // If `next_frame` is 0, we've hit the root and `retaddr` isn't useful. + // Bail without recording the frame. + if (next_frame == 0) + return UnwindResult::kCompleted; + const ModuleCache::Module* module = + module_cache()->GetModuleForAddress(retaddr); + // V8 doesn't conform to the x86_64 ABI re: stack alignment. For V8 frames, + // let the V8 unwinder determine whether the FP is valid or not. + bool is_non_native_module = module && !module->IsNative(); + // If the FP doesn't look correct, don't record this frame. + if (!is_non_native_module && !is_fp_valid(next_frame)) + return UnwindResult::kAborted; + + RegisterContextFramePointer(thread_context) = next_frame; + RegisterContextInstructionPointer(thread_context) = retaddr; + RegisterContextStackPointer(thread_context) = frame + sizeof(uintptr_t) * 2; + stack->emplace_back(retaddr, module); + } + + NOTREACHED(); + return UnwindResult::kCompleted; +} + +// Mac version is defined in NativeUnwinderMac +#if BUILDFLAG(IS_IOS) +std::unique_ptr CreateNativeUnwinder(ModuleCache* module_cache) { + return std::make_unique(); +} +#endif + +} // namespace base diff --git a/base/profiler/native_unwinder_apple.h b/base/profiler/native_unwinder_apple.h new file mode 100644 index 00000000000000..847371fe1b663a --- /dev/null +++ b/base/profiler/native_unwinder_apple.h @@ -0,0 +1,35 @@ +// Copyright 2021 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_NATIVE_UNWINDER_APPLE_H_ +#define BASE_PROFILER_NATIVE_UNWINDER_APPLE_H_ + +#include + +#include + +#include "base/base_export.h" +#include "base/profiler/unwinder.h" + +namespace base { + +// Native unwinder implementation for iOS, ARM64 and X86_64, and macOS 10.14+. +class BASE_EXPORT API_AVAILABLE(ios(12), macosx(10.14)) NativeUnwinderApple + : public Unwinder { + public: + NativeUnwinderApple(); + + NativeUnwinderApple(const NativeUnwinderApple&) = delete; + NativeUnwinderApple& operator=(const NativeUnwinderApple&) = delete; + + // Unwinder: + bool CanUnwindFrom(const Frame& current_frame) const override; + UnwindResult TryUnwind(RegisterContext* thread_context, + uintptr_t stack_top, + std::vector* stack) const override; +}; + +} // namespace base + +#endif // BASE_PROFILER_NATIVE_UNWINDER_APPLE_H_ diff --git a/base/profiler/native_unwinder_apple_unittest.cc b/base/profiler/native_unwinder_apple_unittest.cc new file mode 100644 index 00000000000000..7df6fb222eb7e3 --- /dev/null +++ b/base/profiler/native_unwinder_apple_unittest.cc @@ -0,0 +1,335 @@ +// Copyright 2022 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_apple.h" + +#include "base/mac/mac_util.h" +#include "base/profiler/module_cache.h" +#include "base/profiler/stack_sampling_profiler_test_util.h" +#include "base/profiler/unwinder.h" +#include "build/buildflag.h" +#include "testing/gtest/include/gtest/gtest.h" + +#include + +namespace base { + +namespace { + +constexpr uintptr_t kModuleStart = 0x1000; +constexpr size_t kModuleSize = 0x1000; +constexpr uintptr_t kNonNativeModuleStart = 0x4000; + +// Used to construct test stacks. If `relative` is true, the value should be the +// address `offset` positions from the bottom of the stack (at 8-byte alignment) +// Otherwise, `offset` is added to the stack as an absolute address/value. +// For example, when creating a stack with bottom 0x2000, {false, 0xf00d} will +// become 0xf00d, and {true, 0x3} will become 0x2018. +struct StackEntrySpec { + bool relative; + uintptr_t offset; +}; + +// Enables constructing a stack buffer that has pointers to itself +// and provides convenience methods for calling the unwinder. +struct InputStack { + explicit InputStack(const std::vector& offsets) + : buffer(offsets.size()) { + size_t size = offsets.size(); + for (size_t i = 0; i < size; ++i) { + auto spec = offsets[i]; + if (spec.relative) { + buffer[i] = bottom() + (spec.offset * sizeof(uintptr_t)); + } else { + buffer[i] = spec.offset; + } + } + } + uintptr_t bottom() const { + return reinterpret_cast(buffer.data()); + } + uintptr_t top() const { return bottom() + buffer.size() * sizeof(uintptr_t); } + + private: + std::vector buffer; +}; + +} // namespace + +class NativeUnwinderAppleTest : public testing::Test { + protected: + NativeUnwinderAppleTest() { + if (__builtin_available(macOS 10.14, iOS 12, *)) { + unwinder_ = std::make_unique(); + + auto test_module = + std::make_unique(kModuleStart, kModuleSize); + module_ = test_module.get(); + module_cache_.AddCustomNativeModule(std::move(test_module)); + auto non_native_module = std::make_unique( + kNonNativeModuleStart, kModuleSize, false); + non_native_module_ = non_native_module.get(); + std::vector> wrapper; + wrapper.push_back(std::move(non_native_module)); + module_cache()->UpdateNonNativeModules({}, std::move(wrapper)); + + unwinder_->Initialize(&module_cache_); + } + } + void SetUp() override { +#if BUILDFLAG(IS_MAC) + if (base::mac::IsAtMostOS10_13()) { + GTEST_SKIP() << "NativeUnwinderApple requires macOS 10.14+"; + } +#endif + } + + ModuleCache* module_cache() { return &module_cache_; } + ModuleCache::Module* module() { return module_; } + ModuleCache::Module* non_native_module() { return non_native_module_; } + Unwinder* unwinder() { return unwinder_.get(); } + + private: + std::unique_ptr unwinder_; + base::ModuleCache module_cache_; + raw_ptr module_; + raw_ptr non_native_module_; +}; + +TEST_F(NativeUnwinderAppleTest, FPPointsOutsideOfStack) { + InputStack input({ + {false, 0x1000}, + {false, 0x1000}, + {false, 0x1000}, + {false, 0x1000}, + {false, 0x1000}, + }); + + RegisterContext context; + RegisterContextStackPointer(&context) = input.bottom(); + RegisterContextInstructionPointer(&context) = kModuleStart; + RegisterContextFramePointer(&context) = 0x1; + std::vector stack = { + Frame(RegisterContextInstructionPointer(&context), module())}; + + EXPECT_EQ(UnwindResult::kAborted, + unwinder()->TryUnwind(&context, input.top(), &stack)); + EXPECT_EQ(std::vector({{kModuleStart, module()}}), stack); + + RegisterContextFramePointer(&context) = input.bottom() - sizeof(uintptr_t); + EXPECT_EQ(UnwindResult::kAborted, + unwinder()->TryUnwind(&context, input.top(), &stack)); + EXPECT_EQ(std::vector({{kModuleStart, module()}}), stack); + + RegisterContextFramePointer(&context) = input.top(); + EXPECT_EQ(UnwindResult::kAborted, + unwinder()->TryUnwind(&context, input.top(), &stack)); + EXPECT_EQ(std::vector({{kModuleStart, module()}}), stack); +} + +TEST_F(NativeUnwinderAppleTest, FPPointsToSelf) { + InputStack input({ + {true, 0}, + {false, kModuleStart + 0x10}, + {true, 4}, + {false, kModuleStart + 0x20}, + {false, 0}, + {false, 0}, + }); + + RegisterContext context; + RegisterContextStackPointer(&context) = input.bottom(); + RegisterContextInstructionPointer(&context) = kModuleStart; + RegisterContextFramePointer(&context) = input.bottom(); + std::vector stack = { + Frame(RegisterContextInstructionPointer(&context), module())}; + + EXPECT_EQ(UnwindResult::kAborted, + unwinder()->TryUnwind(&context, input.top(), &stack)); + EXPECT_EQ(std::vector({ + {kModuleStart, module()}, + }), + stack); +} + +// Tests that two frame pointers that point to each other can't create an +// infinite loop +TEST_F(NativeUnwinderAppleTest, FPCycle) { + InputStack input({ + {true, 2}, + {false, kModuleStart + 0x10}, + {true, 0}, + {false, kModuleStart + 0x20}, + {true, 4}, + {false, kModuleStart + 0x30}, + {false, 0}, + {false, 0}, + }); + + RegisterContext context; + RegisterContextStackPointer(&context) = input.bottom(); + RegisterContextInstructionPointer(&context) = kModuleStart; + RegisterContextFramePointer(&context) = input.bottom(); + std::vector stack = { + Frame(RegisterContextInstructionPointer(&context), module())}; + + EXPECT_EQ(UnwindResult::kAborted, + unwinder()->TryUnwind(&context, input.top(), &stack)); + EXPECT_EQ(std::vector({ + {kModuleStart, module()}, + {kModuleStart + 0x10, module()}, + }), + stack); +} + +TEST_F(NativeUnwinderAppleTest, NoModuleForIP) { + uintptr_t not_in_module = kModuleStart - 0x10; + InputStack input({ + {true, 2}, + {false, not_in_module}, + {true, 4}, + {true, kModuleStart + 0x10}, + {false, 0}, + {false, 0}, + }); + + RegisterContext context; + RegisterContextStackPointer(&context) = input.bottom(); + RegisterContextInstructionPointer(&context) = kModuleStart; + RegisterContextFramePointer(&context) = input.bottom(); + std::vector stack = { + Frame(RegisterContextInstructionPointer(&context), module())}; + + EXPECT_EQ(UnwindResult::kAborted, + unwinder()->TryUnwind(&context, input.top(), &stack)); + EXPECT_EQ( + std::vector({{kModuleStart, module()}, {not_in_module, nullptr}}), + stack); +} + +// Tests that testing that checking if there's space to read two values from the +// stack doesn't overflow. +TEST_F(NativeUnwinderAppleTest, FPAdditionOverflows) { + uintptr_t will_overflow = std::numeric_limits::max() - 1; + InputStack input({ + {true, 2}, + {false, kModuleStart + 0x10}, + {false, 0}, + {false, 0}, + }); + + RegisterContext context; + RegisterContextStackPointer(&context) = input.bottom(); + RegisterContextInstructionPointer(&context) = kModuleStart; + RegisterContextFramePointer(&context) = will_overflow; + std::vector stack = { + Frame(RegisterContextInstructionPointer(&context), module())}; + + EXPECT_EQ(UnwindResult::kAborted, + unwinder()->TryUnwind(&context, input.top(), &stack)); + EXPECT_EQ(std::vector({ + {kModuleStart, module()}, + }), + stack); +} + +// Tests the happy path: a successful unwind with no non-native modules. +TEST_F(NativeUnwinderAppleTest, RegularUnwind) { + InputStack input({ + {true, 4}, // fp of frame 1 + {false, kModuleStart + 0x20}, // ip of frame 1 + {false, 0xaaaa}, + {false, 0xaaaa}, + {true, 8}, // fp of frame 2 + {false, kModuleStart + 0x42}, // ip of frame 2 + {false, 0xaaaa}, + {false, 0xaaaa}, + {false, 0}, + {false, 1}, + }); + + RegisterContext context; + RegisterContextStackPointer(&context) = input.bottom(); + RegisterContextInstructionPointer(&context) = kModuleStart; + RegisterContextFramePointer(&context) = input.bottom(); + std::vector stack = { + Frame(RegisterContextInstructionPointer(&context), module())}; + + EXPECT_EQ(UnwindResult::kCompleted, + unwinder()->TryUnwind(&context, input.top(), &stack)); + EXPECT_EQ(std::vector({ + {kModuleStart, module()}, + {kModuleStart + 0x20, module()}, + {kModuleStart + 0x42, module()}, + }), + stack); +} + +// Tests that if a V8 frame is encountered, unwinding stops and +// kUnrecognizedFrame is returned to facilitate continuing with the V8 unwinder. +TEST_F(NativeUnwinderAppleTest, NonNativeFrame) { + InputStack input({ + {true, 4}, // fp of frame 1 + {false, kModuleStart + 0x20}, // ip of frame 1 + {false, 0xaaaa}, + {false, 0xaaaa}, + {true, 8}, // fp of frame 2 + {false, kNonNativeModuleStart + 0x42}, // ip of frame 2 + {false, 0xaaaa}, + {false, 0xaaaa}, + {true, 12}, // fp of frame 3 + {false, kModuleStart + 0x10}, // ip of frame 3 + {true, 0xaaaa}, + {true, 0xaaaa}, + {false, 0}, + {false, 1}, + }); + + RegisterContext context; + RegisterContextStackPointer(&context) = input.bottom(); + RegisterContextInstructionPointer(&context) = kModuleStart; + RegisterContextFramePointer(&context) = input.bottom(); + std::vector stack = { + Frame(RegisterContextInstructionPointer(&context), module())}; + + EXPECT_EQ(UnwindResult::kUnrecognizedFrame, + unwinder()->TryUnwind(&context, input.top(), &stack)); + EXPECT_EQ(std::vector({ + {kModuleStart, module()}, + {kModuleStart + 0x20, module()}, + {kNonNativeModuleStart + 0x42, non_native_module()}, + }), + stack); +} + +// Tests that a V8 frame with an unaligned frame pointer correctly returns +// kUnrecognizedFrame and not kAborted. +TEST_F(NativeUnwinderAppleTest, NonNativeUnaligned) { + InputStack input({ + {true, 4}, // fp of frame 1 + {false, kModuleStart + 0x20}, // ip of frame 1 + {false, 0xaaaa}, + {false, 0xaaaa}, + {true, 7}, // fp of frame 2 + {false, kNonNativeModuleStart + 0x42}, // ip of frame 2 + {false, 0xaaaa}, + {true, 10}, // fp of frame 3 + {false, kModuleStart + 0x10}, // ip of frame 3 + {true, 0xaaaa}, + {false, 0}, + {false, 1}, + }); + + RegisterContext context; + RegisterContextStackPointer(&context) = input.bottom(); + RegisterContextInstructionPointer(&context) = kModuleStart; + RegisterContextFramePointer(&context) = input.bottom(); + std::vector stack = { + Frame(RegisterContextInstructionPointer(&context), module())}; + + EXPECT_EQ(UnwindResult::kUnrecognizedFrame, + unwinder()->TryUnwind(&context, input.top(), &stack)); +} + +} // namespace base diff --git a/base/profiler/native_unwinder_ios.cc b/base/profiler/native_unwinder_ios.cc deleted file mode 100644 index 429eaef19382e7..00000000000000 --- a/base/profiler/native_unwinder_ios.cc +++ /dev/null @@ -1,68 +0,0 @@ -// Copyright 2021 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_ios.h" - -#include - -#include "base/check_op.h" -#include "base/notreached.h" -#include "base/profiler/module_cache.h" -#include "base/profiler/native_unwinder.h" -#include "build/build_config.h" - -namespace base { - -NativeUnwinderIOS::NativeUnwinderIOS() {} - -bool NativeUnwinderIOS::CanUnwindFrom(const Frame& current_frame) const { - return current_frame.module && current_frame.module->IsNative(); -} - -UnwindResult NativeUnwinderIOS::TryUnwind(RegisterContext* thread_context, - uintptr_t stack_top, - std::vector* stack) const { - // We expect the frame corresponding to the |thread_context| register state to - // exist within |stack|. - DCHECK_GT(stack->size(), 0u); - -#if defined(ARCH_CPU_ARM64) - const uintptr_t align_mask = 0x1; - const uintptr_t stack_bottom = thread_context->__sp; - uintptr_t next_frame = thread_context->__fp; -#elif defined(ARCH_CPU_X86_64) - const uintptr_t align_mask = 0xf; - const uintptr_t stack_bottom = thread_context->__rsp; - uintptr_t next_frame = thread_context->__rbp; -#endif - - const auto is_fp_valid = [&](uintptr_t fp) { - return fp >= stack_bottom && fp < stack_top && (fp & align_mask) == 0; - }; - - if (!is_fp_valid(next_frame)) { - return UnwindResult::kAborted; - } - - for (;;) { - uintptr_t retaddr; - uintptr_t frame = next_frame; - next_frame = pthread_stack_frame_decode_np(frame, &retaddr); - - if (!is_fp_valid(frame) || next_frame <= frame) { - return UnwindResult::kCompleted; - } - - stack->emplace_back(retaddr, module_cache()->GetModuleForAddress(retaddr)); - } - - NOTREACHED(); - return UnwindResult::kCompleted; -} - -std::unique_ptr CreateNativeUnwinder(ModuleCache* module_cache) { - return std::make_unique(); -} - -} // namespace base diff --git a/base/profiler/native_unwinder_ios.h b/base/profiler/native_unwinder_ios.h deleted file mode 100644 index 58bb22c4977586..00000000000000 --- a/base/profiler/native_unwinder_ios.h +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2021 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_NATIVE_UNWINDER_IOS_H_ -#define BASE_PROFILER_NATIVE_UNWINDER_IOS_H_ - -#include - -#include "base/profiler/unwinder.h" - -namespace base { - -// Native unwinder implementation for iOS, ARM64 and X86_64. -class NativeUnwinderIOS : public Unwinder { - public: - NativeUnwinderIOS(); - - NativeUnwinderIOS(const NativeUnwinderIOS&) = delete; - NativeUnwinderIOS& operator=(const NativeUnwinderIOS&) = delete; - - // Unwinder: - bool CanUnwindFrom(const Frame& current_frame) const override; - UnwindResult TryUnwind(RegisterContext* thread_context, - uintptr_t stack_top, - std::vector* stack) const override; -}; - -} // namespace base - -#endif // BASE_PROFILER_NATIVE_UNWINDER_IOS_H_ diff --git a/base/profiler/native_unwinder_mac.cc b/base/profiler/native_unwinder_mac.cc index aabd7280f503ea..1804840cf945ef 100644 --- a/base/profiler/native_unwinder_mac.cc +++ b/base/profiler/native_unwinder_mac.cc @@ -12,9 +12,11 @@ #include #include "base/check_op.h" +#include "base/mac/mac_util.h" #include "base/notreached.h" #include "base/profiler/module_cache.h" #include "base/profiler/native_unwinder.h" +#include "base/profiler/native_unwinder_apple.h" #include "base/profiler/profile_builder.h" #include "build/build_config.h" #include "third_party/abseil-cpp/absl/types/optional.h" @@ -350,6 +352,9 @@ absl::optional NativeUnwinderMac::CheckPostconditions( } std::unique_ptr CreateNativeUnwinder(ModuleCache* module_cache) { + if (__builtin_available(macOS 10.14, *)) { + return std::make_unique(); + } return std::make_unique(module_cache); } diff --git a/base/profiler/native_unwinder_mac.h b/base/profiler/native_unwinder_mac.h index 7d41e40390f9df..b2be6a910383a1 100644 --- a/base/profiler/native_unwinder_mac.h +++ b/base/profiler/native_unwinder_mac.h @@ -13,7 +13,8 @@ namespace base { -// Native unwinder implementation for Mac, using libunwind. +// Native unwinder implementation for Mac, using libunwind. Only used by +// macOS < 10.14; newer versions use NativeUnwinderApple. class NativeUnwinderMac : public Unwinder { public: NativeUnwinderMac(ModuleCache* module_cache); diff --git a/base/profiler/stack_sampler_ios.cc b/base/profiler/stack_sampler_ios.cc index 7727b9fb3149b4..8353177d673f1a 100644 --- a/base/profiler/stack_sampler_ios.cc +++ b/base/profiler/stack_sampler_ios.cc @@ -9,7 +9,7 @@ #if defined(ARCH_CPU_ARM64) || defined(ARCH_CPU_X86_64) #include "base/bind.h" #include "base/check.h" -#include "base/profiler/native_unwinder_ios.h" +#include "base/profiler/native_unwinder_apple.h" #include "base/profiler/stack_copier_suspend.h" #include "base/profiler/stack_sampler_impl.h" #include "base/profiler/suspendable_thread_delegate_mac.h" @@ -22,7 +22,9 @@ namespace { std::vector> CreateUnwinders() { std::vector> unwinders; - unwinders.push_back(std::make_unique()); + if (__builtin_available(iOS 12.0, *)) { + unwinders.push_back(std::make_unique()); + } return unwinders; } diff --git a/base/profiler/stack_sampler_mac.cc b/base/profiler/stack_sampler_mac.cc index f1a7df2eb5eaf6..ef5281bd4f274f 100644 --- a/base/profiler/stack_sampler_mac.cc +++ b/base/profiler/stack_sampler_mac.cc @@ -6,7 +6,7 @@ #include "base/bind.h" #include "base/check.h" -#include "base/profiler/native_unwinder_mac.h" +#include "base/profiler/native_unwinder.h" #include "base/profiler/stack_copier_suspend.h" #include "base/profiler/stack_sampler_impl.h" #include "base/profiler/suspendable_thread_delegate_mac.h" @@ -18,7 +18,7 @@ namespace { std::vector> CreateUnwinders( ModuleCache* module_cache) { std::vector> unwinders; - unwinders.push_back(std::make_unique(module_cache)); + unwinders.push_back(CreateNativeUnwinder(module_cache)); return unwinders; } diff --git a/base/profiler/stack_sampling_profiler.cc b/base/profiler/stack_sampling_profiler.cc index fdd6c659d66262..dc3a13d40839ce 100644 --- a/base/profiler/stack_sampling_profiler.cc +++ b/base/profiler/stack_sampling_profiler.cc @@ -748,20 +748,12 @@ TimeTicks StackSamplingProfiler::TestPeer::GetNextSampleTime( } // static -// The profiler is currently supported for Windows x64, MacOSX x64, and Android -// ARM32. +// The profiler is currently supported for Windows x64, macOS, iOS 64-bit, and +// Android ARM32. bool StackSamplingProfiler::IsSupportedForCurrentPlatform() { -#if (BUILDFLAG(IS_WIN) && defined(ARCH_CPU_X86_64)) || \ - (BUILDFLAG(IS_MAC) && defined(ARCH_CPU_X86_64)) || \ - (BUILDFLAG(IS_IOS) && defined(ARCH_CPU_64_BITS)) || \ +#if (BUILDFLAG(IS_WIN) && defined(ARCH_CPU_X86_64)) || BUILDFLAG(IS_MAC) || \ + (BUILDFLAG(IS_IOS) && defined(ARCH_CPU_64_BITS)) || \ (BUILDFLAG(IS_ANDROID) && BUILDFLAG(ENABLE_ARM_CFI_TABLE)) -#if BUILDFLAG(IS_MAC) - // TODO(https://crbug.com/1098119): Fix unwinding on macOS 11. The OS has - // moved all system libraries into the dyld shared cache and this seems to - // break the sampling profiler. - if (base::mac::IsAtLeastOS11()) - return false; -#endif #if BUILDFLAG(IS_WIN) // Do not start the profiler when Application Verifier is in use; running them // simultaneously can cause crashes and has no known use case. diff --git a/chrome/common/profiler/thread_profiler_browsertest.cc b/chrome/common/profiler/thread_profiler_browsertest.cc index 3c4212646693e3..f213be2b772b93 100644 --- a/chrome/common/profiler/thread_profiler_browsertest.cc +++ b/chrome/common/profiler/thread_profiler_browsertest.cc @@ -125,20 +125,6 @@ class ThreadProfilerBrowserTest : public PlatformBrowserTest { } }; -bool ShouldSkipTestForMacOS11() { -#if BUILDFLAG(IS_MAC) - // Sampling profiler does not work and is disabled on macOS 11. - // See https://crbug.com/1101399 and https://crbug.com/1098119. - // DCHECK that that remains the case so these tests are re-enabled when the - // sampling profiler is re-enabled there. - if (base::mac::IsAtLeastOS11()) { - DCHECK(!base::StackSamplingProfiler::IsSupportedForCurrentPlatform()); - return true; - } -#endif - return false; -} - // Wait for a profile with the specified properties. bool WaitForProfile(metrics::SampledProfile::TriggerEvent trigger_event, metrics::Process process, @@ -173,58 +159,42 @@ bool WaitForProfile(metrics::SampledProfile::TriggerEvent trigger_event, // were dropped as a result of bugs introduced by mojo refactorings. IN_PROC_BROWSER_TEST_F(ThreadProfilerBrowserTest, BrowserProcessMainThread) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11."; EXPECT_TRUE(WaitForProfile(metrics::SampledProfile::PROCESS_STARTUP, metrics::BROWSER_PROCESS, metrics::MAIN_THREAD)); } IN_PROC_BROWSER_TEST_F(ThreadProfilerBrowserTest, BrowserProcessIOThread) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11."; EXPECT_TRUE(WaitForProfile(metrics::SampledProfile::PROCESS_STARTUP, metrics::BROWSER_PROCESS, metrics::IO_THREAD)); } IN_PROC_BROWSER_TEST_F(ThreadProfilerBrowserTest, GpuProcessMainThread) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11."; EXPECT_TRUE(WaitForProfile(metrics::SampledProfile::PROCESS_STARTUP, metrics::GPU_PROCESS, metrics::MAIN_THREAD)); } IN_PROC_BROWSER_TEST_F(ThreadProfilerBrowserTest, GpuProcessIOThread) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11."; EXPECT_TRUE(WaitForProfile(metrics::SampledProfile::PROCESS_STARTUP, metrics::GPU_PROCESS, metrics::IO_THREAD)); } IN_PROC_BROWSER_TEST_F(ThreadProfilerBrowserTest, GpuProcessCompositorThread) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11."; EXPECT_TRUE(WaitForProfile(metrics::SampledProfile::PROCESS_STARTUP, metrics::GPU_PROCESS, metrics::COMPOSITOR_THREAD)); } IN_PROC_BROWSER_TEST_F(ThreadProfilerBrowserTest, RendererProcessMainThread) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11."; EXPECT_TRUE(WaitForProfile(metrics::SampledProfile::PROCESS_STARTUP, metrics::RENDERER_PROCESS, metrics::MAIN_THREAD)); } IN_PROC_BROWSER_TEST_F(ThreadProfilerBrowserTest, RendererProcessIOThread) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11."; EXPECT_TRUE(WaitForProfile(metrics::SampledProfile::PROCESS_STARTUP, metrics::RENDERER_PROCESS, metrics::IO_THREAD)); } IN_PROC_BROWSER_TEST_F(ThreadProfilerBrowserTest, RendererProcessCompositorThread) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11."; EXPECT_TRUE(WaitForProfile(metrics::SampledProfile::PROCESS_STARTUP, metrics::RENDERER_PROCESS, metrics::COMPOSITOR_THREAD)); @@ -239,8 +209,6 @@ IN_PROC_BROWSER_TEST_F(ThreadProfilerBrowserTest, #endif IN_PROC_BROWSER_TEST_F(ThreadProfilerBrowserTest, MAYBE_NetworkServiceProcessIOThread) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11."; EXPECT_TRUE(WaitForProfile(metrics::SampledProfile::PROCESS_STARTUP, metrics::NETWORK_SERVICE_PROCESS, metrics::IO_THREAD)); diff --git a/chrome/common/profiler/thread_profiler_platform_configuration_unittest.cc b/chrome/common/profiler/thread_profiler_platform_configuration_unittest.cc index df7abdfd750131..1520e8d370e94a 100644 --- a/chrome/common/profiler/thread_profiler_platform_configuration_unittest.cc +++ b/chrome/common/profiler/thread_profiler_platform_configuration_unittest.cc @@ -12,12 +12,7 @@ #include "components/version_info/version_info.h" #include "testing/gtest/include/gtest/gtest.h" -#if BUILDFLAG(IS_MAC) -#include "base/mac/mac_util.h" -#endif - -#if (BUILDFLAG(IS_WIN) && defined(ARCH_CPU_X86_64)) || \ - (BUILDFLAG(IS_MAC) && defined(ARCH_CPU_X86_64)) || \ +#if (BUILDFLAG(IS_WIN) && defined(ARCH_CPU_X86_64)) || BUILDFLAG(IS_MAC) || \ (BUILDFLAG(IS_ANDROID) && BUILDFLAG(ENABLE_ARM_CFI_TABLE)) #define THREAD_PROFILER_SUPPORTED_ON_PLATFORM true #else @@ -83,24 +78,13 @@ TEST_F(ThreadProfilerPlatformConfigurationTest, IsSupported) { EXPECT_FALSE(config()->IsSupported(absl::nullopt)); #else -#if BUILDFLAG(IS_MAC) - // Sampling profiler does not work on macOS 11.0 yet: - // https://crbug.com/1101399 - const bool on_canary = base::mac::IsAtMostOS10_15(); - const bool on_dev = base::mac::IsAtMostOS10_15(); - const bool on_default = base::mac::IsAtMostOS10_15(); -#else - const bool on_canary = true; - const bool on_dev = true; - const bool on_default = true; -#endif EXPECT_FALSE(config()->IsSupported(version_info::Channel::UNKNOWN)); - EXPECT_EQ(on_canary, config()->IsSupported(version_info::Channel::CANARY)); - EXPECT_EQ(on_dev, config()->IsSupported(version_info::Channel::DEV)); + EXPECT_TRUE(config()->IsSupported(version_info::Channel::CANARY)); + EXPECT_TRUE(config()->IsSupported(version_info::Channel::DEV)); EXPECT_FALSE(config()->IsSupported(version_info::Channel::BETA)); EXPECT_FALSE(config()->IsSupported(version_info::Channel::STABLE)); - EXPECT_EQ(on_default, config()->IsSupported(absl::nullopt)); + EXPECT_TRUE(config()->IsSupported(absl::nullopt)); #endif } diff --git a/chrome/renderer/v8_unwinder_unittest.cc b/chrome/renderer/v8_unwinder_unittest.cc index f16a934170fb11..e9dab3cf2063b0 100644 --- a/chrome/renderer/v8_unwinder_unittest.cc +++ b/chrome/renderer/v8_unwinder_unittest.cc @@ -533,8 +533,7 @@ TEST(V8UnwinderTest, CanUnwindFrom_NullModule) { // Checks that unwinding from C++ through JavaScript and back into C++ succeeds. // NB: unwinding is only supported for 64 bit Windows and Intel macOS. -#if (BUILDFLAG(IS_WIN) && defined(ARCH_CPU_64_BITS)) || \ - (BUILDFLAG(IS_MAC) && defined(ARCH_CPU_X86_64)) +#if (BUILDFLAG(IS_WIN) && defined(ARCH_CPU_64_BITS)) || BUILDFLAG(IS_MAC) #define MAYBE_UnwindThroughV8Frames UnwindThroughV8Frames #else #define MAYBE_UnwindThroughV8Frames DISABLED_UnwindThroughV8Frames diff --git a/services/tracing/public/cpp/stack_sampling/tracing_sampler_profiler_unittest.cc b/services/tracing/public/cpp/stack_sampling/tracing_sampler_profiler_unittest.cc index 50dbae134e4295..8885ef4878cf2a 100644 --- a/services/tracing/public/cpp/stack_sampling/tracing_sampler_profiler_unittest.cc +++ b/services/tracing/public/cpp/stack_sampling/tracing_sampler_profiler_unittest.cc @@ -168,25 +168,9 @@ class TracingSampleProfilerTest : public TracingUnitTest { #endif }; -bool ShouldSkipTestForMacOS11() { -#if BUILDFLAG(IS_MAC) - // The sampling profiler does not work on macOS 11 and is disabled. - // See https://crbug.com/1101399 and https://crbug.com/1098119. - // DCHECK here so that when the sampling profiler is re-enabled on macOS 11, - // these tests are also re-enabled. - if (base::mac::IsAtLeastOS11()) { - DCHECK(!base::StackSamplingProfiler::IsSupportedForCurrentPlatform()); - return true; - } -#endif - return false; -} - } // namespace TEST_F(TracingSampleProfilerTest, OnSampleCompleted) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11"; auto profiler = TracingSamplerProfiler::CreateOnMainThread(); BeginTrace(); base::RunLoop().RunUntilIdle(); @@ -197,8 +181,6 @@ TEST_F(TracingSampleProfilerTest, OnSampleCompleted) { } TEST_F(TracingSampleProfilerTest, JoinRunningTracing) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11"; BeginTrace(); auto profiler = TracingSamplerProfiler::CreateOnMainThread(); base::RunLoop().RunUntilIdle(); @@ -209,8 +191,6 @@ TEST_F(TracingSampleProfilerTest, JoinRunningTracing) { } TEST_F(TracingSampleProfilerTest, TestStartupTracing) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11"; auto profiler = TracingSamplerProfiler::CreateOnMainThread(); TracingSamplerProfiler::SetupStartupTracingForTesting(); base::RunLoop().RunUntilIdle(); @@ -245,8 +225,6 @@ TEST_F(TracingSampleProfilerTest, TestStartupTracing) { } TEST_F(TracingSampleProfilerTest, JoinStartupTracing) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11"; TracingSamplerProfiler::SetupStartupTracingForTesting(); base::RunLoop().RunUntilIdle(); auto profiler = TracingSamplerProfiler::CreateOnMainThread(); @@ -281,8 +259,6 @@ TEST_F(TracingSampleProfilerTest, JoinStartupTracing) { } TEST_F(TracingSampleProfilerTest, SamplingChildThread) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11"; base::Thread sampled_thread("sampling_profiler_test"); sampled_thread.Start(); sampled_thread.task_runner()->PostTask( @@ -301,8 +277,6 @@ TEST_F(TracingSampleProfilerTest, SamplingChildThread) { #if BUILDFLAG(ENABLE_LOADER_LOCK_SAMPLING) TEST_F(TracingSampleProfilerTest, SampleLoaderLockOnMainThread) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11"; LoaderLockEventAnalyzer event_analyzer; bool lock_held = false; @@ -328,8 +302,6 @@ TEST_F(TracingSampleProfilerTest, SampleLoaderLockOnMainThread) { } TEST_F(TracingSampleProfilerTest, SampleLoaderLockAlwaysHeld) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11"; LoaderLockEventAnalyzer event_analyzer; EXPECT_CALL(mock_loader_lock_sampler_, IsLoaderLockHeld()) @@ -348,8 +320,6 @@ TEST_F(TracingSampleProfilerTest, SampleLoaderLockAlwaysHeld) { } TEST_F(TracingSampleProfilerTest, SampleLoaderLockNeverHeld) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11"; LoaderLockEventAnalyzer event_analyzer; EXPECT_CALL(mock_loader_lock_sampler_, IsLoaderLockHeld()) @@ -367,8 +337,6 @@ TEST_F(TracingSampleProfilerTest, SampleLoaderLockNeverHeld) { } TEST_F(TracingSampleProfilerTest, SampleLoaderLockOnChildThread) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11"; LoaderLockEventAnalyzer event_analyzer; // Loader lock should only be sampled on main thread. @@ -391,9 +359,6 @@ TEST_F(TracingSampleProfilerTest, SampleLoaderLockOnChildThread) { } TEST_F(TracingSampleProfilerTest, SampleLoaderLockWithoutMock) { - if (ShouldSkipTestForMacOS11()) - GTEST_SKIP() << "Stack sampler is not supported on macOS 11"; - // Use the real loader lock sampler. This tests that it is initialized // correctly in TracingSamplerProfiler. LoaderLockSamplingThread::SetLoaderLockSamplerForTesting(nullptr);