diff --git a/absl/debugging/stacktrace.cc b/absl/debugging/stacktrace.cc index 30d032a4..ccecff76 100644 --- a/absl/debugging/stacktrace.cc +++ b/absl/debugging/stacktrace.cc @@ -97,25 +97,51 @@ std::atomic custom; template ABSL_ATTRIBUTE_ALWAYS_INLINE inline int Unwind(void** result, uintptr_t* frames, - int* sizes, int max_depth, + int* sizes, size_t max_depth, int skip_count, const void* uc, int* min_dropped_frames) { + bool unwind_with_fixup = internal_stacktrace::ShouldFixUpStack(); + if (unwind_with_fixup) { + if constexpr (kHaveAlloca) { + // Some implementations of FixUpStack may need to be passed frame + // information from Unwind, even if the caller doesn't need that + // information. We allocate the necessary buffers for such implementations + // here. + if (frames == nullptr) { + frames = static_cast(alloca(max_depth * sizeof(*frames))); + } + if (sizes == nullptr) { + sizes = static_cast(alloca(max_depth * sizeof(*sizes))); + } + } + } + Unwinder g = custom.load(std::memory_order_acquire); - int size; + size_t size; // Add 1 to skip count for the unwinder function itself ++skip_count; if (g != nullptr) { - size = (*g)(result, sizes, max_depth, skip_count, uc, min_dropped_frames); + size = static_cast((*g)(result, sizes, static_cast(max_depth), + skip_count, uc, min_dropped_frames)); // Frame pointers aren't returned by existing hooks, so clear them. if (frames != nullptr) { std::fill(frames, frames + size, uintptr_t()); } } else { - size = UnwindImpl( - result, frames, sizes, max_depth, skip_count, uc, min_dropped_frames); + size = static_cast( + unwind_with_fixup + ? UnwindImpl( + result, frames, sizes, static_cast(max_depth), + skip_count, uc, min_dropped_frames) + : UnwindImpl( + result, frames, sizes, static_cast(max_depth), + skip_count, uc, min_dropped_frames)); + } + if (unwind_with_fixup) { + internal_stacktrace::FixUpStack(result, frames, sizes, max_depth, size); } ABSL_BLOCK_TAIL_CALL_OPTIMIZATION(); - return size; + return static_cast(size); } } // anonymous namespace @@ -123,28 +149,8 @@ ABSL_ATTRIBUTE_ALWAYS_INLINE inline int Unwind(void** result, uintptr_t* frames, ABSL_ATTRIBUTE_NOINLINE ABSL_ATTRIBUTE_NO_TAIL_CALL int internal_stacktrace::GetStackFrames(void** result, uintptr_t* frames, int* sizes, int max_depth, int skip_count) { - if (internal_stacktrace::ShouldFixUpStack()) { - if constexpr (kHaveAlloca) { - // Some implementations of FixUpStack may need to be passed frame - // information from Unwind, even if the caller doesn't need that - // information. We allocate the necessary buffers for such implementations - // here. - const size_t nmax = static_cast(max_depth); - if (frames == nullptr) { - frames = static_cast(alloca(nmax * sizeof(*frames))); - } - if (sizes == nullptr) { - sizes = static_cast(alloca(nmax * sizeof(*sizes))); - } - } - size_t depth = static_cast(Unwind( - result, frames, sizes, max_depth, skip_count, nullptr, nullptr)); - internal_stacktrace::FixUpStack(result, frames, sizes, - static_cast(max_depth), depth); - return static_cast(depth); - } - - return Unwind(result, frames, sizes, max_depth, skip_count, + return Unwind(result, frames, sizes, + static_cast(max_depth), skip_count, nullptr, nullptr); } @@ -153,69 +159,24 @@ internal_stacktrace::GetStackFramesWithContext(void** result, uintptr_t* frames, int* sizes, int max_depth, int skip_count, const void* uc, int* min_dropped_frames) { - if (internal_stacktrace::ShouldFixUpStack()) { - if constexpr (kHaveAlloca) { - // Some implementations of FixUpStack may need to be passed frame - // information from Unwind, even if the caller doesn't need that - // information. We allocate the necessary buffers for such implementations - // here. - const size_t nmax = static_cast(max_depth); - if (frames == nullptr) { - frames = static_cast(alloca(nmax * sizeof(*frames))); - } - if (sizes == nullptr) { - sizes = static_cast(alloca(nmax * sizeof(*sizes))); - } - } - size_t depth = static_cast(Unwind( - result, frames, sizes, max_depth, skip_count, uc, min_dropped_frames)); - internal_stacktrace::FixUpStack(result, frames, sizes, - static_cast(max_depth), depth); - return static_cast(depth); - } - - return Unwind(result, frames, sizes, max_depth, skip_count, uc, + return Unwind(result, frames, sizes, + static_cast(max_depth), skip_count, uc, min_dropped_frames); } ABSL_ATTRIBUTE_NOINLINE ABSL_ATTRIBUTE_NO_TAIL_CALL int GetStackTrace( void** result, int max_depth, int skip_count) { - if (internal_stacktrace::ShouldFixUpStack()) { - if constexpr (kHaveAlloca) { - const size_t nmax = static_cast(max_depth); - uintptr_t* frames = - static_cast(alloca(nmax * sizeof(*frames))); - int* sizes = static_cast(alloca(nmax * sizeof(*sizes))); - size_t depth = static_cast(Unwind( - result, frames, sizes, max_depth, skip_count, nullptr, nullptr)); - internal_stacktrace::FixUpStack(result, frames, sizes, nmax, depth); - return static_cast(depth); - } - } - - return Unwind(result, nullptr, nullptr, max_depth, skip_count, + return Unwind(result, nullptr, nullptr, + static_cast(max_depth), skip_count, nullptr, nullptr); } ABSL_ATTRIBUTE_NOINLINE ABSL_ATTRIBUTE_NO_TAIL_CALL int GetStackTraceWithContext(void** result, int max_depth, int skip_count, const void* uc, int* min_dropped_frames) { - if (internal_stacktrace::ShouldFixUpStack()) { - if constexpr (kHaveAlloca) { - const size_t nmax = static_cast(max_depth); - uintptr_t* frames = - static_cast(alloca(nmax * sizeof(*frames))); - int* sizes = static_cast(alloca(nmax * sizeof(*sizes))); - size_t depth = static_cast( - Unwind(result, frames, sizes, max_depth, skip_count, uc, - min_dropped_frames)); - internal_stacktrace::FixUpStack(result, frames, sizes, nmax, depth); - return static_cast(depth); - } - } - - return Unwind(result, nullptr, nullptr, max_depth, skip_count, - uc, min_dropped_frames); + return Unwind(result, nullptr, nullptr, + static_cast(max_depth), skip_count, uc, + min_dropped_frames); } void SetStackUnwinder(Unwinder w) { diff --git a/absl/debugging/stacktrace_test.cc b/absl/debugging/stacktrace_test.cc index e5565c1f..16757e72 100644 --- a/absl/debugging/stacktrace_test.cc +++ b/absl/debugging/stacktrace_test.cc @@ -218,6 +218,59 @@ ABSL_ATTRIBUTE_NOINLINE static void FixupNoFixupEquivalenceNoInline() { TEST(StackTrace, FixupNoFixupEquivalence) { FixupNoFixupEquivalenceNoInline(); } +TEST(StackTrace, CustomUnwinderPerformsFixup) { +#if ABSL_HAVE_ATTRIBUTE_WEAK + // This test is known not to pass on MSVC (due to weak symbols) + + constexpr int kSkip = 1; // Skip our own frame, whose return PCs won't match + constexpr auto kStackCount = 1; + + absl::SetStackUnwinder(absl::DefaultStackUnwinder); + const Cleanup restore_state([enable_fixup = g_enable_fixup, + fixup_calls = g_fixup_calls, + should_fixup_calls = g_should_fixup_calls]() { + absl::SetStackUnwinder(nullptr); + g_enable_fixup = enable_fixup; + g_fixup_calls = fixup_calls; + g_should_fixup_calls = should_fixup_calls; + }); + + StackTrace trace; + + g_enable_fixup = true; + g_should_fixup_calls = 0; + g_fixup_calls = 0; + absl::GetStackTrace(trace.result, kSkip, kStackCount); + EXPECT_GT(g_should_fixup_calls, 0); + EXPECT_GT(g_fixup_calls, 0); + + g_enable_fixup = true; + g_should_fixup_calls = 0; + g_fixup_calls = 0; + absl::GetStackFrames(trace.result, trace.sizes, kSkip, kStackCount); + EXPECT_GT(g_should_fixup_calls, 0); + EXPECT_GT(g_fixup_calls, 0); + + g_enable_fixup = true; + g_should_fixup_calls = 0; + g_fixup_calls = 0; + absl::GetStackTraceWithContext(trace.result, kSkip, kStackCount, nullptr, + nullptr); + EXPECT_GT(g_should_fixup_calls, 0); + EXPECT_GT(g_fixup_calls, 0); + + g_enable_fixup = true; + g_should_fixup_calls = 0; + g_fixup_calls = 0; + absl::GetStackFramesWithContext(trace.result, trace.sizes, kSkip, kStackCount, + nullptr, nullptr); + EXPECT_GT(g_should_fixup_calls, 0); + EXPECT_GT(g_fixup_calls, 0); +#else + GTEST_SKIP() << "Need weak symbol support"; +#endif +} + #if ABSL_HAVE_BUILTIN(__builtin_frame_address) struct FrameInfo { const void* return_address;