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
> */
>