Reorder trapframe/intrframe to put %rbp next to %rip and make it

behave like a real call frame, thus vastly simplifying the ddb back
trace logic.

based on whinging from deraadt@
ok jasper@ mpi@ phessler@

4 files changed, 66 insertions, 151 deletions

diff --git a/sys/arch/amd64/amd64/db_trace.c b/sys/arch/amd64/amd64/db_trace.c
index 5b3e9e27487..8428af0eac0 100644
--- a/sys/arch/amd64/amd64/db_trace.c
+++ b/sys/arch/amd64/amd64/db_trace.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: db_trace.c,v 1.38 2018/02/10 10:25:44 mpi Exp $	*/
+/*	$OpenBSD: db_trace.c,v 1.39 2018/04/26 12:47:02 guenther Exp $	*/
 /*	$NetBSD: db_trace.c,v 1.1 2003/04/26 18:39:27 fvdl Exp $	*/
 
 /*
@@ -73,85 +73,6 @@ struct db_variable * db_eregs = db_regs + nitems(db_regs);
  */
 #define	INKERNEL(va)	(((vaddr_t)(va)) >= VM_MIN_KERNEL_ADDRESS)
 
-#define	NONE		0
-#define	TRAP		1
-#define	SYSCALL		2
-#define	INTERRUPT	3
-#define	AST		4
-
-void db_nextframe(struct callframe **, db_addr_t *, long *, int,
-    int (*) (const char *, ...));
-
-/*
- * Figure out the next frame up in the call stack.
- * For trap(), we print the address of the faulting instruction and
- *   proceed with the calling frame.  We return the ip that faulted.
- *   If the trap was caused by jumping through a bogus pointer, then
- *   the next line in the backtrace will list some random function as
- *   being called.  It should get the argument list correct, though.
- *   It might be possible to dig out from the next frame up the name
- *   of the function that faulted, but that could get hairy.
- */
-void
-db_nextframe(struct callframe **fp, db_addr_t *ip, long *argp, int is_trap,
-    int (*pr)(const char *, ...))
-{
-
-	switch (is_trap) {
-	    case NONE:
-		*ip = (db_addr_t)
-			db_get_value((db_addr_t)&(*fp)->f_retaddr, 8, FALSE);
-		*fp = (struct callframe *)
-			db_get_value((db_addr_t)&(*fp)->f_frame, 8, FALSE);
-		break;
-
-	    default: {
-		struct trapframe *tf;
-
-		/* The only argument to trap() or syscall() is the trapframe. */
-		tf = (struct trapframe *)argp;
-		switch (is_trap) {
-		case TRAP:
-			(*pr)("--- trap (number %d) ---\n", tf->tf_trapno);
-			break;
-		case AST:
-			(*pr)("--- ast ---\n");
-			break;
-		case SYSCALL:
-			(*pr)("--- syscall (number %ld) ---\n", tf->tf_rax);
-			break;
-		case INTERRUPT:
-			(*pr)("--- interrupt ---\n");
-			break;
-		}
-		*fp = (struct callframe *)tf->tf_rbp;
-		*ip = (db_addr_t)tf->tf_rip;
-		break;
-	    }
-	}
-}
-
-static inline int
-db_is_trap(const char *name)
-{
-	if (name != NULL) {
-		if (!strcmp(name, "trap"))
-			return TRAP;
-		if (!strcmp(name, "ast"))
-			return AST;
-		if (!strcmp(name, "syscall"))
-			return SYSCALL;
-		if (name[0] == 'X') {
-			if (!strncmp(name, "Xintr", 5) ||
-			    !strncmp(name, "Xresume", 7) ||
-			    !strncmp(name, "Xrecurse", 8) ||
-			    !strcmp(name, "Xdoreti") ||
-			    !strncmp(name, "Xsoft", 5))
-				return INTERRUPT;
-		}
-	}
-	return NONE;
-}
 
 const unsigned long *db_reg_args[6] = {
 	(unsigned long *)&ddb_regs.tf_rdi,
@@ -170,7 +91,6 @@ db_stack_trace_print(db_expr_t addr, boolean_t have_addr, db_expr_t count,
 	unsigned long	*argp, *arg0;
 	db_addr_t	callpc;
 	unsigned int	cr4save = CR4_SMEP|CR4_SMAP;
-	int		is_trap = 0;
 	boolean_t	kernel_only = TRUE;
 	boolean_t	trace_proc = FALSE;
 	struct proc	*p;
@@ -242,14 +162,10 @@ db_stack_trace_print(db_expr_t addr, boolean_t have_addr, db_expr_t count,
 				offset = 0;
 			}
 		}
-		if (INKERNEL(callpc) && (is_trap = db_is_trap(name)) != NONE)
-			narg = 0;
-		else {
-			is_trap = NONE;
-			narg = db_ctf_func_numargs(sym);
-			if (narg < 0 || narg > 6)
-				narg = 6;
-		}
+
+		narg = db_ctf_func_numargs(sym);
+		if (narg < 0 || narg > 6)
+			narg = 6;
 
 		if (name == NULL)
 			(*pr)("%lx(", callpc);
@@ -291,7 +207,7 @@ db_stack_trace_print(db_expr_t addr, boolean_t have_addr, db_expr_t count,
 		db_printsym(callpc, DB_STGY_PROC, pr);
 		(*pr)("\n");
 
-		if (lastframe == 0 && offset == 0 && !have_addr && !is_trap) {
+		if (lastframe == 0 && offset == 0 && !have_addr) {
 			/* Frame really belongs to next callpc */
 			lastframe = (struct callframe *)(ddb_regs.tf_rsp-8);
 			callpc = (db_addr_t)
@@ -300,19 +216,11 @@ db_stack_trace_print(db_expr_t addr, boolean_t have_addr, db_expr_t count,
 			continue;
 		}
 
-		if (is_trap == INTERRUPT && lastframe != NULL) {
-			/*
-			 * Interrupt routines don't update %rbp, so it still
-			 * points to the frame that was interrupted.  Pull
-			 * back to just above lastframe so we can find the
-			 * trapframe as with syscalls and traps.
-			 */
-			frame = (struct callframe *)&lastframe->f_retaddr;
-			arg0 = &frame->f_arg0;
-		}
-
 		lastframe = frame;
-		db_nextframe(&frame, &callpc, arg0, is_trap, pr);
+		callpc = (db_addr_t)db_get_value(
+		    (db_addr_t)&frame->f_retaddr, 8, FALSE);
+		frame = (struct callframe *)db_get_value(
+		    (db_addr_t)&frame->f_frame, 8, FALSE);
 
 		if (frame == 0) {
 			/* end of chain */
@@ -342,11 +250,6 @@ db_stack_trace_print(db_expr_t addr, boolean_t have_addr, db_expr_t count,
 	}
 	(*pr)("end trace frame: 0x%lx, count: %d\n", frame, count);
 
-	if (count && is_trap != NONE) {
-		db_printsym(callpc, DB_STGY_XTRN, pr);
-		(*pr)(":\n");
-	}
-
 	if (cr4save & CR4_SMAP)
 		lcr4(cr4save);
 }
@@ -365,46 +268,17 @@ db_save_stack_trace(struct db_stack_trace *st)
 
 	lastframe = NULL;
 	for (i = 0; i < DB_STACK_TRACE_MAX && frame != NULL; i++) {
-		struct trapframe *tf;
 		char		*name;
 		db_expr_t	offset;
 		Elf_Sym *	sym;
-		int		is_trap;
 
 		st->st_pc[st->st_count++] = callpc;
 		sym = db_search_symbol(callpc, DB_STGY_ANY, &offset);
 		db_symbol_values(sym, &name, NULL);
 
-		if (INKERNEL(callpc))
-			is_trap = db_is_trap(name);
-		else
-			is_trap = NONE;
-
-		if (is_trap == NONE) {
-			lastframe = frame;
-			callpc = frame->f_retaddr;
-			frame = frame->f_frame;
-		} else {
-			if (is_trap == INTERRUPT) {
-				/*
-				 * Interrupt routines don't update %rbp,
-				 * so it still points to the frame that
-				 * was interrupted.  Pull back to just
-				 * above lastframe so we can find the
-				 * trapframe as with syscalls and traps.
-				 */
-				if (lastframe == NULL)
-					break;
-
-				frame =
-				    (struct callframe *)&lastframe->f_retaddr;
-			}
-			lastframe = frame;
-
-			tf = (struct trapframe *)&frame->f_arg0;
-			callpc = (db_addr_t)tf->tf_rip;
-			frame = (struct callframe *)tf->tf_rbp;
-		}
+		lastframe = frame;
+		callpc = frame->f_retaddr;
+		frame = frame->f_frame;
 
 		if (!INKERNEL(frame))
 			break;
diff --git a/sys/arch/amd64/amd64/machdep.c b/sys/arch/amd64/amd64/machdep.c
index 8810f71dcb8..b84b2e1d5c6 100644
--- a/sys/arch/amd64/amd64/machdep.c
+++ b/sys/arch/amd64/amd64/machdep.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: machdep.c,v 1.241 2018/04/12 17:13:43 deraadt Exp $	*/
+/*	$OpenBSD: machdep.c,v 1.242 2018/04/26 12:47:02 guenther Exp $	*/
 /*	$NetBSD: machdep.c,v 1.3 2003/05/07 22:58:18 fvdl Exp $	*/
 
 /*-
@@ -586,8 +586,27 @@ sendsig(sig_t catcher, int sig, int mask, u_long code, int type,
 	register_t sp, scp, sip;
 	u_long sss;
 
-	memcpy(&ksc, tf, sizeof(*tf));
-	bzero((char *)&ksc + sizeof(*tf), sizeof(ksc) - sizeof(*tf));
+	memset(&ksc, 0, sizeof ksc);
+	ksc.sc_rdi = tf->tf_rdi;
+	ksc.sc_rsi = tf->tf_rsi;
+	ksc.sc_rdx = tf->tf_rdx;
+	ksc.sc_rcx = tf->tf_rcx;
+	ksc.sc_r8  = tf->tf_r8;
+	ksc.sc_r9  = tf->tf_r9;
+	ksc.sc_r10 = tf->tf_r10;
+	ksc.sc_r11 = tf->tf_r11;
+	ksc.sc_r12 = tf->tf_r12;
+	ksc.sc_r13 = tf->tf_r13;
+	ksc.sc_r14 = tf->tf_r14;
+	ksc.sc_r15 = tf->tf_r15;
+	ksc.sc_rbx = tf->tf_rbx;
+	ksc.sc_rax = tf->tf_rax;
+	ksc.sc_rbp = tf->tf_rbp;
+	ksc.sc_rip = tf->tf_rip;
+	ksc.sc_cs  = tf->tf_cs;
+	ksc.sc_rflags = tf->tf_rflags;
+	ksc.sc_rsp = tf->tf_rsp;
+	ksc.sc_ss  = tf->tf_ss;
 	ksc.sc_mask = mask;
 
 	/* Allocate space for the signal handler context. */
@@ -696,9 +715,26 @@ sys_sigreturn(struct proc *p, void *v, register_t *retval)
 		p->p_md.md_flags |= MDP_USEDFPU;
 	}
 
-	ksc.sc_trapno = tf->tf_trapno;
-	ksc.sc_err = tf->tf_err;
-	memcpy(tf, &ksc, sizeof(*tf));
+	tf->tf_rdi = ksc.sc_rdi;
+	tf->tf_rsi = ksc.sc_rsi;
+	tf->tf_rdx = ksc.sc_rdx;
+	tf->tf_rcx = ksc.sc_rcx;
+	tf->tf_r8  = ksc.sc_r8;
+	tf->tf_r9  = ksc.sc_r9;
+	tf->tf_r10 = ksc.sc_r10;
+	tf->tf_r11 = ksc.sc_r11;
+	tf->tf_r12 = ksc.sc_r12;
+	tf->tf_r13 = ksc.sc_r13;
+	tf->tf_r14 = ksc.sc_r14;
+	tf->tf_r15 = ksc.sc_r15;
+	tf->tf_rbx = ksc.sc_rbx;
+	tf->tf_rax = ksc.sc_rax;
+	tf->tf_rbp = ksc.sc_rbp;
+	tf->tf_rip = ksc.sc_rip;
+	tf->tf_cs  = ksc.sc_cs;
+	tf->tf_rflags = ksc.sc_rflags;
+	tf->tf_rsp = ksc.sc_rsp;
+	tf->tf_ss  = ksc.sc_ss;
 
 	/* Restore signal mask. */
 	p->p_sigmask = ksc.sc_mask & ~sigcantmask;
diff --git a/sys/arch/amd64/include/frame.h b/sys/arch/amd64/include/frame.h
index 997adbf570c..c6a152398eb 100644
--- a/sys/arch/amd64/include/frame.h
+++ b/sys/arch/amd64/include/frame.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: frame.h,v 1.7 2018/02/21 19:24:15 guenther Exp $	*/
+/*	$OpenBSD: frame.h,v 1.8 2018/04/26 12:47:02 guenther Exp $	*/
 /*	$NetBSD: frame.h,v 1.1 2003/04/26 18:39:40 fvdl Exp $	*/
 
 /*-
@@ -94,7 +94,7 @@ struct trapframe {
 	int64_t tf_r13;
 	int64_t tf_r14;
 	int64_t tf_r15;
-	int64_t	tf_rbp;
+	int64_t	tf_err;		/* not the hardware position */
 	int64_t	tf_rbx;
 	int64_t	tf_rax;
 	int64_t	tf_gs;
@@ -102,8 +102,8 @@ struct trapframe {
 	int64_t	tf_es;
 	int64_t	tf_ds;
 	int64_t	tf_trapno;
+	int64_t	tf_rbp;	/* hardware puts err here, INTRENTRY() moves it up */
 	/* below portion defined in hardware */
-	int64_t	tf_err;
 	int64_t	tf_rip;
 	int64_t	tf_cs;
 	int64_t	tf_rflags;
@@ -129,7 +129,7 @@ struct intrframe {
 	int64_t if_r13;
 	int64_t if_r14;
 	int64_t if_r15;
-	int64_t	if_rbp;
+	u_int64_t __if_err;	/* for compat with trap frame - err */
 	int64_t	if_rbx;
 	int64_t	if_rax;
 	int64_t	tf_gs;
@@ -137,7 +137,7 @@ struct intrframe {
 	int64_t	tf_es;
 	int64_t	tf_ds;
 	u_int64_t __if_trapno; /* for compat with trap frame - trapno */
-	u_int64_t __if_err;	/* for compat with trap frame - err */
+	int64_t	if_rbp;
 	/* below portion defined in hardware */
 	int64_t	if_rip;
 	int64_t	if_cs;
diff --git a/sys/arch/amd64/include/frameasm.h b/sys/arch/amd64/include/frameasm.h
index 5e384acb9dc..665d8a8e17b 100644
--- a/sys/arch/amd64/include/frameasm.h
+++ b/sys/arch/amd64/include/frameasm.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: frameasm.h,v 1.12 2018/02/21 19:24:15 guenther Exp $	*/
+/*	$OpenBSD: frameasm.h,v 1.13 2018/04/26 12:47:02 guenther Exp $	*/
 /*	$NetBSD: frameasm.h,v 1.1 2003/04/26 18:39:40 fvdl Exp $	*/
 
 #ifndef _AMD64_MACHINE_FRAMEASM_H
@@ -28,6 +28,7 @@
 	movq	%rdi,TF_RDI(%rsp)	; \
 	movq	%rsi,TF_RSI(%rsp)	; \
 	movq	%rbp,TF_RBP(%rsp)	; \
+	leaq	TF_RBP(%rsp),%rbp	; \
 	movq	%rbx,TF_RBX(%rsp)	; \
 	movq	%rdx,TF_RDX(%rsp)	; \
 	movq	%rax,TF_RAX(%rsp)
@@ -49,6 +50,10 @@
 INTRENTRY_LABEL(label):	/* from kernel */ \
 	subq	$152,%rsp		; \
 	movq	%rcx,TF_RCX(%rsp)	; \
+	/* the hardware puts err next to %rip, we move it elsewhere and */ \
+	/* later put %rbp in this slot to make it look like a call frame */ \
+	movq	(TF_RIP - 8)(%rsp),%rcx	; \
+	movq	%rcx,TF_ERR(%rsp)	; \
 	jmp	99f			; \
 98:	/* from userspace */		  \
 	movq	CPUVAR(KERN_RSP),%rax	; \