Re: [rfc] linux-nat: Never PTRACE_CONT a stepping thread

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Tue, 19 Oct 2010 21:24:04 +0200, Pedro Alves wrote:
> What I actually remembered was that using PTRACE_SINGLESTEP,
> and leaving lwp->step true on software single-step archs
> probably isn't a good idea.  And it crossed my mind that we'd
> probably need to make infrun.c know it needs to insert
> software single-step breakpoint in this case too.

OK, I see now the software singlestep is _above_ this linux-nat.c patched
code, not _below_.

Therefore I have reverted it now as it was at a wrong place as the new patch
will need to revert this one anyway.


> I'm also wondering if tweaking the test to use a signal less than
> SIGTRAP (5) rather than SIGUSR1 (10) so that waitpid always
> picks it up over SIGTRAP if both are pending simulateneously
> reveals that we need to rethink this?

OK.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2010-11/msg00005.html

--- src/gdb/ChangeLog	2010/11/01 07:00:09	1.12283
+++ src/gdb/ChangeLog	2010/11/02 01:37:30	1.12284
@@ -1,3 +1,20 @@
+2010-11-02  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	Revert:
+	2010-10-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
+		    Pedro Alves  <pedro@codesourcery.com>
+	* gdbthread.h (currently_stepping): New declaration.
+	* infrun.c (currently_stepping): Remove the forward declaration.
+	(currently_stepping): Make it global.
+	* linux-nat.c (resume_callback) <lp->stopped && lp->status == 0>: New
+	variables tp and step, initialized them.  Pass STEP to to_resume.
+	Print also possibly "PTRACE_SINGLESTEP" if STEP.  Initialize LP->STEP.
+	* remote.c (currently_stepping_callback): New.
+	(remote_vcont_resume)
+	<ptid_equal (ptid, minus_one_ptid) || ptid_is_pid (ptid)>:
+	New variable tp.  Call currently_stepping_callback and step such
+	thread.
+
 2010-11-01  Hui Zhu  <teawater@gmail.com>
 
 	* tracepoint.c (tfile_xfer_partial): Change lma to vma.
--- src/gdb/gdbthread.h	2010/10/17 18:24:47	1.55
+++ src/gdb/gdbthread.h	2010/11/02 01:37:31	1.56
@@ -352,6 +352,4 @@
 
 extern void update_thread_list (void);
 
-extern int currently_stepping (struct thread_info *tp);
-
 #endif /* GDBTHREAD_H */
--- src/gdb/infrun.c	2010/10/17 18:24:47	1.453
+++ src/gdb/infrun.c	2010/11/02 01:37:31	1.454
@@ -74,6 +74,8 @@
 static void set_schedlock_func (char *args, int from_tty,
 				struct cmd_list_element *c);
 
+static int currently_stepping (struct thread_info *tp);
+
 static int currently_stepping_or_nexting_callback (struct thread_info *tp,
 						   void *data);
 
@@ -4849,7 +4851,7 @@
 
 /* Is thread TP in the middle of single-stepping?  */
 
-int
+static int
 currently_stepping (struct thread_info *tp)
 {
   return ((tp->step_range_end && tp->step_resume_breakpoint == NULL)
--- src/gdb/linux-nat.c	2010/10/17 18:24:47	1.185
+++ src/gdb/linux-nat.c	2010/11/02 01:37:32	1.186
@@ -1820,26 +1820,20 @@
     }
   else if (lp->stopped && lp->status == 0)
     {
-      struct thread_info *tp = find_thread_ptid (lp->ptid);
-      /* lp->step may already contain a stale value.  */
-      int step = tp ? currently_stepping (tp) : 0;
-
       if (debug_linux_nat)
 	fprintf_unfiltered (gdb_stdlog,
-			    "RC:  %s %s, 0, 0 (resuming sibling)\n",
-			    step ? "PTRACE_SINGLESTEP" : "PTRACE_CONT",
+			    "RC:  PTRACE_CONT %s, 0, 0 (resuming sibling)\n",
 			    target_pid_to_str (lp->ptid));
 
       linux_ops->to_resume (linux_ops,
 			    pid_to_ptid (GET_LWP (lp->ptid)),
-			    step, TARGET_SIGNAL_0);
+			    0, TARGET_SIGNAL_0);
       if (debug_linux_nat)
 	fprintf_unfiltered (gdb_stdlog,
-			    "RC:  %s %s, 0, 0 (resume sibling)\n",
-			    step ? "PTRACE_SINGLESTEP" : "PTRACE_CONT",
+			    "RC:  PTRACE_CONT %s, 0, 0 (resume sibling)\n",
 			    target_pid_to_str (lp->ptid));
       lp->stopped = 0;
-      lp->step = step;
+      lp->step = 0;
       memset (&lp->siginfo, 0, sizeof (lp->siginfo));
       lp->stopped_by_watchpoint = 0;
     }
--- src/gdb/remote.c	2010/10/20 09:10:48	1.423
+++ src/gdb/remote.c	2010/11/02 01:37:32	1.424
@@ -4416,12 +4416,6 @@
   return p;
 }
 
-static int
-currently_stepping_callback (struct thread_info *tp, void *data)
-{
-  return currently_stepping (tp);
-}
-
 /* Resume the remote inferior by using a "vCont" packet.  The thread
    to be resumed is PTID; STEP and SIGGNAL indicate whether the
    resumed thread should be single-stepped and/or signalled.  If PTID
@@ -4464,8 +4458,6 @@
     }
   else if (ptid_equal (ptid, minus_one_ptid) || ptid_is_pid (ptid))
     {
-      struct thread_info *tp;
-
       /* Resume all threads (of all processes, or of a single
 	 process), with preference for INFERIOR_PTID.  This assumes
 	 inferior_ptid belongs to the set of all threads we are about
@@ -4476,12 +4468,6 @@
 	  p = append_resumption (p, endp, inferior_ptid, step, siggnal);
 	}
 
-      tp = iterate_over_threads (currently_stepping_callback, NULL);
-      if (tp && !ptid_equal (tp->ptid, inferior_ptid))
-	{
-	  p = append_resumption (p, endp, tp->ptid, 1, TARGET_SIGNAL_0);
-	}
-
       /* And continue others without a signal.  */
       p = append_resumption (p, endp, ptid, /*step=*/ 0, TARGET_SIGNAL_0);
     }
--- src/gdb/testsuite/ChangeLog	2010/10/20 23:58:06	1.2490
+++ src/gdb/testsuite/ChangeLog	2010/11/02 01:37:32	1.2491
@@ -1,3 +1,10 @@
+2010-11-02  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	Revert:
+	2010-10-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
+	* gdb.threads/sigstep-threads.exp: New file.
+	* gdb.threads/sigstep-threads.c: New file.
+
 2010-10-20  Michael Snyder  <msnyder@vmware.com>
 
 	* gdb.threads/fork-child-threads.exp: Don't run on remote target.
[ The two files got deleted.  ]