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
diff --git a/absl/debugging/internal/stacktrace_riscv-inl.inc b/absl/debugging/internal/stacktrace_riscv-inl.inc index 98b09a2..7123b71 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_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<void **>(__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 @@ 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 @@ } *min_dropped_frames = num_dropped_frames; } + return n; }