Wed Apr 12 19:47:41 2023 UTC ()
powerpc/ddb: Use db_read_bytes, not direct pointer access.

Mark some powerpc-variant ifdefs with XXX crash(8), not sure yet what
to do about them.

XXX pullup-8
XXX pullup-9
XXX pullup-10


(riastradh)
diff -r1.30 -r1.31 src/sys/arch/powerpc/powerpc/db_disasm.c
diff -r1.61 -r1.62 src/sys/arch/powerpc/powerpc/db_trace.c

cvs diff -r1.30 -r1.31 src/sys/arch/powerpc/powerpc/db_disasm.c (expand / switch to context diff)
--- src/sys/arch/powerpc/powerpc/db_disasm.c 2023/04/12 17:53:32 1.30
+++ src/sys/arch/powerpc/powerpc/db_disasm.c 2023/04/12 19:47:41 1.31
@@ -1,8 +1,8 @@
-/*	$NetBSD: db_disasm.c,v 1.30 2023/04/12 17:53:32 riastradh Exp $	*/
+/*	$NetBSD: db_disasm.c,v 1.31 2023/04/12 19:47:41 riastradh Exp $	*/
 /*	$OpenBSD: db_disasm.c,v 1.2 1996/12/28 06:21:48 rahnds Exp $	*/
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: db_disasm.c,v 1.30 2023/04/12 17:53:32 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: db_disasm.c,v 1.31 2023/04/12 19:47:41 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ppcarch.h"
@@ -1059,7 +1059,8 @@
 {
 	int class;
 	instr_t opcode;
-	opcode = *(instr_t *)(loc);
+
+	db_read_bytes(loc, sizeof(opcode), (char *)&opcode);
 	class = opcode >> 26;
 	(opcodes_base[class])(opcode, loc);
 

cvs diff -r1.61 -r1.62 src/sys/arch/powerpc/powerpc/db_trace.c (expand / switch to context diff)
--- src/sys/arch/powerpc/powerpc/db_trace.c 2023/04/12 17:53:32 1.61
+++ src/sys/arch/powerpc/powerpc/db_trace.c 2023/04/12 19:47:41 1.62
@@ -1,4 +1,4 @@
-/*	$NetBSD: db_trace.c,v 1.61 2023/04/12 17:53:32 riastradh Exp $	*/
+/*	$NetBSD: db_trace.c,v 1.62 2023/04/12 19:47:41 riastradh Exp $	*/
 /*	$OpenBSD: db_trace.c,v 1.3 1997/03/21 02:10:48 niklas Exp $	*/
 
 /*
@@ -28,7 +28,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: db_trace.c,v 1.61 2023/04/12 17:53:32 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: db_trace.c,v 1.62 2023/04/12 19:47:41 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ppcarch.h"
@@ -69,6 +69,13 @@
 #include <ddb/db_sym.h>
 #include <ddb/db_variables.h>
 
+#define	R(P)								      \
+({									      \
+	__typeof__(*(P)) __db_tmp;					      \
+	db_read_bytes((db_addr_t)(P), sizeof(*(P)), (char *)&__db_tmp);	      \
+	__db_tmp;							      \
+})
+
 const struct db_variable db_regs[] = {
 	{ "r0",  (long *)&ddb_regs.r[0],  FCN_NULL, NULL },
 	{ "r1",  (long *)&ddb_regs.r[1],  FCN_NULL, NULL },
@@ -109,7 +116,7 @@
 	{ "cr",  (long *)&ddb_regs.cr,    FCN_NULL, NULL },
 	{ "xer", (long *)&ddb_regs.xer,   FCN_NULL, NULL },
 	{ "mq",  (long *)&ddb_regs.mq,    FCN_NULL, NULL },
-#ifdef PPC_IBM4XX
+#ifdef PPC_IBM4XX		/* XXX crash(8) */
 	{ "dear", (long *)&ddb_regs.dear, FCN_NULL, NULL },
 	{ "esr", (long *)&ddb_regs.esr,   FCN_NULL, NULL },
 	{ "pid", (long *)&ddb_regs.pid,   FCN_NULL, NULL },
@@ -165,8 +172,8 @@
 
 			if (lwpaddr) {
 				l = (struct lwp *)addr;
-				p = l->l_proc;
-				(*pr)("trace: pid %d ", p->p_pid);
+				p = R(&l->l_proc);
+				(*pr)("trace: pid %d ", R(&p->p_pid));
 			} else {
 				(*pr)("trace: pid %d ", (int)addr);
 				p = db_proc_find((pid_t)addr);
@@ -174,15 +181,15 @@
 					(*pr)("not found\n");
 					return;
 				}
-				l = LIST_FIRST(&p->p_lwps);
+				l = R(&LIST_FIRST(&p->p_lwps));
 				if (l == NULL) {
 					(*pr)("trace: no LWP?\n");
 					return;
 				}
 			}
