Deduplicate stack trace implementations in stacktrace.cc

PiperOrigin-RevId: 786730361
Change-Id: I8184cd8a259570ff247077d7e3adaf430bd08ecc

Author: Abseil Team

absl/debugging/stacktrace.cc

@@ -97,54 +97,60 @@ std::atomic<Unwinder> custom;
 
 template <bool IS_STACK_FRAMES, bool IS_WITH_CONTEXT>
 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<uintptr_t*>(alloca(max_depth * sizeof(*frames)));
+      }
+      if (sizes == nullptr) {
+        sizes = static_cast<int*>(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<size_t>((*g)(result, sizes, static_cast<int>(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<IS_STACK_FRAMES, IS_WITH_CONTEXT>(
-        result, frames, sizes, max_depth, skip_count, uc, min_dropped_frames);
+    size = static_cast<size_t>(
+        unwind_with_fixup
+            ? UnwindImpl<true, IS_WITH_CONTEXT>(
+                  result, frames, sizes, static_cast<int>(max_depth),
+                  skip_count, uc, min_dropped_frames)
+            : UnwindImpl<IS_STACK_FRAMES, IS_WITH_CONTEXT>(
+                  result, frames, sizes, static_cast<int>(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<int>(size);
 }
 
 }  // anonymous namespace
 
 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<size_t>(max_depth);
-      if (frames == nullptr) {
-        frames = static_cast<uintptr_t*>(alloca(nmax * sizeof(*frames)));
-      }
-      if (sizes == nullptr) {
-        sizes = static_cast<int*>(alloca(nmax * sizeof(*sizes)));
-      }
-    }
-    size_t depth = static_cast<size_t>(Unwind<true, true>(
-        result, frames, sizes, max_depth, skip_count, nullptr, nullptr));
-    internal_stacktrace::FixUpStack(result, frames, sizes,
-                                    static_cast<size_t>(max_depth), depth);
-    return static_cast<int>(depth);
-  }
-
-  return Unwind<true, false>(result, frames, sizes, max_depth, skip_count,
+  return Unwind<true, false>(result, frames, sizes,
+                             static_cast<size_t>(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<size_t>(max_depth);
-      if (frames == nullptr) {
-        frames = static_cast<uintptr_t*>(alloca(nmax * sizeof(*frames)));
-      }
-      if (sizes == nullptr) {
-        sizes = static_cast<int*>(alloca(nmax * sizeof(*sizes)));
-      }
-    }
-    size_t depth = static_cast<size_t>(Unwind<true, true>(
-        result, frames, sizes, max_depth, skip_count, uc, min_dropped_frames));
-    internal_stacktrace::FixUpStack(result, frames, sizes,
-                                    static_cast<size_t>(max_depth), depth);
-    return static_cast<int>(depth);
-  }
-
-  return Unwind<true, true>(result, frames, sizes, max_depth, skip_count, uc,
+  return Unwind<true, true>(result, frames, sizes,
+                            static_cast<size_t>(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<size_t>(max_depth);
-      uintptr_t* frames =
-          static_cast<uintptr_t*>(alloca(nmax * sizeof(*frames)));
-      int* sizes = static_cast<int*>(alloca(nmax * sizeof(*sizes)));
-      size_t depth = static_cast<size_t>(Unwind<true, false>(
-          result, frames, sizes, max_depth, skip_count, nullptr, nullptr));
-      internal_stacktrace::FixUpStack(result, frames, sizes, nmax, depth);
-      return static_cast<int>(depth);
-    }
-  }
-
-  return Unwind<false, false>(result, nullptr, nullptr, max_depth, skip_count,
+  return Unwind<false, false>(result, nullptr, nullptr,
+                              static_cast<size_t>(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<size_t>(max_depth);
-      uintptr_t* frames =
-          static_cast<uintptr_t*>(alloca(nmax * sizeof(*frames)));
-      int* sizes = static_cast<int*>(alloca(nmax * sizeof(*sizes)));
-      size_t depth = static_cast<size_t>(
-          Unwind<true, true>(result, frames, sizes, max_depth, skip_count, uc,
-                             min_dropped_frames));
-      internal_stacktrace::FixUpStack(result, frames, sizes, nmax, depth);
-      return static_cast<int>(depth);
-    }
-  }
-
-  return Unwind<false, true>(result, nullptr, nullptr, max_depth, skip_count,
-                             uc, min_dropped_frames);
+  return Unwind<false, true>(result, nullptr, nullptr,
+                             static_cast<size_t>(max_depth), skip_count, uc,
+                             min_dropped_frames);
 }
 
 void SetStackUnwinder(Unwinder w) {

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;