Make sure ptrace_sstep(,0) actually clears breakpoints, by replacing the

ptrace guts with a logic similar to what the alpha port does.

3 files changed, 106 insertions, 110 deletions

diff --git a/sys/arch/mips64/include/proc.h b/sys/arch/mips64/include/proc.h
index 2d9682d8800..b78d8f109e8 100644
--- a/sys/arch/mips64/include/proc.h
+++ b/sys/arch/mips64/include/proc.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: proc.h,v 1.5 2009/12/07 19:01:03 miod Exp $	*/
+/*	$OpenBSD: proc.h,v 1.6 2010/11/24 21:01:02 miod Exp $	*/
 
 /*
  * Copyright (c) 1992, 1993
@@ -44,9 +44,12 @@ struct mdproc {
 	struct trap_frame *md_regs;	/* registers on current frame */
 	volatile int md_astpending;	/* AST pending for this process */
 	int	md_flags;		/* machine-dependent flags */
-	long	md_ss_addr;		/* single step address for ptrace */
-	int	md_ss_instr;		/* single step instruction for ptrace */
 	vaddr_t	md_uarea;		/* allocated uarea virtual addr */
+
+	/* ptrace fields */
+	vaddr_t	md_ss_addr;		/* single step address */
+	uint32_t md_ss_instr;		/* saved single step instruction */
+
 /* The following is RM7000 dependent, but kept in for compatibility */
 	int	md_pc_ctrl;		/* performance counter control */
 	int	md_pc_count;		/* performance counter */
diff --git a/sys/arch/mips64/mips64/process_machdep.c b/sys/arch/mips64/mips64/process_machdep.c
index 748c3ca7af2..e26950f0f24 100644
--- a/sys/arch/mips64/mips64/process_machdep.c
+++ b/sys/arch/mips64/mips64/process_machdep.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: process_machdep.c,v 1.15 2010/09/21 20:29:17 miod Exp $	*/
+/*	$OpenBSD: process_machdep.c,v 1.16 2010/11/24 21:01:03 miod Exp $	*/
 
 /*
  * Copyright (c) 1994 Adam Glass
@@ -40,7 +40,7 @@
  * From:
  *	Id: procfs_i386.c,v 4.1 1993/12/17 10:47:45 jsp Rel
  *
- *	$Id: process_machdep.c,v 1.15 2010/09/21 20:29:17 miod Exp $
+ *	$Id: process_machdep.c,v 1.16 2010/11/24 21:01:03 miod Exp $
  */
 
 /*
@@ -78,8 +78,6 @@
 
 #define	REGSIZE (sizeof(struct trap_frame) - sizeof(register_t))
 
-extern void cpu_singlestep(struct proc *);
-
 int
 process_read_regs(p, regs)
 	struct proc *p;
@@ -119,14 +117,7 @@ process_write_regs(p, regs)
 	return (0);
 }
 
-int
-process_sstep(p, sstep)
-	struct proc *p;
-{
-	if (sstep)
-		cpu_singlestep(p);
-	return (0);
-}
+/* process_sstep is in trap.c */
 
 int
 process_set_pc(p, addr)
diff --git a/sys/arch/mips64/mips64/trap.c b/sys/arch/mips64/mips64/trap.c
index 44bae4b610e..0cf070b140b 100644
--- a/sys/arch/mips64/mips64/trap.c
+++ b/sys/arch/mips64/mips64/trap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: trap.c,v 1.70 2010/10/24 15:39:18 miod Exp $	*/
+/*	$OpenBSD: trap.c,v 1.71 2010/11/24 21:01:03 miod Exp $	*/
 
 /*
  * Copyright (c) 1988 University of Utah.
@@ -136,7 +136,9 @@ extern int kdb_trap(int, db_regs_t *);
 void	ast(void);
 void	trap(struct trap_frame *);
 #ifdef PTRACE
-int	cpu_singlestep(struct proc *);
+int	ptrace_read_insn(struct proc *, vaddr_t, uint32_t *);
+int	ptrace_write_insn(struct proc *, vaddr_t, uint32_t);
+int	process_sstep(struct proc *, int);
 #endif
 
 static __inline__ void
@@ -559,8 +561,6 @@ printf("SIG-BUSB @%p pc %p, ra %p\n", trapframe->badvaddr, trapframe->pc, trapfr
 			locr0->v0 = i;
 			locr0->a3 = 1;
 		}
-		if (code == SYS_ptrace)
-			Mips_SyncCache(curcpu());
 #ifdef SYSCALL_DEBUG
 		KERNEL_PROC_LOCK(p);
 		scdebug_ret(p, code, i, rval);
@@ -600,12 +600,6 @@ printf("SIG-BUSB @%p pc %p, ra %p\n", trapframe->badvaddr, trapframe->pc, trapfr
 		/* read break instruction */
 		copyin(va, &instr, sizeof(int32_t));
 
