Author: jhb
Date: Fri Oct 27 03:16:19 2017
New Revision: 325028
URL: https://svnweb.freebsd.org/changeset/base/325028

Log:
  Discard the correct thread event reported for a ptrace stop.
  
  When multiple threads wish to report a tracing event to a debugger,
  both threads call ptracestop() and one thread will win the race to be
  the reporting thread (p->p_xthread).  The debugger uses PT_LWPINFO
  with the process ID to determine which thread / LWP is reporting an
  event and the details of that event.  This event is cleared as a side
  effect of the subsequent ptrace event that resumed the process
  (PT_CONTINUE, PT_STEP, etc.).  However, ptrace() was clearing the
  event identified by the LWP ID passed to the resume request even if
  that wasn't the 'p_xthread'.  This could result in clearing an event
  that had not yet been observed by the debugger and leaving the
  existing event for 'p_thread' pending so that it was reported a second
  time.
  
  Specifically, if the debugger stopped due to a software breakpoint in
  one thread, but then switched to another thread that was used to
  resume (e.g. if the user switched to a different thread and issued a
  step), the resume request (PT_STEP) cleared a pending event (if any)
  for the thread being stepped.  However, the process immediately
  stopped and the first thread reported it's breakpoint event a second
  time.  The debugger decremented the PC for "both" breakpoint events
  which resulted in the PC now pointing into the middle of an
  instruction (on x86) and a SIGILL fault when the process was resumed a
  second time.
  
  To fix, always clear the pending event for 'p_xthread' when resuming a
  process.  ptrace() still honors the requested LWP ID when enabling
  single-stepping (PT_STEP) or setting a different PC (PT_CONTINUE).
  
  Reported by:  GDB testsuite (gdb.threads/continue-pending-status.exp)
  Reviewed by:  kib
  MFC after:    1 week
  Differential Revision:        https://reviews.freebsd.org/D12794

Modified:
  head/sys/kern/sys_process.c
  head/sys/sys/param.h
  head/tests/sys/kern/ptrace_test.c

Modified: head/sys/kern/sys_process.c
==============================================================================
--- head/sys/kern/sys_process.c Thu Oct 26 22:53:55 2017        (r325027)
+++ head/sys/kern/sys_process.c Fri Oct 27 03:16:19 2017        (r325028)
@@ -1129,6 +1129,13 @@ kern_ptrace(struct thread *td, int req, pid_t pid, voi
                }
 
        sendsig:
