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

1 files 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<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 @@ 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;
 }