-#if 0
-		printf("trap: %s (%d) breakpoint %x at %x: (adr %x ins %x)\n",
-			p->p_comm, p->p_pid, instr, trapframe->pc,
-			p->p_md.md_ss_addr, p->p_md.md_ss_instr); /* XXX */
-#endif
-
 		switch ((instr & BREAK_VAL_MASK) >> BREAK_VAL_SHIFT) {
 		case 6:	/* gcc range error */
 			i = SIGFPE;
@@ -630,32 +624,15 @@ printf("SIG-BUSB @%p pc %p, ra %p\n", trapframe->badvaddr, trapframe->pc, trapfr
 #ifdef PTRACE
 		case BREAK_SSTEP_VAL:
 			if (p->p_md.md_ss_addr == (long)va) {
-				struct uio uio;
-				struct iovec iov;
-				int error;
-
-				/*
-				 * Restore original instruction and clear BP
-				 */
-				iov.iov_base = (caddr_t)&p->p_md.md_ss_instr;
-				iov.iov_len = sizeof(int);
-				uio.uio_iov = &iov;
-				uio.uio_iovcnt = 1;
-				uio.uio_offset = (off_t)(long)va;
-				uio.uio_resid = sizeof(int);
-				uio.uio_segflg = UIO_SYSSPACE;
-				uio.uio_rw = UIO_WRITE;
-				uio.uio_procp = curproc;
-				error = process_domem(curproc, p, &uio,
-				    PT_WRITE_I);
-				Mips_SyncCache(curcpu());
-
-				if (error)
-					printf("Warning: can't restore instruction at %x: %x\n",
-					    p->p_md.md_ss_addr,
-					    p->p_md.md_ss_instr);
-
-				p->p_md.md_ss_addr = 0;
+#ifdef DEBUG
+				printf("trap: %s (%d): breakpoint at %p "
+				    "(insn %08x)\n",
+				    p->p_comm, p->p_pid,
+				    p->p_md.md_ss_addr, p->p_md.md_ss_instr);
+#endif
+
+				/* Restore original instruction and clear BP */
+				process_sstep(p, 0);
 				typ = TRAP_BRKPT;
 			} else {
 				typ = TRAP_TRACE;
@@ -1012,87 +989,112 @@ MipsEmulateBranch(struct trap_frame *tf, vaddr_t instPC, uint32_t fsr,
 
 #ifdef PTRACE
 
-/*
- * This routine is called by procxmt() to single step one instruction.
- * We do this by storing a break instruction after the current instruction,
- * resuming execution, and then restoring the old instruction.
- */
 int
-cpu_singlestep(p)
-	struct proc *p;
+ptrace_read_insn(struct proc *p, vaddr_t va, uint32_t *insn)
 {
-	vaddr_t va;
-	struct trap_frame *locr0 = p->p_md.md_regs;
-	int error;
-	int bpinstr = BREAK_SSTEP;
-	uint32_t curinstr;
-	struct uio uio;
 	struct iovec iov;
+	struct uio uio;
 
-	/*
-	 * Fetch what's at the current location.
-	 */
-	iov.iov_base = (caddr_t)&curinstr;
+	iov.iov_base = (caddr_t)insn;
 	iov.iov_len = sizeof(uint32_t);
 	uio.uio_iov = &iov;
 	uio.uio_iovcnt = 1;
-	uio.uio_offset = (off_t)locr0->pc;
+	uio.uio_offset = (off_t)va;
 	uio.uio_resid = sizeof(uint32_t);
 	uio.uio_segflg = UIO_SYSSPACE;
 	uio.uio_rw = UIO_READ;
-	uio.uio_procp = curproc;
-	process_domem(curproc, p, &uio, PT_READ_I);
+	uio.uio_procp = p;
+	return process_domem(p, p, &uio, PT_READ_I);
+}
+
+int
+ptrace_write_insn(struct proc *p, vaddr_t va, uint32_t insn)
+{
+	struct iovec iov;
+	struct uio uio;
+
+	iov.iov_base = (caddr_t)&insn;
+	iov.iov_len = sizeof(uint32_t);
+	uio.uio_iov = &iov;
+	uio.uio_iovcnt = 1;
+	uio.uio_offset = (off_t)va;
+	uio.uio_resid = sizeof(uint32_t);
+	uio.uio_segflg = UIO_SYSSPACE;
+	uio.uio_rw = UIO_WRITE;
+	uio.uio_procp = p;
+	return process_domem(p, p, &uio, PT_WRITE_I);
+}
+
+/*
+ * This routine is called by procxmt() to single step one instruction.
+ * We do this by storing a break instruction after the current instruction,
+ * resuming execution, and then restoring the old instruction.
+ */
+int
+process_sstep(struct proc *p, int sstep)
+{
+	struct trap_frame *locr0 = p->p_md.md_regs;
+	int rc;
+	uint32_t curinstr;
+	vaddr_t va;
+
+	if (sstep == 0) {
+		/* clear the breakpoint */
+		if (p->p_md.md_ss_addr != 0) {
+			rc = ptrace_write_insn(p, p->p_md.md_ss_addr,
+			    p->p_md.md_ss_instr);
+#ifdef DIAGNOSTIC
+			if (rc != 0)
+				printf("WARNING: %s (%d): can't restore "
+				    "instruction at %p: %08x\n",
+				    p->p_comm, p->p_pid,
+				    p->p_md.md_ss_addr, p->p_md.md_ss_instr);
+#endif
+			p->p_md.md_ss_addr = 0;
+		} else
+			rc = 0;
+		return rc;
+	}
+
+	/* read current instruction */
+	rc = ptrace_read_insn(p, locr0->pc, &curinstr);
+	if (rc != 0)
+		return rc;
 
 	/* compute next address after current location */
-	if (curinstr != 0)
+	if (curinstr != 0 /* nop */)
 		va = (vaddr_t)MipsEmulateBranch(locr0,
 		    locr0->pc, locr0->fsr, curinstr);
 	else
 		va = locr0->pc + 4;
-	if (p->p_md.md_ss_addr) {
-		printf("SS %s (%d): breakpoint already set at %x (va %x)\n",
-			p->p_comm, p->p_pid, p->p_md.md_ss_addr, va); /* XXX */
-		return (EFAULT);
+#ifdef DIAGNOSTIC
+	/* should not happen */
+	if (p->p_md.md_ss_addr != 0) {
+		printf("WARNING: %s (%d): breakpoint request "
+		    "at %p, already set at %p\n",
+		    p->p_comm, p->p_pid, va, p->p_md.md_ss_addr);
+		return EFAULT;
 	}
+#endif
 
-	/*
-	 * Fetch what's at the current location.
-	 */
-	iov.iov_base = (caddr_t)&p->p_md.md_ss_instr;
-	iov.iov_len = sizeof(int);
-	uio.uio_iov = &iov;
-	uio.uio_iovcnt = 1;
-	uio.uio_offset = (off_t)va;
-	uio.uio_resid = sizeof(int);
-	uio.uio_segflg = UIO_SYSSPACE;
-	uio.uio_rw = UIO_READ;
-	uio.uio_procp = curproc;
-	process_domem(curproc, p, &uio, PT_READ_I);
+	/* read next instruction */
+	rc = ptrace_read_insn(p, va, &p->p_md.md_ss_instr);
+	if (rc != 0)
+		return rc;
 
-	/*
-	 * Store breakpoint instruction at the "next" location now.
-	 */
-	iov.iov_base = (caddr_t)&bpinstr;
-	iov.iov_len = sizeof(int);
-	uio.uio_iov = &iov;
-	uio.uio_iovcnt = 1;
-	uio.uio_offset = (off_t)va;
-	uio.uio_resid = sizeof(int);
-	uio.uio_segflg = UIO_SYSSPACE;
-	uio.uio_rw = UIO_WRITE;
-	uio.uio_procp = curproc;
-	error = process_domem(curproc, p, &uio, PT_WRITE_I);
-	Mips_SyncCache(curcpu());
-	if (error)
-		return (EFAULT);
+	/* replace with a breakpoint instruction */
+	rc = ptrace_write_insn(p, va, BREAK_SSTEP);
+	if (rc != 0)
+		return rc;
 
 	p->p_md.md_ss_addr = va;
-#if 0
-	printf("SS %s (%d): breakpoint set at %x: %x (pc %x) br %x\n",
-		p->p_comm, p->p_pid, p->p_md.md_ss_addr,
-		p->p_md.md_ss_instr, locr0[PC], curinstr); /* XXX */
+
+#ifdef DEBUG
+	printf("%s (%d): breakpoint set at %p: %08x (pc %p %08x)\n",
+		p->p_comm, p->p_pid,
+		p->p_md.md_ss_addr, p->p_md.md_ss_instr, locr0->pc, curinstr);
 #endif
-	return (0);
+	return 0;
 }
 
 #endif /* PTRACE */