On Tue, Mar 01, 2016 at 11:50:28PM +0100, Martin Pieuchot wrote:
> Trying to get a backtrace after setting a breakpoint on the ``syscall''
> symbol on ddb(4) does not work well. On am64 it faults:
>
> Breakpoint at syscall: pushq %rbp
> ddb{0}> tr
> syscall() at syscall
> Xsyscall() at Xsyscall+0xcb
> uvm_fault(0xffffff000f6c3100, 0x1008, 0, 1) -> e
> kernel: page fault trap, code=0
> Faulted in DDB; continuing...
>
> This is simply because the logic in db_stack_trace_print()/db_nextframe()
> is rather simple and assume the first frame is completely in kernel.
>
> The diff below improves the situation, but is not completely correct
> since we cannot really compare the frame pointers.
>
> Breakpoint at syscall: pushq %rbp
> ddb{0}> tr
> syscall() at syscall
> --- syscall (number 74) ---
> Bad user frame pointer: 0x1000
> end trace frame: 0x1000, count: -1
> 0x70116b2845a:
>
> I'm interested in any input as I'm facing the same problem when I'm
> instrumenting any kernel entry point. But if you think the approach
> below is still a step forward I'll happily commit it.
The diff below seems to improve things, so I would say go for it.
-ml
>
>
> Index: arch/amd64/amd64/db_trace.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/db_trace.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 db_trace.c
> --- arch/amd64/amd64/db_trace.c 1 Mar 2016 21:35:13 -0000 1.16
> +++ arch/amd64/amd64/db_trace.c 1 Mar 2016 22:49:10 -0000
> @@ -176,7 +176,7 @@ db_stack_trace_print(db_expr_t addr, boo
> char *modif, int (*pr)(const char *, ...))
> {
> struct callframe *frame, *lastframe;
> - long *argp;
> + long *argp, *arg0;
> db_addr_t callpc;
> int is_trap = 0;
> boolean_t kernel_only = TRUE;
> @@ -235,7 +235,7 @@ db_stack_trace_print(db_expr_t addr, boo
> offset = 0;
> }
> }
> - if (INKERNEL(frame) && name) {
> + if (INKERNEL(callpc) && name) {
> if (!strcmp(name, "trap")) {
> is_trap = TRAP;
> } else if (!strcmp(name, "ast")) {
> @@ -265,14 +265,15 @@ db_stack_trace_print(db_expr_t addr, boo
> if (lastframe == 0 && offset == 0 && !have_addr) {
> /*
> * We have a breakpoint before the frame is set up
> - * Use %esp instead
> + * Use %rsp instead
> */
> - argp = &((struct callframe
> *)(ddb_regs.tf_rsp-8))->f_arg0;
> + arg0 =
> + &((struct callframe *)(ddb_regs.tf_rsp-8))->f_arg0;
> } else {
> - argp = &frame->f_arg0;
> + arg0 = &frame->f_arg0;
> }
>
> - while (narg) {
> + for (argp = arg0; narg > 0; ) {
> (*pr)("%lx", db_get_value((db_addr_t)argp, 8, FALSE));
> argp++;
> if (--narg != 0)
> @@ -282,7 +283,7 @@ db_stack_trace_print(db_expr_t addr, boo
> db_printsym(callpc, DB_STGY_PROC, pr);
> (*pr)("\n");
>
> - if (lastframe == 0 && offset == 0 && !have_addr) {
> + if (lastframe == 0 && offset == 0 && !have_addr && !is_trap) {
> /* Frame really belongs to next callpc */
> lastframe = (struct callframe *)(ddb_regs.tf_rsp-8);
> callpc = (db_addr_t)
> @@ -299,9 +300,11 @@ db_stack_trace_print(db_expr_t addr, boo
> * trapframe as with syscalls and traps.
> */
> frame = (struct callframe *)&lastframe->f_retaddr;
> + arg0 = &frame->f_arg0;
> }
> +
> lastframe = frame;
> - db_nextframe(&frame, &callpc, &frame->f_arg0, is_trap, pr);
> + db_nextframe(&frame, &callpc, arg0, is_trap, pr);
>
> if (frame == 0) {
> /* end of chain */
> Index: arch/i386/i386/db_trace.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/i386/db_trace.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 db_trace.c
> --- arch/i386/i386/db_trace.c 1 Mar 2016 21:35:13 -0000 1.18
> +++ arch/i386/i386/db_trace.c 1 Mar 2016 22:32:08 -0000
> @@ -158,7 +158,7 @@ db_stack_trace_print(db_expr_t addr, boo
> char *modif, int (*pr)(const char *, ...))
> {
> struct callframe *frame, *lastframe;
> - int *argp;
> + int *argp, *arg0;
> db_addr_t callpc;
> int is_trap = 0;
> boolean_t kernel_only = TRUE;
> @@ -224,7 +224,7 @@ db_stack_trace_print(db_expr_t addr, boo
> offset = 0;
> }
> }
> - if (INKERNEL((int)frame) && name) {
> + if (INKERNEL(callpc) && name) {
> if (!strcmp(name, "trap")) {
> is_trap = TRAP;
> } else if (!strcmp(name, "ast")) {
> @@ -255,12 +255,13 @@ db_stack_trace_print(db_expr_t addr, boo
> * We have a breakpoint before the frame is set up
> * Use %esp instead
> */
> - argp = &((struct callframe
> *)(ddb_regs.tf_esp-4))->f_arg0;
> + arg0 =
> + &((struct callframe *)(ddb_regs.tf_esp-4))->f_arg0;
> } else {
> - argp = &frame->f_arg0;
> + arg0 = &frame->f_arg0;
> }
>
> - while (narg) {
> + for (argp = arg0; narg > 0; ) {
> (*pr)("%x", db_get_value((int)argp, 4, FALSE));
> argp++;
> if (--narg != 0)
> @@ -270,7 +271,7 @@ db_stack_trace_print(db_expr_t addr, boo
> db_printsym(callpc, DB_STGY_PROC, pr);
> (*pr)("\n");
>
> - if (lastframe == 0 && offset == 0 && !have_addr) {
> + if (lastframe == 0 && offset == 0 && !have_addr && !is_trap) {
> /* Frame really belongs to next callpc */
> lastframe = (struct callframe *)(ddb_regs.tf_esp-4);
> callpc = (db_addr_t)
> @@ -279,7 +280,7 @@ db_stack_trace_print(db_expr_t addr, boo
> }
>
> lastframe = frame;
> - db_nextframe(&frame, &callpc, &frame->f_arg0, is_trap, pr);
> + db_nextframe(&frame, &callpc, arg0, is_trap, pr);
>
> if (frame == 0) {
> /* end of chain */
>
