[rfc] linux-nat: Never PTRACE_CONT a stepping thread
Pedro Alves
pedro@codesourcery.com
Sun Oct 17 19:04:00 GMT 2010
On Sunday 17 October 2010 19:27:45, Jan Kratochvil wrote:
> On Sat, 16 Oct 2010 19:09:52 +0200, Pedro Alves wrote:
> > This patch alone on top of current mainline fixes the sigstep-threads.exp
> > test from <http://sourceware.org/ml/gdb-patches/2010-09/msg00360.html>.
> > Maybe check that test in along with this patch?
>
> OK, yes, I will recheck that patch on top of this one.
>
>
> > The corresponding remote.c patch (pasted below) that translates this
> > into a vCont with three actions (e.g., "vCont;C1e:795a;s:7959;c") also
> > fixes that test for gdbserver linux.
>
> Included it.
>
>
> > It'd be much cleaner to make the target_resume interface similar
> > to gdbserver's target_ops->resume interface (pass in a list of
> > resume actions, similar to vCont), but that shouldn't be a
> > prerequisite. Maybe someday...
>
> I agree that interface is better. Not a scope of this patch, though.
Thanks. I just belatedly realized that this probably
breaks software single-step archs though. :-/
>
>
> Checked-in.
>
> Thanks for the review.
>
>
> Jan
>
>
> http://sourceware.org/ml/gdb-cvs/2010-10/msg00102.html
>
> --- src/gdb/ChangeLog 2010/10/17 17:45:16 1.12266
> +++ src/gdb/ChangeLog 2010/10/17 18:24:46 1.12267
> @@ -1,4 +1,19 @@
> 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-10-17 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * infrun.c (follow_exec): Replace symbol_file_add_main by
> symbol_file_add with SYMFILE_DEFER_BP_RESET, set_initial_language and
> --- src/gdb/gdbthread.h 2010/01/12 21:40:24 1.54
> +++ src/gdb/gdbthread.h 2010/10/17 18:24:47 1.55
> @@ -352,4 +352,6 @@
>
> extern void update_thread_list (void);
>
> +extern int currently_stepping (struct thread_info *tp);
> +
> #endif /* GDBTHREAD_H */
> --- src/gdb/infrun.c 2010/10/17 17:45:16 1.452
> +++ src/gdb/infrun.c 2010/10/17 18:24:47 1.453
> @@ -74,8 +74,6 @@
> 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);
>
> @@ -4851,7 +4849,7 @@
>
> /* Is thread TP in the middle of single-stepping? */
>
> -static int
> +int
> currently_stepping (struct thread_info *tp)
> {
> return ((tp->step_range_end && tp->step_resume_breakpoint == NULL)
> --- src/gdb/linux-nat.c 2010/09/06 13:59:02 1.184
> +++ src/gdb/linux-nat.c 2010/10/17 18:24:47 1.185
> @@ -1820,20 +1820,26 @@
> }
> 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: PTRACE_CONT %s, 0, 0 (resuming sibling)\n",
> + "RC: %s %s, 0, 0 (resuming sibling)\n",
> + step ? "PTRACE_SINGLESTEP" : "PTRACE_CONT",
> target_pid_to_str (lp->ptid));
>
> linux_ops->to_resume (linux_ops,
> pid_to_ptid (GET_LWP (lp->ptid)),
> - 0, TARGET_SIGNAL_0);
> + step, TARGET_SIGNAL_0);
> if (debug_linux_nat)
> fprintf_unfiltered (gdb_stdlog,
> - "RC: PTRACE_CONT %s, 0, 0 (resume sibling)\n",
> + "RC: %s %s, 0, 0 (resume sibling)\n",
> + step ? "PTRACE_SINGLESTEP" : "PTRACE_CONT",
> target_pid_to_str (lp->ptid));
> lp->stopped = 0;
> - lp->step = 0;
> + lp->step = step;
> memset (&lp->siginfo, 0, sizeof (lp->siginfo));
> lp->stopped_by_watchpoint = 0;
> }
> --- src/gdb/remote.c 2010/07/28 20:20:26 1.421
> +++ src/gdb/remote.c 2010/10/17 18:24:47 1.422
> @@ -4416,6 +4416,12 @@
> 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
> @@ -4458,6 +4464,8 @@
> }
> 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
> @@ -4468,6 +4476,12 @@
> 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/17 17:45:17 1.2482
> +++ src/gdb/testsuite/ChangeLog 2010/10/17 18:24:47 1.2483
> @@ -1,5 +1,10 @@
> 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-17 Jan Kratochvil <jan.kratochvil@redhat.com>
> +
> * gdb.base/pie-execl.exp: New file.
> * gdb.base/pie-execl.c: New file.
>
> --- src/gdb/testsuite/gdb.threads/sigstep-threads.c
> +++ src/gdb/testsuite/gdb.threads/sigstep-threads.c 2010-10-17 18:24:59.596239000 +0000
> @@ -0,0 +1,54 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2010 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +#include <pthread.h>
> +#include <assert.h>
> +#include <signal.h>
> +
> +#include <asm/unistd.h>
> +#include <unistd.h>
> +#define tgkill(tgid, tid, sig) syscall (__NR_tgkill, (tgid), (tid), (sig))
> +#define gettid() syscall (__NR_gettid)
> +
> +static volatile int var;
> +
> +static void
> +handler (int signo) /* step-0 */
> +{ /* step-0 */
> + var++; /* step-1 */
> + tgkill (getpid (), gettid (), SIGUSR1); /* step-2 */
> +}
> +
> +static void *
> +start (void *arg)
> +{
> + signal (SIGUSR1, handler);
> + tgkill (getpid (), gettid (), SIGUSR1);
> + assert (0);
> +
> + return NULL;
> +}
> +
> +int
> +main (void)
> +{
> + pthread_t thread;
> +
> + pthread_create (&thread, NULL, start, NULL);
> + start (NULL); /* main-start */
> + return 0;
> +}
> --- src/gdb/testsuite/gdb.threads/sigstep-threads.exp
> +++ src/gdb/testsuite/gdb.threads/sigstep-threads.exp 2010-10-17 18:24:59.896865000 +0000
> @@ -0,0 +1,74 @@
> +# Copyright 2010 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +set testfile sigstep-threads
> +set srcfile ${testfile}.c
> +set executable ${testfile}
> +set binfile ${objdir}/${subdir}/${executable}
> +
> +if { [gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
> + untested ${testfile}.exp
> + return -1
> +}
> +
> +clean_restart $executable
> +
> +if ![runto_main] {
> + return -1;
> +}
> +
> +# `noprint' would not test the full logic of GDB.
> +gdb_test "handle SIGUSR1 nostop print pass" "\r\nSIGUSR1\[ \t\]+No\[ \t\]+Yes\[ \t\]+Yes\[ \t\].*"
> +
> +gdb_test_no_output "set scheduler-locking off"
> +
> +gdb_breakpoint [gdb_get_line_number "step-1"]
> +gdb_test_no_output {set $step1=$bpnum}
> +gdb_continue_to_breakpoint "step-1" ".* step-1 .*"
> +gdb_test_no_output {disable $step1}
> +
> +# 1 as we are now stopped at the `step-1' label.
> +set step_at 1
> +for {set i 0} {$i < 100} {incr i} {
> + set test "step $i"
> + # Presume this step failed - as in the case of a timeout.
> + set failed 1
> + gdb_test_multiple "step" $test {
> + -re "\r\nProgram received signal SIGUSR1, User defined signal 1.\r\n" {
> + exp_continue -continue_timer
> + }
> + -re "step-(\[012\]).*\r\n$gdb_prompt $" {
> + set now $expect_out(1,string)
> + if {$step_at == 2 && $now == 1} {
> + set failed 0
> + } elseif {$step_at == 1 && $now == 2} {
> + set failed 0
> + # Continue over the re-signalling back to the handle entry.
> + gdb_test_no_output {enable $step1} ""
> + gdb_test "continue" " step-1 .*" ""
> + set now 1
> + gdb_test_no_output {disable $step1} ""
> + } else {
> + fail $test
> + }
> + set step_at $now
> + }
> + }
> + if $failed {
> + return
> + }
> +}
> +# We can never reliably say the racy problematic case has been tested.
> +pass "step"
>
--
Pedro Alves
More information about the Gdb-patches
mailing list