From owner-svn-src-head@freebsd.org Tue Apr 24 17:53:17 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 90005FAD899; Tue, 24 Apr 2018 17:53:17 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 37180840F1; Tue, 24 Apr 2018 17:53:17 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 31A571672A; Tue, 24 Apr 2018 17:53:17 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w3OHrHv8052859; Tue, 24 Apr 2018 17:53:17 GMT (envelope-from jhb@FreeBSD.org) Received: (from jhb@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w3OHrGri052856; Tue, 24 Apr 2018 17:53:16 GMT (envelope-from jhb@FreeBSD.org) Message-Id: <201804241753.w3OHrGri052856@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: jhb set sender to jhb@FreeBSD.org using -f From: John Baldwin Date: Tue, 24 Apr 2018 17:53:16 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r332951 - in head/sys/mips: include mips X-SVN-Group: head X-SVN-Commit-Author: jhb X-SVN-Commit-Paths: in head/sys/mips: include mips X-SVN-Commit-Revision: 332951 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Apr 2018 17:53:17 -0000 Author: jhb Date: Tue Apr 24 17:53:16 2018 New Revision: 332951 URL: https://svnweb.freebsd.org/changeset/base/332951 Log: Fix PT_STEP single-stepping for mips. Note that GDB at least implements single stepping for MIPS using software breakpoints explicitly rather than using PT_STEP, so this has only been tested via tests in ptrace_test which now pass rather than fail. - Fix several places to use uintptr_t instead of int for virtual addresses. - Check for errors from ptrace_read_int() when setting a breakpoint for a step. - Properly check for errors from ptrace_write_int() as it returns non-zero, not negative values on failure. - Change the error returns for ptrace_read_int() and ptrace_write_int() from ENOMEM to EFAULT. - Clear a single step breakpoint when it traps rather than waiting for it to be cleared from ptrace(). This matches the behavior of the arm port and in general seems a bit more reliable than waiting for ptrace() to clear it via FIX_SSTEP. - Drop the PROC_LOCK around ptrace_write_int() in ptrace_clear_single_step() since it can sleep. - Reorder the breakpoint handler in trap() to only read the instruction if the address matches the current thread's breakpoint address. - Replace various #if 0'd debugging printfs with KTR_PTRACE traces. Tested on: mips64 Modified: head/sys/mips/include/proc.h head/sys/mips/mips/pm_machdep.c head/sys/mips/mips/trap.c Modified: head/sys/mips/include/proc.h ============================================================================== --- head/sys/mips/include/proc.h Tue Apr 24 17:46:33 2018 (r332950) +++ head/sys/mips/include/proc.h Tue Apr 24 17:53:16 2018 (r332951) @@ -55,7 +55,7 @@ struct mdthread { #else int md_upte[KSTACK_PAGES]; #endif - int md_ss_addr; /* single step address for ptrace */ + uintptr_t md_ss_addr; /* single step address for ptrace */ int md_ss_instr; /* single step instruction for ptrace */ register_t md_saved_intr; u_int md_spinlock_count; Modified: head/sys/mips/mips/pm_machdep.c ============================================================================== --- head/sys/mips/mips/pm_machdep.c Tue Apr 24 17:46:33 2018 (r332950) +++ head/sys/mips/mips/pm_machdep.c Tue Apr 24 17:53:16 2018 (r332951) @@ -216,29 +216,29 @@ ptrace_set_pc(struct thread *td, unsigned long addr) } static int -ptrace_read_int(struct thread *td, off_t addr, int *v) +ptrace_read_int(struct thread *td, uintptr_t addr, int *v) { if (proc_readmem(td, td->td_proc, addr, v, sizeof(*v)) != sizeof(*v)) - return (ENOMEM); + return (EFAULT); return (0); } static int -ptrace_write_int(struct thread *td, off_t addr, int v) +ptrace_write_int(struct thread *td, uintptr_t addr, int v) { if (proc_writemem(td, td->td_proc, addr, &v, sizeof(v)) != sizeof(v)) - return (ENOMEM); + return (EFAULT); return (0); } int ptrace_single_step(struct thread *td) { - unsigned va; + uintptr_t va; struct trapframe *locr0 = td->td_frame; - int i; + int error; int bpinstr = MIPS_BREAK_SSTEP; int curinstr; struct proc *p; @@ -248,46 +248,52 @@ ptrace_single_step(struct thread *td) /* * Fetch what's at the current location. */ - ptrace_read_int(td, (off_t)locr0->pc, &curinstr); + error = ptrace_read_int(td, locr0->pc, &curinstr); + if (error) + goto out; + CTR3(KTR_PTRACE, + "ptrace_single_step: tid %d, current instr at %#lx: %#08x", + td->td_tid, locr0->pc, curinstr); + /* compute next address after current location */ - if(curinstr != 0) { + if (locr0->cause & MIPS_CR_BR_DELAY) { va = MipsEmulateBranch(locr0, locr0->pc, locr0->fsr, (uintptr_t)&curinstr); } else { va = locr0->pc + 4; } if (td->td_md.md_ss_addr) { - printf("SS %s (%d): breakpoint already set at %x (va %x)\n", + printf("SS %s (%d): breakpoint already set at %lx (va %lx)\n", p->p_comm, p->p_pid, td->td_md.md_ss_addr, va); /* XXX */ - PROC_LOCK(p); - return (EFAULT); + error = EFAULT; + goto out; } td->td_md.md_ss_addr = va; /* * Fetch what's at the current location. */ - ptrace_read_int(td, (off_t)va, &td->td_md.md_ss_instr); + error = ptrace_read_int(td, (off_t)va, &td->td_md.md_ss_instr); + if (error) + goto out; /* * Store breakpoint instruction at the "next" location now. */ - i = ptrace_write_int (td, va, bpinstr); + error = ptrace_write_int(td, va, bpinstr); /* - * The sync'ing of I & D caches is done by procfs_domem() - * through procfs_rwmem(). + * The sync'ing of I & D caches is done by proc_rwmem() + * through proc_writemem(). */ +out: PROC_LOCK(p); - if (i < 0) - return (EFAULT); -#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 */ -#endif - return (0); + if (error == 0) + CTR3(KTR_PTRACE, + "ptrace_single_step: tid %d, break set at %#lx: (%#08x)", + td->td_tid, va, td->td_md.md_ss_instr); + return (error); } @@ -471,8 +477,8 @@ exec_setregs(struct thread *td, struct image_params *i int ptrace_clear_single_step(struct thread *td) { - int i; struct proc *p; + int error; p = td->td_proc; PROC_LOCK_ASSERT(p, MA_OWNED); @@ -482,12 +488,19 @@ ptrace_clear_single_step(struct thread *td) /* * Restore original instruction and clear BP */ - i = ptrace_write_int (td, td->td_md.md_ss_addr, td->td_md.md_ss_instr); + PROC_UNLOCK(p); + CTR3(KTR_PTRACE, + "ptrace_clear_single_step: tid %d, restore instr at %#lx: %#08x", + td->td_tid, td->td_md.md_ss_addr, td->td_md.md_ss_instr); + error = ptrace_write_int(td, td->td_md.md_ss_addr, + td->td_md.md_ss_instr); + PROC_LOCK(p); - /* The sync'ing of I & D caches is done by procfs_domem(). */ + /* The sync'ing of I & D caches is done by proc_rwmem(). */ - if (i < 0) { - log(LOG_ERR, "SS %s %d: can't restore instruction at %x: %x\n", + if (error != 0) { + log(LOG_ERR, + "SS %s %d: can't restore instruction at %lx: %x\n", p->p_comm, p->p_pid, td->td_md.md_ss_addr, td->td_md.md_ss_instr); } Modified: head/sys/mips/mips/trap.c ============================================================================== --- head/sys/mips/mips/trap.c Tue Apr 24 17:46:33 2018 (r332950) +++ head/sys/mips/mips/trap.c Tue Apr 24 17:53:16 2018 (r332951) @@ -526,7 +526,7 @@ trap(struct trapframe *trapframe) char *msg = NULL; intptr_t addr = 0; register_t pc; - int cop; + int cop, error; register_t *frame_regs; trapdebug_enter(trapframe, 0); @@ -825,34 +825,34 @@ dofault: intptr_t va; uint32_t instr; + i = SIGTRAP; + ucode = TRAP_BRKPT; + addr = trapframe->pc; + /* compute address of break instruction */ va = trapframe->pc; if (DELAYBRANCH(trapframe->cause)) va += sizeof(int); + if (td->td_md.md_ss_addr != va) + break; + /* read break instruction */ instr = fuword32((caddr_t)va); -#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 - if (td->td_md.md_ss_addr != va || - instr != MIPS_BREAK_SSTEP) { - i = SIGTRAP; - ucode = TRAP_BRKPT; - addr = trapframe->pc; + + if (instr != MIPS_BREAK_SSTEP) break; - } - /* - * The restoration of the original instruction and - * the clearing of the breakpoint will be done later - * by the call to ptrace_clear_single_step() in - * issignal() when SIGTRAP is processed. - */ - addr = trapframe->pc; - i = SIGTRAP; - ucode = TRAP_TRACE; + + CTR3(KTR_PTRACE, + "trap: tid %d, single step at %#lx: %#08x", + td->td_tid, va, instr); + PROC_LOCK(p); + _PHOLD(p); + error = ptrace_clear_single_step(td); + _PRELE(p); + PROC_UNLOCK(p); + if (error == 0) + ucode = TRAP_TRACE; break; }