Rework the single step implementation; mostly delaying breakpoint

removal so that single stepping delay slots work. Also added the ability
to single step through system calls (which cause an implicit branch if
successfull).

works for me, "looks ok" kettenis@

4 files changed, 150 insertions, 154 deletions

diff --git a/sys/arch/m88k/include/proc.h b/sys/arch/m88k/include/proc.h
index 1db1c6a3c00..c658452b17a 100644
--- a/sys/arch/m88k/include/proc.h
+++ b/sys/arch/m88k/include/proc.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: proc.h,v 1.1 2004/04/26 12:34:05 miod Exp $ */
+/*	$OpenBSD: proc.h,v 1.2 2005/05/18 16:44:37 miod Exp $ */
 /*
  * Copyright (c) 1992, 1993
  *	The Regents of the University of California.  All rights reserved.
@@ -44,21 +44,27 @@
 #ifndef __MACHINE_PROC_H__
 #define __MACHINE_PROC_H__
 
-#include <machine/pcb.h>
+struct trapframe;
 
 /*
  * Machine-dependent part of the proc structure for mvme88k.
  */
 struct mdproc {
-	struct   trapframe *md_tf;    /* trap/syscall registers */
-	struct   fpstate *md_fpstate; /* fpu state, if any; always resident */
-	int      md_upte[UPAGES];     /* ptes for mapping u page */
+	struct trapframe *md_tf;	/* trap/syscall registers */
+	int	md_upte[UPAGES];	/* ptes for mapping u page */
 
-	unsigned md_ss_addr;          /* single step address for ptrace */
-	unsigned md_ss_instr;         /* single step instruction for ptrace */
-	unsigned md_ss_taken_addr;    /* single step address for ptrace */
-	unsigned md_ss_taken_instr;   /* single step instruction for ptrace */
+        /*
+         * Single stepping is done by moving two breakpoints in the
+         * process' code:
+         * - one breakpoint for regular flow (i.e. the immediately following
+         *   instruction)
+         * - one breakpoint for potential branches.
+         * Using two breakpoints allows us to not worry about the delay slots.
+         */
+	vaddr_t	md_bp0va;
+	u_int	md_bp0save;
+	vaddr_t	md_bp1va;
+	u_int	md_bp1save;
 };
 
 #endif /* __MACHINE_PROC_H__ */
-
diff --git a/sys/arch/m88k/include/ptrace.h b/sys/arch/m88k/include/ptrace.h
index d6fa059eeda..f258f2fff90 100644
--- a/sys/arch/m88k/include/ptrace.h
+++ b/sys/arch/m88k/include/ptrace.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ptrace.h,v 1.2 2005/05/16 11:47:14 miod Exp $ */
+/*	$OpenBSD: ptrace.h,v 1.3 2005/05/18 16:44:37 miod Exp $ */
 /*
  * Copyright (c) 1999, Steve Murphree, Jr.
  * Copyright (c) 1992, 1993
@@ -53,8 +53,4 @@
 #define	PT_GETREGS	(PT_FIRSTMACH + 1)
 #define	PT_SETREGS	(PT_FIRSTMACH + 2)
 
-#ifdef _KERNEL
-int cpu_singlestep(struct proc *);
-#endif
-
 #endif /* __MACHINE_PTRACE_H__ */
diff --git a/sys/arch/m88k/m88k/process_machdep.c b/sys/arch/m88k/m88k/process_machdep.c
index ac78b0f9fe5..13a6acb4dd1 100644
--- a/sys/arch/m88k/m88k/process_machdep.c
+++ b/sys/arch/m88k/m88k/process_machdep.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: process_machdep.c,v 1.2 2005/05/16 11:47:16 miod Exp $ */
+/*	$OpenBSD: process_machdep.c,v 1.3 2005/05/18 16:44:41 miod Exp $ */
 
 /*
  * Copyright (c) 1993 The Regents of the University of California.
@@ -67,9 +67,6 @@
 #include <machine/psl.h>
 #include <machine/reg.h>
 #include <machine/trap.h>
-#if 0
-#include <machine/frame.h>
-#endif
 #include <sys/ptrace.h>
 
 int
@@ -92,15 +89,7 @@ process_write_regs(p, regs)
 	return (0);
 }
 
-int
-process_sstep(p, sstep)
-	struct proc *p;
-	int sstep;
-{
-	if (sstep)
-		cpu_singlestep(p);
-	return (0);
-}
+/* process_sstep() is in trap.c */
 
 int
 process_set_pc(p, addr)