-			(*pr)("lid %d ", l->l_lid);
+			(*pr)("lid %d ", R(&l->l_lid));
 			pcb = lwp_getpcb(l);
-			frame = (db_addr_t)pcb->pcb_sp;
+			frame = (db_addr_t)R(&pcb->pcb_sp);
 			(*pr)("at %p\n", frame);
 		} else
 			frame = (db_addr_t)addr;
@@ -192,7 +199,7 @@
 	for (;;) {
 		if (frame < PAGE_SIZE)
 			break;
-		frame = *(db_addr_t *)frame;
+		frame = R((db_addr_t *)frame);
 	    next_frame:
 		args = (db_addr_t *)(frame + 8);
 		if (frame < PAGE_SIZE)
@@ -200,7 +207,7 @@
 	        if (count-- == 0)
 			break;
 
-		lr = *(db_addr_t *)(frame + 4) - 4;
+		lr = R((db_addr_t *)(frame + 4)) - 4;
 		if ((lr & 3) || (lr < 0x100)) {
 			(*pr)("saved LR(0x%x) is invalid.", lr);
 			break;
@@ -208,36 +215,42 @@
 
 		(*pr)("0x%08lx: ", frame);
 		if (lr + 4 == (db_addr_t) trapexit ||
-#if !defined(_KERNEL) || defined(PPC_BOOKE)
+#if !defined(_KERNEL) || defined(PPC_BOOKE) /* XXX crash(*) */
 		    lr + 4 == (db_addr_t) intrcall ||
 #endif
 		    lr + 4 == (db_addr_t) sctrapexit) {
 			const char *trapstr;
-			struct trapframe *tf = &((struct ktrapframe *)frame)->ktf_tf;
-			(*pr)("%s ", tf->tf_srr1 & PSL_PR ? "user" : "kernel");
+			struct trapframe *tf =
+			    &((struct ktrapframe *)frame)->ktf_tf;
+			(*pr)("%s ",
+			    R(&tf->tf_srr1) & PSL_PR ? "user" : "kernel");
 			if (lr + 4 == (db_addr_t) sctrapexit) {
-				(*pr)("SC trap #%d by ", tf->tf_fixreg[0]);
+				(*pr)("SC trap #%d by ", R(&tf->tf_fixreg[0]));
 				goto print_trap;
 			}
-			switch (tf->tf_exc) {
+			switch (R(&tf->tf_exc)) {
 			case EXC_DSI:
-#ifdef PPC_OEA
+#ifdef PPC_OEA			/* XXX crash(*) */
 				(*pr)("DSI %s trap @ %#x by ",
-				    tf->tf_dsisr & DSISR_STORE ? "write" : "read",
-				    tf->tf_dar);
+				    (R(&tf->tf_dsisr) & DSISR_STORE
+					? "write"
+					: "read"),
+				    R(&tf->tf_dar));
 #endif
-#ifdef PPC_IBM4XX
+#ifdef PPC_IBM4XX		/* XXX crash(*) */
 				trapstr = "DSI";
 dsi:
 				(*pr)("%s %s trap @ %#x by ", trapstr,
-				    tf->tf_esr & ESR_DST ? "write" : "read",
-				    tf->tf_dear);
+				    (R(&tf->tf_esr) & ESR_DST
+					? "write"
+					: "read"),
+				    R(&tf->tf_dear));
 #endif
 				goto print_trap;
 			case EXC_ALI:
-#ifdef PPC_OEA
+#ifdef PPC_OEA			/* XXX crash(8) */
 				(*pr)("ALI trap @ %#x (DSISR %#x) ",
-				    tf->tf_dar, tf->tf_dsisr);
+				    R(&tf->tf_dar), R(&tf->tf_dsisr));
 				goto print_trap;
 #else
 				trapstr = "ALI"; break;
@@ -258,7 +271,7 @@
 			case EXC_SMI: trapstr = "SMI"; break;
 			case EXC_RST: trapstr = "RST"; break;
 			case EXC_DTMISS: trapstr = "DTMISS";
-#ifdef PPC_IBM4XX
+#ifdef PPC_IBM4XX		/* XXX crash(8) */
 				goto dsi;
 #endif
 				break;
@@ -271,41 +284,46 @@
 			if (trapstr != NULL) {
 				(*pr)("%s trap by ", trapstr);
 			} else {
-				(*pr)("trap %#x by ", tf->tf_exc);
+				(*pr)("trap %#x by ", R(&tf->tf_exc));
 			}
 		   print_trap:
-			lr = (db_addr_t) tf->tf_srr0;
+			lr = (db_addr_t)R(&tf->tf_srr0);
 			diff = 0;
 			symname = NULL;
-			if (in_kernel && (tf->tf_srr1 & PSL_PR) == 0) {
+			if (in_kernel && (R(&tf->tf_srr1) & PSL_PR) == 0) {
 				sym = db_search_symbol(lr, DB_STGY_ANY, &diff);
 				db_symbol_values(sym, &symname, 0);
 			}
 			if (symname == NULL || !strcmp(symname, "end")) {
-				(*pr)("%p: srr1=%#x\n", lr, tf->tf_srr1);
+				(*pr)("%p: srr1=%#x\n", lr, R(&tf->tf_srr1));
 			} else {
 				(*pr)("%s+%#x: srr1=%#x\n", symname,
-				    diff, tf->tf_srr1);
+				    diff, R(&tf->tf_srr1));
 			}
 			(*pr)("%-10s  r1=%#x cr=%#x xer=%#x ctr=%#x",
-			    "", tf->tf_fixreg[1], tf->tf_cr, tf->tf_xer, tf->tf_ctr);
-#ifdef PPC_OEA
-			if (tf->tf_exc == EXC_DSI)
-				(*pr)(" dsisr=%#x", tf->tf_dsisr);
-#ifdef PPC_OEA601
-			if ((mfpvr() >> 16) == MPC601)
-				(*pr)(" mq=%#x", tf->tf_mq);
+			    "",
+			    R(&tf->tf_fixreg[1]),
+			    R(&tf->tf_cr),
+			    R(&tf->tf_xer),
+			    R(&tf->tf_ctr));
+#ifdef PPC_OEA			/* XXX crash(8) */
+			if (R(&tf->tf_exc) == EXC_DSI)
+				(*pr)(" dsisr=%#x", R(&tf->tf_dsisr));
+#ifdef PPC_OEA601		/* XXX crash(8) */
+			if ((mfpvr() >> 16) == MPC601) /* XXX crash(8) */
+				(*pr)(" mq=%#x", R(&tf->tf_mq));
 #endif /* PPC_OEA601 */
 #endif /* PPC_OEA */
-#ifdef PPC_IBM4XX
-			if (tf->tf_exc == EXC_DSI ||
-			    tf->tf_exc == EXC_DTMISS)
-				(*pr)(" dear=%#x", tf->tf_dear);
-			(*pr)(" esr=%#x pid=%#x", tf->tf_esr, tf->tf_pid);
+#ifdef PPC_IBM4XX		/* XXX crash(8) */
+			if (R(&tf->tf_exc) == EXC_DSI ||
+			    R(&tf->tf_exc) == EXC_DTMISS)
+				(*pr)(" dear=%#x", R(&tf->tf_dear));
+			(*pr)(" esr=%#x pid=%#x", R(&tf->tf_esr),
+			    R(&tf->tf_pid));
 #endif
 			(*pr)("\n");
-			frame = (db_addr_t) tf->tf_fixreg[1];
-			in_kernel = !(tf->tf_srr1 & PSL_PR);
+			frame = (db_addr_t)R(&tf->tf_fixreg[1]);
+			in_kernel = !(R(&tf->tf_srr1) & PSL_PR);
 			if (kernel_only && !in_kernel)
 				break;
 			goto next_frame;
@@ -324,8 +342,8 @@
 		if (full)
 			/* Print all the args stored in that stackframe. */
 			(*pr)("(%lx, %lx, %lx, %lx, %lx, %lx, %lx, %lx)",
-				args[0], args[1], args[2], args[3],
-				args[4], args[5], args[6], args[7]);
+			    R(&args[0]), R(&args[1]), R(&args[2]), R(&args[3]),
+			    R(&args[4]), R(&args[5]), R(&args[6]), R(&args[7]));
 		(*pr)("\n");
 	}
 }