> Date: Mon, 21 Feb 2022 17:34:14 +0000
> From: Visa Hankala <[email protected]>
>
> On riscv64, ddb's stack unwinder performs poorly. The main problem is
> that the exception handlers use a frame structure (trapframe) that
> differs from the typical call frame.
>
> The following patch does several adjustments, including:
>
> * Detect and handle exception frames. (Alternatively, the relevant part
> of struct trapframe could be adjusted to match with callframe, as has
> been done on amd64 and i386.)
>
> * Don't offset ra with -4 when printing function addresses. It is safer
> to print the original address and let the user decipher the meaning,
> in part because the fixed offset is wrong with compressed instructions.
>
> * Stop if frame pointer is badly misaligned.
>
> * Print the frame address where unwinding stopped.
>
> OK?
looks reasonable to me
ok kettenis@
> Index: arch/riscv64/riscv64/db_trace.c
> ===================================================================
> RCS file: src/sys/arch/riscv64/riscv64/db_trace.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 db_trace.c
> --- arch/riscv64/riscv64/db_trace.c 9 Jul 2021 20:59:51 -0000 1.4
> +++ arch/riscv64/riscv64/db_trace.c 21 Feb 2022 17:22:43 -0000
> @@ -43,6 +43,9 @@
> #include <ddb/db_sym.h>
> #include <ddb/db_output.h>
>
> +extern unsigned char cpu_exception_handler_supervisor[];
> +extern unsigned char cpu_exception_handler_user[];
> +
> db_regs_t ddb_regs;
>
> #define INKERNEL(va) (((vaddr_t)(va)) & (1ULL << 63))
> @@ -51,7 +54,7 @@ void
> db_stack_trace_print(db_expr_t addr, int have_addr, db_expr_t count,
> char *modif, int (*pr)(const char *, ...))
> {
> - vaddr_t frame, lastframe, ra, lastra, sp;
> + vaddr_t frame, lastframe, ra, sp, subr;
> char c, *cp = modif;
> db_expr_t offset;
> Elf_Sym * sym;
> @@ -68,60 +71,73 @@ db_stack_trace_print(db_expr_t addr, int
> }
>
> if (!have_addr) {
> - sp = ddb_regs.tf_sp;
> ra = ddb_regs.tf_ra;
> - lastra = ddb_regs.tf_ra;
> frame = ddb_regs.tf_s[0];
> } else {
> sp = addr;
> db_read_bytes(sp - 16, sizeof(vaddr_t), (char *)&frame);
> db_read_bytes(sp - 8, sizeof(vaddr_t), (char *)&ra);
> - lastra = 0;
> }
>
> - while (count-- && frame != 0) {
> - lastframe = frame;
> -
> - sym = db_search_symbol(lastra, DB_STGY_ANY, &offset);
> - db_symbol_values(sym, &name, NULL);
> + while (count != 0 && frame != 0) {
> + if (INKERNEL(frame)) {
> + sym = db_search_symbol(ra, DB_STGY_ANY, &offset);
> + db_symbol_values(sym, &name, NULL);
> + } else {
> + sym = NULL;
> + name = NULL;
> + }
>
> if (name == NULL || strcmp(name, "end") == 0) {
> - (*pr)("%llx at 0x%lx", lastra, ra - 4);
> + (*pr)("%llx() at 0x%lx", ra, ra);
> } else {
> (*pr)("%s() at ", name);
> - db_printsym(ra - 4, DB_STGY_PROC, pr);
> + db_printsym(ra, DB_STGY_PROC, pr);
> }
> (*pr)("\n");
>
> - // can we detect traps ?
> - db_read_bytes(frame - 16, sizeof(vaddr_t), (char *)&frame);
> - if (frame == 0)
> + if ((frame & 0x7) != 0) {
> + (*pr)("bad frame pointer: 0x%lx\n", frame);
> break;
> - lastra = ra;
> - db_read_bytes(frame - 8, sizeof(vaddr_t), (char *)&ra);
> + }
> +
> + subr = 0;
> + if (sym != NULL)
> + subr = ra - (vaddr_t)offset;
>
> -#if 0
> - if (name != NULL) {
> - if ((strcmp (name, "handle_el0_irq") == 0) ||
> - (strcmp (name, "handle_el1_irq") == 0)) {
> - (*pr)("--- interrupt ---\n");
> - } else if (
> - (strcmp (name, "handle_el0_sync") == 0) ||
> - (strcmp (name, "handle_el1_sync") == 0)) {
> - (*pr)("--- trap ---\n");
> + lastframe = frame;
> + if (subr == (vaddr_t)cpu_exception_handler_supervisor ||
> + subr == (vaddr_t)cpu_exception_handler_user) {
> + struct trapframe *tf = (struct trapframe *)frame;
> +
> + db_read_bytes((vaddr_t)&tf->tf_ra, sizeof(ra),
> + (char *)&ra);
> + db_read_bytes((vaddr_t)&tf->tf_s[0], sizeof(frame),
> + (char *)&frame);
> + } else {
> + db_read_bytes(frame - 16, sizeof(frame),
> + (char *)&frame);
> + if (frame == 0)
> + break;
> + if ((frame & 0x7) != 0) {
> + (*pr)("bad frame pointer: 0x%lx\n", frame);
> + break;
> }
> + db_read_bytes(frame - 8, sizeof(ra), (char *)&ra);
> }
> -#endif
> +
> if (INKERNEL(frame)) {
> if (frame <= lastframe) {
> - (*pr)("Bad frame pointer: 0x%lx\n", frame);
> + (*pr)("bad frame pointer: 0x%lx\n", frame);
> break;
> }
> } else {
> - if (kernel_only)
> + if (kernel_only) {
> + (*pr)("end of kernel\n");
> break;
> + }
> }
> -
> --count;
> }
> + (*pr)("end trace frame: 0x%lx, count: %d\n", frame, count);
> }
>
>
