Page MenuHomeFreeBSD
Differential D28026 Diff 83220 sys/arm64/arm64/db_trace.c

Changeset View

Standalone View

View Options

sys/arm64/arm64/db_trace.c

Show All 36 Lines
#include <machine/pcb.h> #include <machine/pcb.h>
#include <ddb/ddb.h> #include <ddb/ddb.h>
#include <ddb/db_sym.h> #include <ddb/db_sym.h>
#include <machine/armreg.h> #include <machine/armreg.h>
#include <machine/debug_monitor.h> #include <machine/debug_monitor.h>
#include <machine/stack.h> #include <machine/stack.h>
#include <machine/vmparam.h>
#define FRAME_NORMAL 0
#define FRAME_SYNC 1
#define FRAME_IRQ 2
#define FRAME_SERROR 3
#define FRAME_UNHANDLED 4
void void
db_md_list_watchpoints() db_md_list_watchpoints()
{ {
dbg_show_watchpoint(); dbg_show_watchpoint();
} }
int int
Show All 12 Lines
static void static void
db_stack_trace_cmd(struct thread *td, struct unwind_state *frame) db_stack_trace_cmd(struct thread *td, struct unwind_state *frame)
{ {
c_db_sym_t sym; c_db_sym_t sym;
const char *name; const char *name;
db_expr_t value; db_expr_t value;
db_expr_t offset; db_expr_t offset;
int frame_type;
while (1) { while (1) {
uintptr_t pc = frame->pc; sym = db_search_symbol(frame->pc, DB_STGY_ANY, &offset);
if (!unwind_frame(td, frame))
break;
sym = db_search_symbol(pc, DB_STGY_ANY, &offset);
if (sym == C_DB_SYM_NULL) { if (sym == C_DB_SYM_NULL) {
value = 0; value = 0;
name = "(null)"; name = "(null)";
} else } else
db_symbol_values(sym, &name, &value); db_symbol_values(sym, &name, &value);
db_printf("%s() at ", name); db_printf("%s() at ", name);
db_printsym(frame->pc, DB_STGY_PROC); db_printsym(frame->pc, DB_STGY_PROC);
db_printf("\n"); db_printf("\n");
db_printf("\t pc = 0x%016lx lr = 0x%016lx\n", pc, if (strcmp(name, "handle_el0_sync") == 0 ||
frame->pc); strcmp(name, "handle_el1h_sync") == 0)
db_printf("\t sp = 0x%016lx fp = 0x%016lx\n", frame->sp, frame_type = FRAME_SYNC;
frame->fp); else if (strcmp(name, "handle_el0_irq") == 0 ||
/* TODO: Show some more registers */ strcmp(name, "handle_el1h_irq") == 0)
db_printf("\n"); frame_type = FRAME_IRQ;
else if (strcmp(name, "handle_serror") == 0)
frame_type = FRAME_SERROR;
else if (strcmp(name, "handle_empty_exception") == 0)
frame_type = FRAME_UNHANDLED;
else
frame_type = FRAME_NORMAL;
if (frame_type != FRAME_NORMAL) {
struct trapframe *tf;
tf = (struct trapframe *)(uintptr_t)frame->fp - 1;
if (!kstack_contains(td, (vm_offset_t)tf,
sizeof(*tf))) {
db_printf("--- invalid trapframe %p\n", tf);
break;
} }
switch (frame_type) {
case FRAME_SYNC:
db_printf("--- exception, esr %#x\n",
tf->tf_esr);
break;
case FRAME_IRQ:
db_printf("--- interrupt\n");
break;
case FRAME_SERROR:
db_printf("--- system error, esr %#x\n",
tf->tf_esr);
break;
case FRAME_UNHANDLED:
db_printf("--- unhandled exception, esr %#x\n",
tf->tf_esr);
break;
default:
__assert_unreachable();
jrtc27AuthorUnsubmitted
Done
Inline Actions

Wasn't sure whether to panic here (which is what amd64 does) or just print a big error message and continue given this will almost certainly lead to a panic loop as DDB keeps having to unwind (an ever growing stack with) the same problematic frame.

jrtc27: Wasn't sure whether to panic here (which is what amd64 does) or just print a big error message…
jhbUnsubmitted
Done
Inline Actions

I think for modern kernel code we now have __assert_unreachable(). We didn't have that yet when the amd64 code was written. This definitely falls into that camp as you've handled all the values frame_type could possibly be set to. The compiler might not even require a default case if you instead made the frame types an enum.

jhb: I think for modern kernel code we now have `__assert_unreachable()`. We didn't have that yet…
jrtc27AuthorUnsubmitted
Done
Inline Actions

Sadly this isn't C++'s enum class, there's very little benefit to using a real enum.

jrtc27: Sadly this isn't C++'s `enum class`, there's very little benefit to using a real enum.
break;
} }
frame->fp = tf->tf_x[29];
frame->pc = tf->tf_elr;
if (!INKERNEL(frame->fp))
break;
} else {
if (strcmp(name, "fork_trampoline") == 0)
break;
if (!unwind_frame(td, frame))
break;
}
}
}
markjUnsubmitted
Done
Inline Actions

