From f8a757d016cf802511922c11b66560d3906f7838 Mon Sep 17 00:00:00 2001
From: Mark Johnston <markj@FreeBSD.org>
Date: Tue, 21 Jul 2015 23:13:11 +0000
Subject: [PATCH] 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
---
 sys/amd64/amd64/db_trace.c | 26 +++++++++++++++++++-------
 sys/i386/i386/db_trace.c   | 23 ++++++++++++++++++-----
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/sys/amd64/amd64/db_trace.c b/sys/amd64/amd64/db_trace.c
index 67322adeaab..0957373565f 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 f79765674bb..c667fbfcb97 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
-- 
2.45.2