This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [rfc] linux-nat: Never PTRACE_CONT a stepping thread
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: Pedro Alves <pedro at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 2 Nov 2010 02:55:42 +0100
- Subject: Re: [rfc] linux-nat: Never PTRACE_CONT a stepping thread
- References: <20100921234325.GA31267@host1.dyn.jankratochvil.net> <201010172004.18986.pedro@codesourcery.com> <20101018085138.GA25628@host1.dyn.jankratochvil.net> <201010192024.04881.pedro@codesourcery.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. ]