Improve stack trace saving on amd64:

* Replace unnecessary use of db_get_value() with direct access of
  f_retaddr. The trace function assumes that the chain of frames
  is good in terms of alignment and content.

* Replace the for loop with a while loop and drop the check
  "frame != NULL" from the loop condition. The check is redundant with
  the "!INKERNEL(frame)" condition. The code already assumes that the
  initial frame is valid.

* Stop iteration if f_retaddr is a non-kernel address in order to
  not save a user address at the end of the trace. This additionally
  fixes a kernel crash that would happen if the user thread had
  a broken frame pointer.

* Fix erroneous skipping of the second stack frame.

OK mpi@

1 files changed, 5 insertions, 11 deletions

diff --git a/sys/arch/amd64/amd64/db_trace.c b/sys/arch/amd64/amd64/db_trace.c
index bc454ddd58f..050fee7188c 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.42 2018/05/04 02:54:23 visa Exp $	*/
+/*	$OpenBSD: db_trace.c,v 1.43 2019/02/02 14:32:58 visa Exp $	*/
 /*	$NetBSD: db_trace.c,v 1.1 2003/04/26 18:39:27 fvdl Exp $	*/
 
 /*
@@ -258,27 +258,21 @@ void
 db_save_stack_trace(struct db_stack_trace *st)
 {
 	struct callframe *frame, *lastframe;
-	db_addr_t callpc;
-	unsigned int i;
 
 	frame = __builtin_frame_address(0);
-
-	callpc = db_get_value((db_addr_t)&frame->f_retaddr, 8, FALSE);
-	frame = frame->f_frame;
-
-	lastframe = NULL;
 	st->st_count = 0;
-	for (i = 0; i < DB_STACK_TRACE_MAX && frame != NULL; i++) {
-		st->st_pc[st->st_count++] = callpc;
+	while (st->st_count < DB_STACK_TRACE_MAX) {
+		st->st_pc[st->st_count++] = frame->f_retaddr;
 
 		lastframe = frame;
-		callpc = frame->f_retaddr;
 		frame = frame->f_frame;
 
 		if (!INKERNEL(frame))
 			break;
 		if (frame <= lastframe)
 			break;
+		if (!INKERNEL(frame->f_retaddr))
+			break;
 	}
 }