svn commit: r285775 - in head/sys: amd64/amd64 i386/i386

[Mark Johnston]

Author: markj
Date: Tue Jul 21 23:13:11 2015
New Revision: 285775
URL: https://svnweb.freebsd.org/changeset/base/285775

Log:
  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

Modified:
  head/sys/amd64/amd64/db_trace.c
  head/sys/i386/i386/db_trace.c

Modified: head/sys/amd64/amd64/db_trace.c
==============================================================================
--- head/sys/amd64/amd64/db_trace.c	Tue Jul 21 23:07:55 2015	(r285774)
+++ head/sys/amd64/amd64/db_trace.c	Tue Jul 21 23:13:11 2015	(r285775)
@@ -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
 }
 
 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 t
 		 */
 		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 t
 				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 
 
 	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

Modified: head/sys/i386/i386/db_trace.c
==============================================================================
--- head/sys/i386/i386/db_trace.c	Tue Jul 21 23:07:55 2015	(r285774)
+++ head/sys/i386/i386/db_trace.c	Tue Jul 21 23:13:11 2015	(r285775)
@@ -389,7 +389,7 @@ db_nextframe(struct i386_frame **fp, db_
 
 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 t
 		 */
 		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 t
 				    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 
 
 	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