+               /*
+                * Clear the pending event for the thread that just
+                * reported its event (p_xthread).  This may not be
+                * the thread passed to PT_CONTINUE, PT_STEP, etc. if
+                * the debugger is resuming a different thread.
+                */
+               td2 = p->p_xthread;
                if (proctree_locked) {
                        sx_xunlock(&proctree_lock);
                        proctree_locked = 0;

Modified: head/sys/sys/param.h
==============================================================================
--- head/sys/sys/param.h        Thu Oct 26 22:53:55 2017        (r325027)
+++ head/sys/sys/param.h        Fri Oct 27 03:16:19 2017        (r325028)
@@ -58,7 +58,7 @@
  *             in the range 5 to 9.
  */
 #undef __FreeBSD_version
-#define __FreeBSD_version 1200051      /* Master, propagated to newvers */
+#define __FreeBSD_version 1200052      /* Master, propagated to newvers */
 
 /*
  * __FreeBSD_kernel__ indicates that this system uses the kernel of FreeBSD,

Modified: head/tests/sys/kern/ptrace_test.c
==============================================================================
--- head/tests/sys/kern/ptrace_test.c   Thu Oct 26 22:53:55 2017        
(r325027)
+++ head/tests/sys/kern/ptrace_test.c   Fri Oct 27 03:16:19 2017        
(r325028)
@@ -3463,6 +3463,174 @@ ATF_TC_BODY(ptrace__PT_STEP_with_signal, tc)
        ATF_REQUIRE(errno == ECHILD);
 }
 
+#if defined(__amd64__) || defined(__i386__)
+/*
+ * Only x86 both define breakpoint() and have a PC after breakpoint so
+ * that restarting doesn't retrigger the breakpoint.
+ */
+static void *
+continue_thread(void *arg)
+{
+       breakpoint();
+       return (NULL);
+}
+
+static __dead2 void
+continue_thread_main(void)
+{
+       pthread_t threads[2];
+
+       CHILD_REQUIRE(pthread_create(&threads[0], NULL, continue_thread,
+           NULL) == 0);
+       CHILD_REQUIRE(pthread_create(&threads[1], NULL, continue_thread,
+           NULL) == 0);
+       CHILD_REQUIRE(pthread_join(threads[0], NULL) == 0);
+       CHILD_REQUIRE(pthread_join(threads[1], NULL) == 0);
+       exit(1);
+}
+
+/*
+ * Ensure that PT_CONTINUE clears the status of the thread that
+ * triggered the stop even if a different thread's LWP was passed to
+ * PT_CONTINUE.
+ */
+ATF_TC_WITHOUT_HEAD(ptrace__PT_CONTINUE_different_thread);
+ATF_TC_BODY(ptrace__PT_CONTINUE_different_thread, tc)
+{
+       struct ptrace_lwpinfo pl;
+       pid_t fpid, wpid;
+       lwpid_t lwps[2];
+       bool hit_break[2];
+       int i, j, status;
+
+       ATF_REQUIRE((fpid = fork()) != -1);
+       if (fpid == 0) {
+               trace_me();
+               continue_thread_main();
+       }
+
+       /* The first wait() should report the stop from SIGSTOP. */
+       wpid = waitpid(fpid, &status, 0);
+       ATF_REQUIRE(wpid == fpid);
+       ATF_REQUIRE(WIFSTOPPED(status));
+       ATF_REQUIRE(WSTOPSIG(status) == SIGSTOP);
+
+       ATF_REQUIRE(ptrace(PT_LWPINFO, wpid, (caddr_t)&pl,
+           sizeof(pl)) != -1);
+
+       ATF_REQUIRE(ptrace(PT_LWP_EVENTS, wpid, NULL, 1) == 0);
+
+       /* Continue the child ignoring the SIGSTOP. */
+       ATF_REQUIRE(ptrace(PT_CONTINUE, fpid, (caddr_t)1, 0) == 0);
+
+       /* One of the new threads should report it's birth. */
+       wpid = waitpid(fpid, &status, 0);
+       ATF_REQUIRE(wpid == fpid);
+       ATF_REQUIRE(WIFSTOPPED(status));
+       ATF_REQUIRE(WSTOPSIG(status) == SIGTRAP);
+
+       ATF_REQUIRE(ptrace(PT_LWPINFO, wpid, (caddr_t)&pl, sizeof(pl)) != -1);
+       ATF_REQUIRE((pl.pl_flags & (PL_FLAG_BORN | PL_FLAG_SCX)) ==
+           (PL_FLAG_BORN | PL_FLAG_SCX));
+       lwps[0] = pl.pl_lwpid;
+
+       /*
+        * Suspend this thread to ensure both threads are alive before
+        * hitting the breakpoint.
+        */
+       ATF_REQUIRE(ptrace(PT_SUSPEND, lwps[0], NULL, 0) != -1);
+
+       ATF_REQUIRE(ptrace(PT_CONTINUE, fpid, (caddr_t)1, 0) == 0);
+
+       /* Second thread should report it's birth. */
+       wpid = waitpid(fpid, &status, 0);
+       ATF_REQUIRE(wpid == fpid);
+       ATF_REQUIRE(WIFSTOPPED(status));
+       ATF_REQUIRE(WSTOPSIG(status) == SIGTRAP);
+
+       ATF_REQUIRE(ptrace(PT_LWPINFO, wpid, (caddr_t)&pl, sizeof(pl)) != -1);
+       ATF_REQUIRE((pl.pl_flags & (PL_FLAG_BORN | PL_FLAG_SCX)) ==
+           (PL_FLAG_BORN | PL_FLAG_SCX));
+       ATF_REQUIRE(pl.pl_lwpid != lwps[0]);
+       lwps[1] = pl.pl_lwpid;
+
+       /* Resume both threads waiting for breakpoint events. */
+       hit_break[0] = hit_break[1] = false;
+       ATF_REQUIRE(ptrace(PT_RESUME, lwps[0], NULL, 0) != -1);
+       ATF_REQUIRE(ptrace(PT_CONTINUE, fpid, (caddr_t)1, 0) == 0);
+
+       /* One thread should report a breakpoint. */
+       wpid = waitpid(fpid, &status, 0);
+       ATF_REQUIRE(wpid == fpid);
+       ATF_REQUIRE(WIFSTOPPED(status));
+       ATF_REQUIRE(WSTOPSIG(status) == SIGTRAP);
+
+       ATF_REQUIRE(ptrace(PT_LWPINFO, wpid, (caddr_t)&pl, sizeof(pl)) != -1);
+       ATF_REQUIRE((pl.pl_flags & PL_FLAG_SI) != 0);
+       ATF_REQUIRE(pl.pl_siginfo.si_signo == SIGTRAP &&
+           pl.pl_siginfo.si_code == TRAP_BRKPT);
+       if (pl.pl_lwpid == lwps[0])
+               i = 0;
+       else
+               i = 1;
+       hit_break[i] = true;
+
+       /*
+        * Resume both threads but pass the other thread's LWPID to
+        * PT_CONTINUE.
+        */
+       ATF_REQUIRE(ptrace(PT_CONTINUE, lwps[i ^ 1], (caddr_t)1, 0) == 0);
+
+       /*
+        * Will now get two thread exit events and one more breakpoint
+        * event.
+        */
+       for (j = 0; j < 3; j++) {
+               wpid = waitpid(fpid, &status, 0);
+               ATF_REQUIRE(wpid == fpid);
+               ATF_REQUIRE(WIFSTOPPED(status));
+               ATF_REQUIRE(WSTOPSIG(status) == SIGTRAP);
+
+               ATF_REQUIRE(ptrace(PT_LWPINFO, wpid, (caddr_t)&pl,
+                   sizeof(pl)) != -1);
+               
+               if (pl.pl_lwpid == lwps[0])
+                       i = 0;
+               else
+                       i = 1;
+
+               ATF_REQUIRE_MSG(lwps[i] != 0, "event for exited thread");
+               if (pl.pl_flags & PL_FLAG_EXITED) {
+                       ATF_REQUIRE_MSG(hit_break[i],
+                           "exited thread did not report breakpoint");
+                       lwps[i] = 0;
+               } else {
+                       ATF_REQUIRE((pl.pl_flags & PL_FLAG_SI) != 0);
+                       ATF_REQUIRE(pl.pl_siginfo.si_signo == SIGTRAP &&
+                           pl.pl_siginfo.si_code == TRAP_BRKPT);
+                       ATF_REQUIRE_MSG(!hit_break[i],
+                           "double breakpoint event");
+                       hit_break[i] = true;
+               }
+
+               ATF_REQUIRE(ptrace(PT_CONTINUE, fpid, (caddr_t)1, 0) == 0);
+       }
+
+       /* Both threads should have exited. */
+       ATF_REQUIRE(lwps[0] == 0);
+       ATF_REQUIRE(lwps[1] == 0);
+
+       /* The last event should be for the child process's exit. */
+       wpid = waitpid(fpid, &status, 0);
+       ATF_REQUIRE(WIFEXITED(status));
+       ATF_REQUIRE(WEXITSTATUS(status) == 1);
+
+       wpid = wait(&status);
+       ATF_REQUIRE(wpid == -1);
+       ATF_REQUIRE(errno == ECHILD);
+}
+#endif
+
 ATF_TP_ADD_TCS(tp)
 {
 
@@ -3515,6 +3683,9 @@ ATF_TP_ADD_TCS(tp)
        ATF_TP_ADD_TC(tp, ptrace__event_mask_sigkill_discard);
        ATF_TP_ADD_TC(tp, ptrace__PT_ATTACH_with_SBDRY_thread);
        ATF_TP_ADD_TC(tp, ptrace__PT_STEP_with_signal);
+#if defined(__amd64__) || defined(__i386__)
+       ATF_TP_ADD_TC(tp, ptrace__PT_CONTINUE_different_thread);
+#endif
 
        return (atf_no_error());
 }
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to