Re: [PATCH 1/2] arm64: Implement arch_stack_walk_reliable

From: Weinan Liu
Date: Wed Mar 19 2025 - 14:38:52 EST


On Tue, Mar 18, 2025 at 10:39 PM Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
>
> On Tue, Mar 18, 2025 at 08:58:52PM -0700, Song Liu wrote:
> > On a closer look, I think we also need some logic in unwind_find_stack()
> > so that we can see when the unwinder hits the exception boundary. For
> > this reason, we may still need unwind_state.unreliable. I will look into
> > fixing this and send v2.
>
> Isn't that what FRAME_META_TYPE_PT_REGS is for?
>
> Maybe it can just tell kunwind_stack_walk() to set a bit in
> kunwind_state which tells kunwind_next_frame_record_meta() to quit the
> unwind early for the FRAME_META_TYPE_PT_REGS case.  That also has the
> benefit of stopping the unwind as soon as the exception is encounterd.
>

After reviewing the code flow, it seems like we should treat all -EINVALID
cases or `FRAME_META_TYPE_PT_REGS` cases as unreliable unwinds.

Would a simplification like the one below work?
Or we can return a special value for success cases in kunwind_next_regs_pc()

```
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index 69d0567a0c38..0eb69fa6161a 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -296,7 +296,8 @@ do_kunwind(struct kunwind_state *state, kunwind_consume_fn consume_state,
if (!consume_state(state, cookie))
break;
ret = kunwind_next(state);
- if (ret < 0)
+ if (ret < 0 || state->source == KUNWIND_SOURCE_REGS_PC)
+ state->common.unreliable = true;
break;
}
}
```

--
Weinan