I'd find the control flow a bit easier to follow if the continue above was removed and we had

} else if (strcmp(name, "fork_trampoline") == 0) {
    break;
} else if (!unwind_frame(td, frame)) {
    break;
}

but it's just a suggestion.

markj: I'd find the control flow a bit easier to follow if the `continue` above was removed and we had…
jrtc27AuthorUnsubmitted
Done
Inline Actions

Hm, I can see the argument, though I do feel they're unrelated, especially the side-effecting unwind_frame condition shouldn't be lumped in with the others IMO. I did it this way because (a) it's how RISC-V does it (b) it can be thought of as a list of special cases.

Though putting the entire rest of the block inside an else rather than chaining else ifs should work for both of us and could be done for RISC-V too to remove the continue there (which is a little simpler due to the trap type being in scause rather than being based on which trap handler was called).

jrtc27: Hm, I can see the argument, though I do feel they're unrelated, especially the side-effecting…
jhbUnsubmitted
Done
Inline Actions

I'm ambivalent. Exception frames certainly are kind of special in how they are treated, so

if (trapframe) {
  big blob of code;
  continue;
}

/* normal frames */

Seems ok to me. It's at least a bit less painful that what we do over in i386 where we have a return in the middle of db_nextframe() for "normal" frames.

jhb: I'm ambivalent. Exception frames certainly are kind of special in how they are treated, so…
jrtc27AuthorUnsubmitted
Done
Inline Actions

Hopefully easier to follow now

jrtc27: Hopefully easier to follow now
int int
db_trace_thread(struct thread *thr, int count) db_trace_thread(struct thread *thr, int count)
{ {
struct unwind_state frame; struct unwind_state frame;
struct pcb *ctx; struct pcb *ctx;
if (thr != curthread) { if (thr != curthread) {
ctx = kdb_thr_ctx(thr); ctx = kdb_thr_ctx(thr);
frame.sp = (uintptr_t)ctx->pcb_sp;
frame.fp = (uintptr_t)ctx->pcb_x[29]; frame.fp = (uintptr_t)ctx->pcb_x[29];
frame.pc = (uintptr_t)ctx->pcb_lr; frame.pc = (uintptr_t)ctx->pcb_lr;
db_stack_trace_cmd(thr, &frame); db_stack_trace_cmd(thr, &frame);
} else } else
db_trace_self(); db_trace_self();
return (0); return (0);
} }
void void
db_trace_self(void) db_trace_self(void)
{ {
struct unwind_state frame; struct unwind_state frame;
uintptr_t sp;
__asm __volatile("mov %0, sp" : "=&r" (sp));
frame.sp = sp;
frame.fp = (uintptr_t)__builtin_frame_address(0); frame.fp = (uintptr_t)__builtin_frame_address(0);
frame.pc = (uintptr_t)db_trace_self; frame.pc = (uintptr_t)db_trace_self;
db_stack_trace_cmd(curthread, &frame); db_stack_trace_cmd(curthread, &frame);
} }