On Thu, Feb 25, 2016 at 03:10:54PM +0100, Martin Pieuchot wrote: > This rename and move the i386 and amd64 ``callframe'' structures in order > to use it in MI code. My goal is to unify our architectures to be able > to retrieve the PC of the calling function with as little MD code as > possible. > > There's no functional change in this diff. ok?
This reads ok to me. ok mlarkin@ > > Index: amd64/amd64/db_trace.c > =================================================================== > RCS file: /cvs/src/sys/arch/amd64/amd64/db_trace.c,v > retrieving revision 1.13 > diff -u -p -r1.13 db_trace.c > --- amd64/amd64/db_trace.c 28 Jun 2015 01:16:28 -0000 1.13 > +++ amd64/amd64/db_trace.c 25 Feb 2016 14:06:10 -0000 > @@ -100,12 +100,6 @@ db_x86_64_regop(struct db_variable *vp, > */ > #define INKERNEL(va) (((vaddr_t)(va)) >= VM_MIN_KERNEL_ADDRESS) > > -struct x86_64_frame { > - struct x86_64_frame *f_frame; > - long f_retaddr; > - long f_arg0; > -}; > - > #define NONE 0 > #define TRAP 1 > #define SYSCALL 2 > @@ -118,8 +112,8 @@ db_addr_t db_kdintr_symbol_value = 0; > boolean_t db_trace_symbols_found = FALSE; > > void db_find_trace_symbols(void); > -int db_numargs(struct x86_64_frame *); > -void db_nextframe(struct x86_64_frame **, db_addr_t *, long *, int, > +int db_numargs(struct callframe *); > +void db_nextframe(struct callframe **, db_addr_t *, long *, int, > int (*) (const char *, ...)); > > void > @@ -143,7 +137,7 @@ db_find_trace_symbols(void) > * reliably determine the values currently, just return 0. > */ > int > -db_numargs(struct x86_64_frame *fp) > +db_numargs(struct callframe *fp) > { > return 0; > } > @@ -159,7 +153,7 @@ db_numargs(struct x86_64_frame *fp) > * of the function that faulted, but that could get hairy. > */ > void > -db_nextframe(struct x86_64_frame **fp, db_addr_t *ip, long *argp, int > is_trap, > +db_nextframe(struct callframe **fp, db_addr_t *ip, long *argp, int is_trap, > int (*pr)(const char *, ...)) > { > > @@ -167,7 +161,7 @@ db_nextframe(struct x86_64_frame **fp, d > case NONE: > *ip = (db_addr_t) > db_get_value((db_addr_t)&(*fp)->f_retaddr, 8, FALSE); > - *fp = (struct x86_64_frame *) > + *fp = (struct callframe *) > db_get_value((db_addr_t)&(*fp)->f_frame, 8, FALSE); > break; > > @@ -190,7 +184,7 @@ db_nextframe(struct x86_64_frame **fp, d > (*pr)("--- interrupt ---\n"); > break; > } > - *fp = (struct x86_64_frame *)tf->tf_rbp; > + *fp = (struct callframe *)tf->tf_rbp; > *ip = (db_addr_t)tf->tf_rip; > break; > } > @@ -201,7 +195,7 @@ void > db_stack_trace_print(db_expr_t addr, boolean_t have_addr, db_expr_t count, > char *modif, int (*pr)(const char *, ...)) > { > - struct x86_64_frame *frame, *lastframe; > + struct callframe *frame, *lastframe; > long *argp; > db_addr_t callpc; > int is_trap = 0; > @@ -226,7 +220,7 @@ db_stack_trace_print(db_expr_t addr, boo > } > > if (!have_addr) { > - frame = (struct x86_64_frame *)ddb_regs.tf_rbp; > + frame = (struct callframe *)ddb_regs.tf_rbp; > callpc = (db_addr_t)ddb_regs.tf_rip; > } else { > if (trace_proc) { > @@ -235,13 +229,13 @@ db_stack_trace_print(db_expr_t addr, boo > (*pr) ("db_trace.c: process not found\n"); > return; > } > - frame = (struct x86_64_frame *)p->p_addr->u_pcb.pcb_rbp; > + frame = (struct callframe *)p->p_addr->u_pcb.pcb_rbp; > } else { > - frame = (struct x86_64_frame *)addr; > + frame = (struct callframe *)addr; > } > callpc = (db_addr_t) > db_get_value((db_addr_t)&frame->f_retaddr, 8, FALSE); > - frame = (struct x86_64_frame *)frame->f_frame; > + frame = (struct callframe *)frame->f_frame; > } > > lastframe = 0; > @@ -304,7 +298,7 @@ db_stack_trace_print(db_expr_t addr, boo > * We have a breakpoint before the frame is set up > * Use %esp instead > */ > - argp = &((struct x86_64_frame > *)(ddb_regs.tf_rsp-8))->f_arg0; > + argp = &((struct callframe > *)(ddb_regs.tf_rsp-8))->f_arg0; > } else { > argp = &frame->f_arg0; > } > @@ -323,7 +317,7 @@ db_stack_trace_print(db_expr_t addr, boo > > if (lastframe == 0 && offset == 0 && !have_addr) { > /* Frame really belongs to next callpc */ > - lastframe = (struct x86_64_frame *)(ddb_regs.tf_rsp-8); > + lastframe = (struct callframe *)(ddb_regs.tf_rsp-8); > callpc = (db_addr_t) > db_get_value((db_addr_t)&lastframe->f_retaddr, > 8, FALSE); > @@ -337,7 +331,7 @@ db_stack_trace_print(db_expr_t addr, boo > * back to just above lastframe so we can find the > * trapframe as with syscalls and traps. > */ > - frame = (struct x86_64_frame *)&lastframe->f_retaddr; > + frame = (struct callframe *)&lastframe->f_retaddr; > } > lastframe = frame; > db_nextframe(&frame, &callpc, &frame->f_arg0, is_trap, pr); > Index: amd64/include/frame.h > =================================================================== > RCS file: /cvs/src/sys/arch/amd64/include/frame.h,v > retrieving revision 1.5 > diff -u -p -r1.5 frame.h > --- amd64/include/frame.h 23 Mar 2011 16:54:34 -0000 1.5 > +++ amd64/include/frame.h 25 Feb 2016 14:06:10 -0000 > @@ -160,4 +160,10 @@ struct switchframe { > int64_t sf_rip; > }; > > +struct callframe { > + struct callframe *f_frame; > + long f_retaddr; > + long f_arg0; > +}; > + > #endif /* _MACHINE_FRAME_H_ */ > Index: i386/i386/db_trace.c > =================================================================== > RCS file: /cvs/src/sys/arch/i386/i386/db_trace.c,v > retrieving revision 1.15 > diff -u -p -r1.15 db_trace.c > --- i386/i386/db_trace.c 28 Jun 2015 01:11:27 -0000 1.15 > +++ i386/i386/db_trace.c 25 Feb 2016 14:06:06 -0000 > @@ -68,12 +68,6 @@ struct db_variable *db_eregs = db_regs + > */ > #define INKERNEL(va) (((vaddr_t)(va)) >= VM_MIN_KERNEL_ADDRESS) > > -struct i386_frame { > - struct i386_frame *f_frame; > - int f_retaddr; > - int f_arg0; > -}; > - > #define NONE 0 > #define TRAP 1 > #define SYSCALL 2 > @@ -86,8 +80,8 @@ db_addr_t db_kdintr_symbol_value = 0; > boolean_t db_trace_symbols_found = FALSE; > > void db_find_trace_symbols(void); > -int db_numargs(struct i386_frame *); > -void db_nextframe(struct i386_frame **, db_addr_t *, int *, int, > +int db_numargs(struct callframe *); > +void db_nextframe(struct callframe **, db_addr_t *, int *, int, > int (*pr)(const char *, ...)); > > void > @@ -108,7 +102,7 @@ db_find_trace_symbols(void) > * Figure out how many arguments were passed into the frame at "fp". > */ > int > -db_numargs(struct i386_frame *fp) > +db_numargs(struct callframe *fp) > { > int *argp; > int inst; > @@ -141,7 +135,7 @@ db_numargs(struct i386_frame *fp) > * of the function that faulted, but that could get hairy. > */ > void > -db_nextframe(struct i386_frame **fp, db_addr_t *ip, int *argp, int > is_trap, > +db_nextframe(struct callframe **fp, db_addr_t *ip, int *argp, int > is_trap, > int (*pr)(const char *, ...)) > { > > @@ -149,7 +143,7 @@ db_nextframe(struct i386_frame **fp, db_ > case NONE: > *ip = (db_addr_t) > db_get_value((int) &(*fp)->f_retaddr, 4, FALSE); > - *fp = (struct i386_frame *) > + *fp = (struct callframe *) > db_get_value((int) &(*fp)->f_frame, 4, FALSE); > break; > > @@ -172,7 +166,7 @@ db_nextframe(struct i386_frame **fp, db_ > (*pr)("--- interrupt ---\n"); > break; > } > - *fp = (struct i386_frame *)tf->tf_ebp; > + *fp = (struct callframe *)tf->tf_ebp; > *ip = (db_addr_t)tf->tf_eip; > break; > } > @@ -183,7 +177,7 @@ void > db_stack_trace_print(db_expr_t addr, boolean_t have_addr, db_expr_t count, > char *modif, int (*pr)(const char *, ...)) > { > - struct i386_frame *frame, *lastframe; > + struct callframe *frame, *lastframe; > int *argp; > db_addr_t callpc; > int is_trap = 0; > @@ -214,7 +208,7 @@ db_stack_trace_print(db_expr_t addr, boo > count = 65535; > > if (!have_addr) { > - frame = (struct i386_frame *)ddb_regs.tf_ebp; > + frame = (struct callframe *)ddb_regs.tf_ebp; > callpc = (db_addr_t)ddb_regs.tf_eip; > } else if (trace_thread) { > (*pr) ("db_trace.c: can't trace thread\n"); > @@ -224,11 +218,11 @@ db_stack_trace_print(db_expr_t addr, boo > (*pr) ("db_trace.c: process not found\n"); > return; > } > - frame = (struct i386_frame *)p->p_addr->u_pcb.pcb_ebp; > + frame = (struct callframe *)p->p_addr->u_pcb.pcb_ebp; > callpc = (db_addr_t) > db_get_value((int)&frame->f_retaddr, 4, FALSE); > } else { > - frame = (struct i386_frame *)addr; > + frame = (struct callframe *)addr; > callpc = (db_addr_t) > db_get_value((int)&frame->f_retaddr, 4, FALSE); > } > @@ -292,7 +286,7 @@ db_stack_trace_print(db_expr_t addr, boo > * We have a breakpoint before the frame is set up > * Use %esp instead > */ > - argp = &((struct i386_frame > *)(ddb_regs.tf_esp-4))->f_arg0; > + argp = &((struct callframe > *)(ddb_regs.tf_esp-4))->f_arg0; > } else { > argp = &frame->f_arg0; > } > @@ -311,7 +305,7 @@ db_stack_trace_print(db_expr_t addr, boo > > if (lastframe == 0 && offset == 0 && !have_addr) { > /* Frame really belongs to next callpc */ > - lastframe = (struct i386_frame *)(ddb_regs.tf_esp-4); > + lastframe = (struct callframe *)(ddb_regs.tf_esp-4); > callpc = (db_addr_t) > db_get_value((int)&lastframe->f_retaddr, 4, > FALSE); > continue; > Index: i386/include/frame.h > =================================================================== > RCS file: /cvs/src/sys/arch/i386/include/frame.h,v > retrieving revision 1.10 > diff -u -p -r1.10 frame.h > --- i386/include/frame.h 3 Jul 2010 04:54:32 -0000 1.10 > +++ i386/include/frame.h 25 Feb 2016 14:06:06 -0000 > @@ -112,6 +112,12 @@ struct switchframe { > int sf_eip; > }; > > +struct callframe { > + struct callframe *f_frame; > + int f_retaddr; > + int f_arg0; > +}; > + > /* > * Signal frame > */ >