From 7a8d20b7841b43cc685230058130bd0f019e4004 Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Wed, 8 Jun 2022 12:42:23 -0700 Subject: absl: correct the stack trace path on RISCV When we would perform a stacktrace using the frame pointer walking, because we did the adjustment for the return address separately, we were misaligning the stack size and frame. Simplify the logic and correct the offset. The recovered frame pointer provides us with the return address of the current frame and the previous frame's frame pointer. Subsequently, we decide if we want to record this frame or not. The value in `next_frame_pointer` already points to the value from the previous stack frame (that is the next frame pointer to iterate). As such, the value computed by `ComputeStackFrameSize` is the value for the current frame. This was offset by one previously. Take the opportunity to clean up some of the local comments, fixing typos and splitting up the comments to reflect the lines that they are associated with. PiperOrigin-RevId: 453744059 Change-Id: If14813e0ac36f327f4b7594472f2222d05c478aa --- absl/debugging/internal/stacktrace_riscv-inl.inc | 25 +++++++++++------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/absl/debugging/internal/stacktrace_riscv-inl.inc b/absl/debugging/internal/stacktrace_riscv-inl.inc index 98b09a22..7123b71b 100644 --- a/absl/debugging/internal/stacktrace_riscv-inl.inc +++ b/absl/debugging/internal/stacktrace_riscv-inl.inc @@ -171,26 +171,21 @@ ABSL_ATTRIBUTE_NO_SANITIZE_ADDRESS // May read random elements from stack. ABSL_ATTRIBUTE_NO_SANITIZE_MEMORY // May read random elements from stack. static int UnwindImpl(void **result, int *sizes, int max_depth, int skip_count, const void *ucp, int *min_dropped_frames) { + // The `frame_pointer` that is computed here points to the top of the frame. + // The two words preceding the address are the return address and the previous + // frame pointer. #if defined(__GNUC__) void **frame_pointer = reinterpret_cast(__builtin_frame_address(0)); #else #error reading stack pointer not yet supported on this platform #endif - skip_count++; // Skip the frame for this function. int n = 0; - - // The `frame_pointer` that is computed here points to the top of the frame. - // The two words preceding the address are the return address and the previous - // frame pointer. To find a PC value associated with the current frame, we - // need to go down a level in the call chain. So we remember the return - // address of the last frame seen. This does not work for the first stack - // frame, which belongs to `UnwindImp()` but we skip the frame for - // `UnwindImp()` anyway. - void *prev_return_address = nullptr; - + void *return_address = nullptr; while (frame_pointer && n < max_depth) { - // The absl::GetStackFrames routine si called when we are in some + return_address = frame_pointer[-1]; + + // The absl::GetStackFrames routine is called when we are in some // informational context (the failure signal handler for example). Use the // non-strict unwinding rules to produce a stack trace that is as complete // as possible (even if it contains a few bogus entries in some rare cases). @@ -200,15 +195,16 @@ static int UnwindImpl(void **result, int *sizes, int max_depth, int skip_count, if (skip_count > 0) { skip_count--; } else { - result[n] = prev_return_address; + result[n] = return_address; if (IS_STACK_FRAMES) { sizes[n] = ComputeStackFrameSize(frame_pointer, next_frame_pointer); } n++; } - prev_return_address = frame_pointer[-1]; + frame_pointer = next_frame_pointer; } + if (min_dropped_frames != nullptr) { // Implementation detail: we clamp the max of frames we are willing to // count, so as not to spend too much time in the loop below. @@ -225,6 +221,7 @@ static int UnwindImpl(void **result, int *sizes, int max_depth, int skip_count, } *min_dropped_frames = num_dropped_frames; } + return n; } -- cgit v1.2.3