Improve stack unwinding on i386 and amd64 after an IP fault.

If we can't find a symbol corresponding to the faulting instruction, assume
that the previously-executed function is a call and attempt to find the
calling function using the return address on the stack. Otherwise we end
up associating the last stack frame with the current call, which is
incorrect and causes the unwinder to skip printing of the calling function,
resulting in a confusing backtrace.

Reviewed by:	jhb
Sponsored by:	EMC / Isilon Storage Division
Differential Revision:	https://reviews.freebsd.org/D2859

2 files changed, 37 insertions, 12 deletions

diff --git a/sys/amd64/amd64/db_trace.c b/sys/amd64/amd64/db_trace.c
index 67322adeaabc..0957373565f7 100644
--- a/sys/amd64/amd64/db_trace.c
+++ b/sys/amd64/amd64/db_trace.c
@@ -200,7 +200,7 @@ static void
 db_print_stack_entry(const char *name, db_addr_t callpc, void *frame)
 {
 
-	db_printf("%s() at ", name);
+	db_printf("%s() at ", name != NULL ? name : "??");
 	db_printsym(callpc, DB_STGY_PROC);
 	if (frame != NULL)
 		db_printf("/frame 0x%lx", (register_t)frame);
@@ -331,8 +331,8 @@ db_nextframe(struct amd64_frame **fp, db_addr_t *ip, struct thread *td)
 }
 
 static int
-db_backtrace(struct thread *td, struct trapframe *tf,
-    struct amd64_frame *frame, db_addr_t pc, int count)
+db_backtrace(struct thread *td, struct trapframe *tf, struct amd64_frame *frame,
+    db_addr_t pc, register_t sp, int count)
 {
 	struct amd64_frame *actframe;
 	const char *name;
@@ -361,7 +361,20 @@ db_backtrace(struct thread *td, struct trapframe *tf,
 		 */
 		actframe = frame;
 		if (first) {
-			if (tf != NULL) {
+			first = FALSE;
+			if (sym == C_DB_SYM_NULL && sp != 0) {
+				/*
+				 * If a symbol couldn't be found, we've probably
+				 * jumped to a bogus location, so try and use
+				 * the return address to find our caller.
+				 */
+				db_print_stack_entry(name, pc, NULL);
+				pc = db_get_value(sp, 8, FALSE);
+				if (db_search_symbol(pc, DB_STGY_PROC,
+				    &offset) == C_DB_SYM_NULL)
+					break;
+				continue;
+			} else if (tf != NULL) {
 				int instr;
 
 				instr = db_get_value(pc, 4, FALSE);
@@ -390,7 +403,6 @@ db_backtrace(struct thread *td, struct trapframe *tf,
 				db_print_stack_entry(name, pc, actframe);
 				break;
 			}
-			first = FALSE;
 		}
 
 		db_print_stack_entry(name, pc, actframe);
@@ -429,7 +441,7 @@ db_trace_self(void)
 	frame = (struct amd64_frame *)rbp;
 	callpc = (db_addr_t)db_get_value((long)&frame->f_retaddr, 8, FALSE);
 	frame = frame->f_frame;
-	db_backtrace(curthread, NULL, frame, callpc, -1);
+	db_backtrace(curthread, NULL, frame, callpc, 0, -1);
 }
 
 int
@@ -439,7 +451,7 @@ db_trace_thread(struct thread *thr, int count)
 
 	ctx = kdb_thr_ctx(thr);
 	return (db_backtrace(thr, NULL, (struct amd64_frame *)ctx->pcb_rbp,
-		    ctx->pcb_rip, count));
+	    ctx->pcb_rip, ctx->pcb_rsp, count));
 }
 
 int
diff --git a/sys/i386/i386/db_trace.c b/sys/i386/i386/db_trace.c
index f79765674bb6..c667fbfcb97a 100644
--- a/sys/i386/i386/db_trace.c
+++ b/sys/i386/i386/db_trace.c
@@ -389,7 +389,7 @@ db_nextframe(struct i386_frame **fp, db_addr_t *ip, struct thread *td)
 
 static int
 db_backtrace(struct thread *td, struct trapframe *tf, struct i386_frame *frame,
-    db_addr_t pc, int count)
+    db_addr_t pc, register_t sp, int count)
 {
 	struct i386_frame *actframe;
 #define MAXNARG	16
@@ -446,7 +446,21 @@ db_backtrace(struct thread *td, struct trapframe *tf, struct i386_frame *frame,
 		 */
 		actframe = frame;
 		if (first) {
-			if (tf != NULL) {
+			first = FALSE;
+			if (sym == C_DB_SYM_NULL && sp != 0) {
+				/*
+				 * If a symbol couldn't be found, we've probably
+				 * jumped to a bogus location, so try and use
+				 * the return address to find our caller.
+				 */
+				db_print_stack_entry(name, 0, 0, 0, pc,
+				    NULL);
+				pc = db_get_value(sp, 4, FALSE);
+				if (db_search_symbol(pc, DB_STGY_PROC,
+				    &offset) == C_DB_SYM_NULL)
+					break;
+				continue;
+			} else if (tf != NULL) {
 				instr = db_get_value(pc, 4, FALSE);
 				if ((instr & 0xffffff) == 0x00e58955) {
 					/* pushl %ebp; movl %esp, %ebp */
@@ -474,7 +488,6 @@ db_backtrace(struct thread *td, struct trapframe *tf, struct i386_frame *frame,
 				    actframe);
 				break;
 			}
-			first = FALSE;
 		}
 
 		argp = &actframe->f_arg0;
@@ -521,7 +534,7 @@ db_trace_self(void)
 	frame = (struct i386_frame *)ebp;
 	callpc = (db_addr_t)db_get_value((int)&frame->f_retaddr, 4, FALSE);
 	frame = frame->f_frame;
-	db_backtrace(curthread, NULL, frame, callpc, -1);
+	db_backtrace(curthread, NULL, frame, callpc, 0, -1);
 }
 
 int
@@ -531,7 +544,7 @@ db_trace_thread(struct thread *thr, int count)
 
 	ctx = kdb_thr_ctx(thr);
 	return (db_backtrace(thr, NULL, (struct i386_frame *)ctx->pcb_ebp,
-		    ctx->pcb_eip, count));
+	    ctx->pcb_eip, ctx->pcb_esp, count));
 }
 
 int