Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 13 Nov 2017 21:09:09 +0000 (UTC)
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r325777 - head/sys/kern
Message-ID:  <201711132109.vADL99GU040431@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: jhb
Date: Mon Nov 13 21:09:08 2017
New Revision: 325777
URL: https://svnweb.freebsd.org/changeset/base/325777

Log:
  Pull the PT_ATTACH case out of the 'sendsig:' block.
  
  Most of the conditionals in the 'sendsig:' block are now only different
  for PT_ATTACH vs other continue requests.  Pull the PT_ATTACH-specific
  logic up into the PT_ATTACH case and simplify the 'sendsig:' block.  This
  also permits moving the unlock of proctree_lock above the sendsig: label
  since PT_KILL doesn't hold the lock and and the other cases all fall
  through to the label.
  
  Reviewed by:	kib
  Differential Revision:	https://reviews.freebsd.org/D13073

Modified:
  head/sys/kern/sys_process.c

Modified: head/sys/kern/sys_process.c
==============================================================================
--- head/sys/kern/sys_process.c	Mon Nov 13 20:49:08 2017	(r325776)
+++ head/sys/kern/sys_process.c	Mon Nov 13 21:09:08 2017	(r325777)
@@ -920,11 +920,28 @@ kern_ptrace(struct thread *td, int req, pid_t pid, voi
 		if (p->p_pptr != td->td_proc) {
 			proc_reparent(p, td->td_proc);
 		}
-		data = SIGSTOP;
 		CTR2(KTR_PTRACE, "PT_ATTACH: pid %d, oppid %d", p->p_pid,
 		    p->p_oppid);
-		goto sendsig;	/* in PT_CONTINUE below */
 
+		sx_xunlock(&proctree_lock);
+		proctree_locked = 0;
+		MPASS(p->p_xthread == NULL);
+		MPASS((p->p_flag & P_STOPPED_TRACE) == 0);
+
+		/*
+		 * If already stopped due to a stop signal, clear the
+		 * existing stop before triggering a traced SIGSTOP.
+		 */
+		if ((p->p_flag & P_STOPPED_SIG) != 0) {
+			PROC_SLOCK(p);
+			p->p_flag &= ~(P_STOPPED_SIG | P_WAITED);
+			thread_unsuspend(p);
+			PROC_SUNLOCK(p);
+		}
+
+		kern_psignal(p, SIGSTOP);
+		break;
+
 	case PT_CLEARSTEP:
 		CTR2(KTR_PTRACE, "PT_CLEARSTEP: tid %d (pid %d)", td2->td_tid,
 		    p->p_pid);
@@ -1123,12 +1140,11 @@ kern_ptrace(struct thread *td, int req, pid_t pid, voi
 			break;
 		}
 
+		sx_xunlock(&proctree_lock);
+		proctree_locked = 0;
+
 	sendsig:
-		/* proctree_locked is true for all but PT_KILL. */
-		if (proctree_locked) {
-			sx_xunlock(&proctree_lock);
-			proctree_locked = 0;
-		}
+		MPASS(proctree_locked == 0);
 		
 		/* 
 		 * Clear the pending event for the thread that just
@@ -1138,54 +1154,36 @@ kern_ptrace(struct thread *td, int req, pid_t pid, voi
 		 *
 		 * Deliver any pending signal via the reporting thread.
 		 */
-		if ((p->p_flag & P_STOPPED_TRACE) != 0) {
-			MPASS(p->p_xthread != NULL);
-			p->p_xthread->td_dbgflags &= ~TDB_XSIG;
-			p->p_xthread->td_xsig = data;
-			p->p_xthread = NULL;
-			p->p_xsig = data;
-		} else {
-			MPASS(p->p_xthread == NULL);
-			MPASS(req == PT_ATTACH);
-		}
+		MPASS(p->p_xthread != NULL);
+		p->p_xthread->td_dbgflags &= ~TDB_XSIG;
+		p->p_xthread->td_xsig = data;
+		p->p_xthread = NULL;
+		p->p_xsig = data;
 
-		if ((p->p_flag & (P_STOPPED_SIG | P_STOPPED_TRACE)) != 0) {
-			/*
-			 * P_WKILLED is insurance that a PT_KILL/SIGKILL always
-			 * works immediately, even if another thread is
-			 * unsuspended first and attempts to handle a different
-			 * signal or if the POSIX.1b style signal queue cannot
-			 * accommodate any new signals.
-			 */
-			if (data == SIGKILL)
-				p->p_flag |= P_WKILLED;
+		/*
+		 * P_WKILLED is insurance that a PT_KILL/SIGKILL
+		 * always works immediately, even if another thread is
+		 * unsuspended first and attempts to handle a
+		 * different signal or if the POSIX.1b style signal
+		 * queue cannot accommodate any new signals.
+		 */
+		if (data == SIGKILL)
+			p->p_flag |= P_WKILLED;
 
-			if (req == PT_DETACH) {
-				FOREACH_THREAD_IN_PROC(p, td3)
-					td3->td_dbgflags &= ~TDB_SUSPEND;
-			}
-
-			/*
-			 * Unsuspend all threads.  To leave a thread
-			 * suspended, use PT_SUSPEND to suspend it
-			 * before continuing the process.
-			 */
-			PROC_SLOCK(p);
-			p->p_flag &= ~(P_STOPPED_TRACE|P_STOPPED_SIG|P_WAITED);
-			thread_unsuspend(p);
-			PROC_SUNLOCK(p);
+		if (req == PT_DETACH) {
+			FOREACH_THREAD_IN_PROC(p, td3)
+			    td3->td_dbgflags &= ~TDB_SUSPEND;
 		}
 
 		/*
-		 * For requests other than PT_ATTACH, P_STOPPED_TRACE
-		 * was set, so any pending signal in 'data' is
-		 * delivered via the reporting thread (p_xthread).
-		 * For PT_ATTACH the process is not yet stopped for
-		 * tracing, so P_STOPPED_TRACE cannot be set and the
-		 * SIGSTOP must be delivered to the process.
+		 * Unsuspend all threads.  To leave a thread
+		 * suspended, use PT_SUSPEND to suspend it before
+		 * continuing the process.
 		 */
-		if (req == PT_ATTACH)
-			kern_psignal(p, data);
+		PROC_SLOCK(p);
+		p->p_flag &= ~(P_STOPPED_TRACE | P_STOPPED_SIG | P_WAITED);
+		thread_unsuspend(p);
+		PROC_SUNLOCK(p);
 		break;
 
 	case PT_WRITE_I:

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201711132109.vADL99GU040431>