diff --git a/sys/arch/m88k/m88k/trap.c b/sys/arch/m88k/m88k/trap.c
index 627d8c739de..bf5316de288 100644
--- a/sys/arch/m88k/m88k/trap.c
+++ b/sys/arch/m88k/m88k/trap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: trap.c,v 1.17 2005/04/30 16:42:36 miod Exp $	*/
+/*	$OpenBSD: trap.c,v 1.18 2005/05/18 16:44:41 miod Exp $	*/
 /*
  * Copyright (c) 2004, Miodrag Vallat.
  * Copyright (c) 1998 Steve Murphree, Jr.
@@ -87,7 +87,7 @@ extern int procfs_domem(struct proc *, struct proc *, void *, struct uio *);
 __dead void panictrap(int, struct trapframe *);
 __dead void error_fatal(struct trapframe *);
 int double_reg_fixup(struct trapframe *);
-int ss_put_value(struct proc *, unsigned, unsigned, int);
+int ss_put_value(struct proc *, vaddr_t, u_int);
 
 extern void regdump(struct trapframe *f);
 
@@ -506,46 +506,35 @@ user_fault:
 		 * T_STEPBPT trap.
 		 */
 		{
-			unsigned va;
-			unsigned instr;
-			unsigned pc = PC_REGS(&frame->tf_regs);
+			u_int instr;
+			vaddr_t pc = PC_REGS(&frame->tf_regs);
 
 			/* read break instruction */
-			copyin((caddr_t)pc, &instr, sizeof(unsigned));
-#if 0
-			printf("trap: %s (%d) breakpoint %x at %x: (adr %x ins %x)\n",
-			       p->p_comm, p->p_pid, instr, pc,
-			       p->p_md.md_ss_addr, p->p_md.md_ss_instr); /* XXX */
-#endif
+			copyin((caddr_t)pc, &instr, sizeof(u_int));
+
 			/* check and see if we got here by accident */
-			if ((p->p_md.md_ss_addr != pc &&
-			     p->p_md.md_ss_taken_addr != pc) ||
+			if ((p->p_md.md_bp0va != pc &&
+			     p->p_md.md_bp1va != pc) ||
 			    instr != SSBREAKPOINT) {
 				sig = SIGTRAP;
 				fault_type = TRAP_TRACE;
 				break;
 			}
-			/* restore original instruction and clear BP  */
-			va = p->p_md.md_ss_addr;
-			if (va != 0) {
-				instr = p->p_md.md_ss_instr;
-				ss_put_value(p, va, instr, sizeof(instr));
-			}
 
-			/* branch taken instruction */
-			instr = p->p_md.md_ss_taken_instr;
-			if (instr != 0) {
-				va = p->p_md.md_ss_taken_addr;
-				ss_put_value(p, va, instr, sizeof(instr));
+			/* restore original instruction and clear breakpoint */
+			if (p->p_md.md_bp0va == pc) {
+				ss_put_value(p, pc, p->p_md.md_bp0save);
+				p->p_md.md_bp0va = 0;
+			}
+			if (p->p_md.md_bp1va == pc) {
+				ss_put_value(p, pc, p->p_md.md_bp1save);
+				p->p_md.md_bp1va = 0;
 			}
+
 #if 1
 			frame->tf_sfip = frame->tf_snip;
 			frame->tf_snip = pc | NIP_V;
 #endif
-			p->p_md.md_ss_addr = 0;
-			p->p_md.md_ss_instr = 0;
-			p->p_md.md_ss_taken_addr = 0;
-			p->p_md.md_ss_taken_instr = 0;
 			sig = SIGTRAP;
 			fault_type = TRAP_BRKPT;
 		}
@@ -1015,31 +1004,31 @@ m88110_user_fault:
 		 * T_STEPBPT trap.
 		 */
 		{
-			unsigned instr;
-			unsigned pc = PC_REGS(&frame->tf_regs);
+			u_int instr;
+			vaddr_t pc = PC_REGS(&frame->tf_regs);
 
 			/* read break instruction */
-			copyin((caddr_t)pc, &instr, sizeof(unsigned));
-#if 0
-			printf("trap: %s (%d) breakpoint %x at %x: (adr %x ins %x)\n",
-			       p->p_comm, p->p_pid, instr, pc,
-			       p->p_md.md_ss_addr, p->p_md.md_ss_instr); /* XXX */
-#endif
+			copyin((caddr_t)pc, &instr, sizeof(u_int));
+
 			/* check and see if we got here by accident */
-#ifdef notyet
-			if (p->p_md.md_ss_addr != pc || instr != SSBREAKPOINT) {
+			if ((p->p_md.md_bp0va != pc &&
+			     p->p_md.md_bp1va != pc) ||
+			    instr != SSBREAKPOINT) {
 				sig = SIGTRAP;
 				fault_type = TRAP_TRACE;
 				break;
 			}
-#endif
-			/* restore original instruction and clear BP  */
-			instr = p->p_md.md_ss_instr;
-			if (instr != 0)
-				ss_put_value(p, pc, instr, sizeof(instr));
 
-			p->p_md.md_ss_addr = 0;
-			p->p_md.md_ss_instr = 0;
+			/* restore original instruction and clear breakpoint */
+			if (p->p_md.md_bp0va == pc) {
+				ss_put_value(p, pc, p->p_md.md_bp0save);
+				p->p_md.md_bp0va = 0;
+			}
+			if (p->p_md.md_bp1va == pc) {
+				ss_put_value(p, pc, p->p_md.md_bp1save);
+				p->p_md.md_bp1va = 0;
+			}
+
 			sig = SIGTRAP;
 			fault_type = TRAP_BRKPT;
 		}
@@ -1486,50 +1475,50 @@ child_return(arg)
 
 #include <sys/ptrace.h>
 
-unsigned ss_get_value(struct proc *, unsigned, int);
-unsigned ss_branch_taken(unsigned, unsigned,
-    unsigned (*func)(unsigned int, struct reg *), struct reg *);
-unsigned int ss_getreg_val(unsigned int, struct reg *);
-int ss_inst_branch(unsigned);
-int ss_inst_delayed(unsigned);
-unsigned ss_next_instr_address(struct proc *, unsigned, unsigned);
+vaddr_t	ss_branch_taken(u_int, vaddr_t, u_int (*func)(u_int, struct reg *),
+	    struct reg *);
+u_int	ss_getreg_val(u_int, struct reg *);
+int	ss_get_value(struct proc *, vaddr_t, u_int *);
+int	ss_inst_branch(unsigned);
+int	ss_inst_delayed(unsigned);
+int	ss_put_breakpoint(struct proc *, vaddr_t, vaddr_t *, u_int *);
 
-unsigned
-ss_get_value(struct proc *p, unsigned addr, int size)
+#define	SYSCALL_INSTR	0xf000d080	/* tb0 0,r0,128 */
+
+int
+ss_get_value(struct proc *p, vaddr_t addr, u_int *value)
 {
 	struct uio uio;
 	struct iovec iov;
-	unsigned value;
 
-	iov.iov_base = (caddr_t)&value;
-	iov.iov_len = size;
+	iov.iov_base = (caddr_t)value;
+	iov.iov_len = sizeof(u_int);
 	uio.uio_iov = &iov;
 	uio.uio_iovcnt = 1;
 	uio.uio_offset = (off_t)addr;
-	uio.uio_resid = size;
+	uio.uio_resid = sizeof(u_int);
 	uio.uio_segflg = UIO_SYSSPACE;
 	uio.uio_rw = UIO_READ;
 	uio.uio_procp = curproc;
-	procfs_domem(curproc, p, NULL, &uio);
-	return value;
+	return (procfs_domem(curproc, p, NULL, &uio));
 }
 
 int
-ss_put_value(struct proc *p, unsigned addr, unsigned value, int size)
+ss_put_value(struct proc *p, vaddr_t addr, u_int value)
 {
 	struct uio uio;
 	struct iovec iov;
 
 	iov.iov_base = (caddr_t)&value;
-	iov.iov_len = size;
+	iov.iov_len = sizeof(u_int);
 	uio.uio_iov = &iov;
 	uio.uio_iovcnt = 1;
 	uio.uio_offset = (off_t)addr;
-	uio.uio_resid = size;
+	uio.uio_resid = sizeof(u_int);
 	uio.uio_segflg = UIO_SYSSPACE;
 	uio.uio_rw = UIO_WRITE;
 	uio.uio_procp = curproc;
-	return procfs_domem(curproc, p, NULL, &uio);
+	return (procfs_domem(curproc, p, NULL, &uio));
 }
 
 /*
@@ -1543,10 +1532,20 @@ ss_put_value(struct proc *p, unsigned addr, unsigned value, int size)
  *
  * If the instruction is not a control flow instruction, panic.
  */
-unsigned
-ss_branch_taken(unsigned inst, unsigned pc,
-    unsigned (*func)(unsigned int, struct reg *), struct reg *func_data)
+vaddr_t
+ss_branch_taken(u_int inst, vaddr_t pc, u_int (*func)(u_int, struct reg *),
+    struct reg *func_data)
 {
+	/* check for system call */
+	if (inst == SYSCALL_INSTR) {
+		/*
+		 * The regular (pc + 4) breakpoint will match the error
+		 * return. Successfull system calls return at (pc + 8),
+		 * so we'll set up a branch breakpoint there.
+		 */
+		return (pc + 8);
+	}
+
 	/* check if br/bsr */
 	if ((inst & 0xf0000000) == 0xc0000000) {
 		/* signed 26 bit pc relative displacement, shift left two bits */
@@ -1584,14 +1583,14 @@ ss_branch_taken(unsigned inst, unsigned pc,
  *              Returns the value of the register in the specified
  *              frame. Only makes sense for general registers.
  */
-unsigned int
-ss_getreg_val(unsigned int regno, struct reg *regs)
+u_int
+ss_getreg_val(u_int regno, struct reg *regs)
 {
 	return (regno == 0 ? 0 : regs->r[regno]);
 }
 
 int
-ss_inst_branch(unsigned ins)
+ss_inst_branch(u_int ins)
 {
 	/* check high five bits */
 
@@ -1600,21 +1599,21 @@ ss_inst_branch(unsigned ins)
 	case 0x1a: /* bb0 */
 	case 0x1b: /* bb1 */
 	case 0x1d: /* bcnd */
-		return TRUE;
+		return (TRUE);
 		break;
 	case 0x1e: /* could be jmp */
 		if ((ins & 0xfffffbe0) == 0xf400c000)
-			return TRUE;
+			return (TRUE);
 	}
 
-	return FALSE;
+	return (FALSE);
 }
 
 /* ss_inst_delayed - this instruction is followed by a delay slot. Could be
    br.n, bsr.n bb0.n, bb1.n, bcnd.n or jmp.n or jsr.n */
 
 int
-ss_inst_delayed(unsigned ins)
+ss_inst_delayed(u_int ins)
 {
 	/* check the br, bsr, bb0, bb1, bcnd cases */
 	switch ((ins & 0xfc000000) >> (32 - 6)) {
@@ -1623,77 +1622,83 @@ ss_inst_delayed(unsigned ins)
 	case 0x35: /* bb0 */
 	case 0x37: /* bb1 */
 	case 0x3b: /* bcnd */
-		return TRUE;
+		return (TRUE);
 	}
 
 	/* check the jmp, jsr cases */
 	/* mask out bits 0-4, bit 11 */
-	return ((ins & 0xfffff7e0) == 0xf400c400) ? TRUE : FALSE;
+	return (((ins & 0xfffff7e0) == 0xf400c400) ? TRUE : FALSE);
 }
 
-unsigned
-ss_next_instr_address(struct proc *p, unsigned pc, unsigned delay_slot)
+int
+ss_put_breakpoint(struct proc *p, vaddr_t va, vaddr_t *bpva, u_int *bpsave)
 {
-	if (delay_slot == 0)
-		return (pc + 4);
-	else {
-		if (ss_inst_delayed(ss_get_value(p, pc, sizeof(int))))
-			return (pc + 4);
-		else
-			return pc;
+	int rc;
+
+	/* Restore previous breakpoint if we did not trigger it. */
+	if (*bpva != 0) {
+		ss_put_value(p, *bpva, *bpsave);
+		*bpva = 0;
 	}
+
+	/* Save instruction. */
+	if ((rc = ss_get_value(p, va, bpsave)) != 0)
+		return (rc);
+
+	/* Store breakpoint instruction at the location now. */
+	*bpva = va;
+	return (ss_put_value(p, va, SSBREAKPOINT));
 }
 
 int
-cpu_singlestep(p)
-	struct proc *p;
+process_sstep(struct proc *p, int sstep)
 {
 	struct reg *sstf = USER_REGS(p);
 	unsigned pc, brpc;
-	int bpinstr = SSBREAKPOINT;
-	unsigned curinstr;
+	unsigned instr;
+	int rc;
+
+	if (sstep == 0) {
+		/* Restore previous breakpoints if any. */
+		if (p->p_md.md_bp0va != 0) {
+			ss_put_value(p, p->p_md.md_bp0va, p->p_md.md_bp0save);
+			p->p_md.md_bp0va = 0;
+		}
+		if (p->p_md.md_bp1va != 0) {
+			ss_put_value(p, p->p_md.md_bp1va, p->p_md.md_bp1save);
+			p->p_md.md_bp1va = 0;
+		}
+
+		return (0);
+	}
 
-	pc = PC_REGS(sstf);
 	/*
-	 * User was stopped at pc, e.g. the instruction
-	 * at pc was not executed.
+	 * User was stopped at pc, e.g. the instruction at pc was not executed.
 	 * Fetch what's at the current location.
 	 */
-	curinstr = ss_get_value(p, pc, sizeof(int));
-
-	/* compute next address after current location */
-	if (curinstr != 0) {
-		if (ss_inst_branch(curinstr) ||
-		    inst_call(curinstr) || inst_return(curinstr)) {
-			brpc = ss_branch_taken(curinstr, pc, ss_getreg_val, sstf);
-			if (brpc != pc) {   /* self-branches are hopeless */
-				p->p_md.md_ss_taken_addr = brpc;
-				p->p_md.md_ss_taken_instr =
-				    ss_get_value(p, brpc, sizeof(int));
-				/* Store breakpoint instruction at the
-				   "next" location now. */
-				if (ss_put_value(p, brpc, bpinstr,
-				    sizeof(int)) != 0)
-					return (EFAULT);
-			}
-		}
-		pc = ss_next_instr_address(p, pc, 0);
-	} else {
-		pc = PC_REGS(sstf) + 4;
-	}
+	pc = PC_REGS(sstf);
+	if ((rc = ss_get_value(p, pc, &instr)) != 0)
+		return (rc);
 
-	if (p->p_md.md_ss_addr != NULL) {
-		return (EFAULT);
+	/*
+	 * Find if this instruction may cause a branch, and set up a breakpoint
+	 * at the branch location.
+	 */
+	if (ss_inst_branch(instr) || inst_call(instr) || inst_return(instr) ||
+	    instr == SYSCALL_INSTR) {
+		brpc = ss_branch_taken(instr, pc, ss_getreg_val, sstf);
+
+		/* self-branches are hopeless */
+		if (brpc != pc && brpc != 0) {
+			if ((rc = ss_put_breakpoint(p, brpc,
+			    &p->p_md.md_bp1va, &p->p_md.md_bp1save)) != 0)
+				return (rc);
+		}
 	}
 
-	p->p_md.md_ss_addr = pc;
-
-	/* Fetch what's at the "next" location. */
-	p->p_md.md_ss_instr = ss_get_value(p, pc, sizeof(int));
-
-	/* Store breakpoint instruction at the "next" location now. */
-	if (ss_put_value(p, pc, bpinstr, sizeof(int)) != 0)
-		return (EFAULT);
+	if ((rc = ss_put_breakpoint(p, pc + 4,
+	    &p->p_md.md_bp0va, &p->p_md.md_bp0save)) != 0)
+		return (rc);
 
 	return (0);
 }
@@ -1723,8 +1728,8 @@ splassert_check(int wantipl, const char *func)
  * aligned addresses will trigger a misaligned address exception.
  *
  * This routine attempts to recover these (valid) statements, by simulating
- * the splitted form of the instruction. If it fails, it returns the
- * appropriate signal number to deliver.
+ * the split form of the instruction. If it fails, it returns the appropriate
+ * signal number to deliver.
  */
 int
 double_reg_fixup(struct trapframe *frame)