[Crash-utility] Re: [PATCH v1 2/3] ppc64: check sp at the start of stack back trace
Hi Tao,
On 18/09/24 05:12, Tao Liu wrote:
Previously the newsp read from stack is unchecked, it would expect
the newsp hold a valid sp pointer of the next stack frame by default,
which is not always true, see the following stack:
R1: c00000000a327660 NIP: c0000000002b9984
#0 [c00000000a327660] crash_setup_regs at c0000000002b9984
c00000000a327660: c00000000015a7b0 c000000001bc8600
c00000000a327670: 0000000000000000 c000000002e13b54
c00000000a327680: c00000000a327850 0000000000004000
c00000000a327690: c0000000002ba078 c0000000026f2ca8
The newsp will be c00000000015a7b0, which is not a valid stack pointer
and as a result it will fail the stack back trace afterwards. This usually
happens at the beginning of back trace, once we get the correct sp value,
the stack can be back traced successfully.
Seems weird that this issue occurred in the first place. The stack frame
should have a valid previous sp at 0th offset to sp, according to the
PowerISA document.
Anyways this patch is also good, doesn't seem problematic for the
previous working cases.
Thanks,
Aditya Gupta
> This patch will search and check for the valid newsp at the beginning of
> stack back trace. For the above example, a valid newsp would be assigned
> as c00000000a327850.
>
> Before:
> crash> bt
> ...
> #0 [c00000000a327660] crash_setup_regs at c0000000002b9984
> After:
> crash> bt
> ...
> #0 [c00000000a327660] crash_setup_regs at c0000000002b9984
> #1 [c00000000a327850] panic at c000000000167de4
> #2 [c00000000a3278f0] sysrq_handle_crash at c000000000a4d3a8
> #3 [c00000000a327950] __handle_sysrq at c000000000a4dec4
> #4 [c00000000a3279f0] write_sysrq_trigger at c000000000a4e984
> ...
> Signed-off-by: Tao Liu <ltao(a)redhat.com>
> ---
> ppc64.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/ppc64.c b/ppc64.c
> index 6e5f155..8af7b8a 100644
> --- a/ppc64.c
> +++ b/ppc64.c
> @@ -2148,6 +2148,19 @@ ppc64_back_trace(struct gnu_request *req, struct bt_info *bt)
>
> while (INSTACK(req->sp, bt)) {
> newsp = *(ulong *)&bt->stackbuf[req->sp - bt->stackbase];
> +
> + if (!newpc) {
> + ulong *up;
> + int i;
> + for (i = 0, up = (ulong *)&bt->stackbuf[req->sp - bt->stackbase];
> + i < (req->sp - bt->stackbase) / sizeof(ulong); i++, up++) {
> + if (INSTACK(*up, bt) && is_kernel_text(*(up + 2))) {
> + newsp = *up;
> + break;
> + }
> + }
> + }
> +
> if ((req->name = closest_symbol(req->pc)) == NULL) {
> if (CRASHDEBUG(1)) {
> error(FATAL,