From 7ae1a6a6ccda41aa8bbe9adb0f7fcde8bf8d5cb3 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Fri, 25 Apr 2014 19:07:33 +0100 Subject: [PATCH] PR server/16255: gdbserver cannot attach to a second inferior that is multi-threaded. On Linux, we need to explicitly ptrace attach to all lwps of a process. Because GDB might not be connected yet when an attach is requested, and thus it may not be possible to activate thread_db, as that requires access to symbols (IOW, gdbserver --attach), a while ago we make linux_attach loop over the lwps as listed by /proc/PID/task to find the lwps to attach to. linux_attach_lwp_1 has: ... if (initial) /* If lwp is the tgid, we handle adding existing threads later. Otherwise we just add lwp without bothering about any other threads. */ ptid = ptid_build (lwpid, lwpid, 0); else { /* Note that extracting the pid from the current inferior is safe, since we're always called in the context of the same process as this new thread. */ int pid = pid_of (current_inferior); ptid = ptid_build (pid, lwpid, 0); } That "safe" comment referred to linux_attach_lwp being called by thread-db.c. But this was clearly missed when a new call to linux_attach_lwp_1 was added to linux_attach. As a result, current_inferior will be set to some random process, and non-initial lwps of the second inferior get assigned the pid of the wrong inferior. E.g., in the case of attaching to two inferiors, for the second inferior (and so on), non-initial lwps of the second inferior get assigned the pid of the first inferior. This doesn't trigger on the first inferior, when current_inferior is NULL, add_thread switches the current inferior to the newly added thread. Rather than making linux_attach switch current_inferior temporarily (thus avoiding further reliance on global state), or making linux_attach_lwp_1 get the tgid from /proc, which add extra syscalls, and will be wrong in case of the user having originally attached directly to a non-tgid lwp, and then that lwp spawning new clones (the ptid.pid field of further new clones should be the same as the original lwp's pid, which is not the tgid), we note that callers of linux_attach_lwp/linux_attach_lwp_1 always have the right pid handy already, so they can pass it down along with the lwpid. The only other reason for the "initial" parameter is to error out instead of warn in case of attach failure, when we're first attaching to a process. There are only three callers of linux_attach_lwp/linux_attach_lwp_1, and each wants to print a different warn/error string, so we can just move the error/warn out of linux_attach_lwp_1 to the callers, thus getting rid of the "initial" parameter. There really nothing gdbserver-specific about attaching to two threaded processes, so this adds a new test under gdb.multi/. The test passes cleanly against the native GNU/Linux target, but fails/triggers the bug against GDBserver (before the patch), with the native-extended-remote board (as plain remote doesn't support multi-process). Tested on x86_64 Fedora 17, with the native-extended-gdbserver board. gdb/gdbserver/ 2014-04-25 Pedro Alves PR server/16255 * linux-low.c (linux_attach_fail_reason_string): New function. (linux_attach_lwp): Delete. (linux_attach_lwp_1): Rename to ... (linux_attach_lwp): ... this. Take a ptid instead of a pid as argument. Remove "initial" parameter. Return int instead of void. Don't error or warn here. (linux_attach): Adjust to call linux_attach_lwp. Call error on failure to attach to the tgid. Call warning when failing to attach to an lwp. * linux-low.h (linux_attach_lwp): Take a ptid instead of a pid as argument. Remove "initial" parameter. Return int instead of void. Don't error or warn here. (linux_attach_fail_reason_string): New declaration. * thread-db.c (attach_thread): Adjust to linux_attach_lwp's interface change. Use linux_attach_fail_reason_string. gdb/ 2014-04-25 Pedro Alves PR server/16255 * common/linux-ptrace.c (linux_ptrace_attach_warnings): Rename to ... (linux_ptrace_attach_fail_reason): ... this. Remove "warning: " and newline from built string. * common/linux-ptrace.h (linux_ptrace_attach_warnings): Rename to ... (linux_ptrace_attach_fail_reason): ... this. * linux-nat.c (linux_nat_attach): Adjust to use linux_ptrace_attach_fail_reason. gdb/testsuite/ 2014-04-25 Simon Marchi Pedro Alves PR server/16255 * gdb.multi/multi-attach.c: New file. * gdb.multi/multi-attach.exp: New file. --- gdb/ChangeLog | 11 +++ gdb/common/linux-ptrace.c | 16 ++-- gdb/common/linux-ptrace.h | 2 +- gdb/gdbserver/linux-low.c | 105 ++++++++++++----------- gdb/gdbserver/linux-low.h | 11 ++- gdb/gdbserver/thread-db.c | 18 ++-- gdb/linux-nat.c | 7 +- gdb/testsuite/ChangeLog | 7 ++ gdb/testsuite/gdb.multi/multi-attach.c | 48 +++++++++++ gdb/testsuite/gdb.multi/multi-attach.exp | 60 +++++++++++++ 10 files changed, 217 insertions(+), 68 deletions(-) create mode 100644 gdb/testsuite/gdb.multi/multi-attach.c create mode 100644 gdb/testsuite/gdb.multi/multi-attach.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index cf5731ebf4f..1ac9bcf61de 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,14 @@ +2014-04-25 Pedro Alves + + PR server/16255 + * common/linux-ptrace.c (linux_ptrace_attach_warnings): Rename to ... + (linux_ptrace_attach_fail_reason): ... this. Remove "warning: " + and newline from built string. + * common/linux-ptrace.h (linux_ptrace_attach_warnings): Rename to ... + (linux_ptrace_attach_fail_reason): ... this. + * linux-nat.c (linux_nat_attach): Adjust to use + linux_ptrace_attach_fail_reason. + 2014-04-25 Pedro Alves * remote.c (struct remote_state): Remove multi_process_aware, diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c index 7c1b78a55b6..efbd1ea10bc 100644 --- a/gdb/common/linux-ptrace.c +++ b/gdb/common/linux-ptrace.c @@ -37,24 +37,24 @@ there are no supported features. */ static int current_ptrace_options = -1; -/* Find all possible reasons we could fail to attach PID and append these - newline terminated reason strings to initialized BUFFER. '\0' termination - of BUFFER must be done by the caller. */ +/* Find all possible reasons we could fail to attach PID and append + these as strings to the already initialized BUFFER. '\0' + termination of BUFFER must be done by the caller. */ void -linux_ptrace_attach_warnings (pid_t pid, struct buffer *buffer) +linux_ptrace_attach_fail_reason (pid_t pid, struct buffer *buffer) { pid_t tracerpid; tracerpid = linux_proc_get_tracerpid (pid); if (tracerpid > 0) - buffer_xml_printf (buffer, _("warning: process %d is already traced " - "by process %d\n"), + buffer_xml_printf (buffer, _("process %d is already traced " + "by process %d"), (int) pid, (int) tracerpid); if (linux_proc_pid_is_zombie (pid)) - buffer_xml_printf (buffer, _("warning: process %d is a zombie " - "- the process has already terminated\n"), + buffer_xml_printf (buffer, _("process %d is a zombie " + "- the process has already terminated"), (int) pid); } diff --git a/gdb/common/linux-ptrace.h b/gdb/common/linux-ptrace.h index 38bb9ea92d4..881d9c99628 100644 --- a/gdb/common/linux-ptrace.h +++ b/gdb/common/linux-ptrace.h @@ -83,7 +83,7 @@ struct buffer; #define __WALL 0x40000000 /* Wait for any child. */ #endif -extern void linux_ptrace_attach_warnings (pid_t pid, struct buffer *buffer); +extern void linux_ptrace_attach_fail_reason (pid_t pid, struct buffer *buffer); extern void linux_ptrace_init_warnings (void); extern void linux_enable_event_reporting (pid_t pid); extern int linux_supports_tracefork (void); diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index c847c62f72a..0ef8d600bf3 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -635,49 +635,41 @@ linux_create_inferior (char *program, char **allargs) return pid; } +char * +linux_attach_fail_reason_string (ptid_t ptid, int err) +{ + static char *reason_string; + struct buffer buffer; + char *warnings; + long lwpid = ptid_get_lwp (ptid); + + xfree (reason_string); + + buffer_init (&buffer); + linux_ptrace_attach_fail_reason (lwpid, &buffer); + buffer_grow_str0 (&buffer, ""); + warnings = buffer_finish (&buffer); + if (warnings[0] != '\0') + reason_string = xstrprintf ("%s (%d), %s", + strerror (err), err, warnings); + else + reason_string = xstrprintf ("%s (%d)", + strerror (err), err); + xfree (warnings); + return reason_string; +} + /* Attach to an inferior process. */ -static void -linux_attach_lwp_1 (unsigned long lwpid, int initial) +int +linux_attach_lwp (ptid_t ptid) { - ptid_t ptid; struct lwp_info *new_lwp; + int lwpid = ptid_get_lwp (ptid); if (ptrace (PTRACE_ATTACH, lwpid, (PTRACE_TYPE_ARG3) 0, (PTRACE_TYPE_ARG4) 0) != 0) - { - struct buffer buffer; - - if (!initial) - { - /* If we fail to attach to an LWP, just warn. */ - fprintf (stderr, "Cannot attach to lwp %ld: %s (%d)\n", lwpid, - strerror (errno), errno); - fflush (stderr); - return; - } - - /* If we fail to attach to a process, report an error. */ - buffer_init (&buffer); - linux_ptrace_attach_warnings (lwpid, &buffer); - buffer_grow_str0 (&buffer, ""); - error ("%sCannot attach to lwp %ld: %s (%d)", buffer_finish (&buffer), - lwpid, strerror (errno), errno); - } - - if (initial) - /* If lwp is the tgid, we handle adding existing threads later. - Otherwise we just add lwp without bothering about any other - threads. */ - ptid = ptid_build (lwpid, lwpid, 0); - else - { - /* Note that extracting the pid from the current inferior is - safe, since we're always called in the context of the same - process as this new thread. */ - int pid = pid_of (current_inferior); - ptid = ptid_build (pid, lwpid, 0); - } + return errno; new_lwp = add_lwp (ptid); @@ -747,12 +739,8 @@ linux_attach_lwp_1 (unsigned long lwpid, int initial) end of the list, and so the new thread has not yet reached wait_for_sigstop (but will). */ new_lwp->stop_expected = 1; -} -void -linux_attach_lwp (unsigned long lwpid) -{ - linux_attach_lwp_1 (lwpid, 0); + return 0; } /* Attach to PID. If PID is the tgid, attach to it and all @@ -761,9 +749,16 @@ linux_attach_lwp (unsigned long lwpid) static int linux_attach (unsigned long pid) { + ptid_t ptid = ptid_build (pid, pid, 0); + int err; + /* Attach to PID. We will check for other threads soon. */ - linux_attach_lwp_1 (pid, 1); + err = linux_attach_lwp (ptid); + if (err != 0) + error ("Cannot attach to process %ld: %s", + pid, linux_attach_fail_reason_string (ptid, err)); + linux_add_process (pid, 1); if (!non_stop) @@ -794,13 +789,13 @@ linux_attach (unsigned long pid) { /* At this point we attached to the tgid. Scan the task for existing threads. */ - unsigned long lwp; int new_threads_found; int iterations = 0; - struct dirent *dp; while (iterations < 2) { + struct dirent *dp; + new_threads_found = 0; /* Add all the other threads. While we go through the threads, new threads may be spawned. Cycle through @@ -808,19 +803,29 @@ linux_attach (unsigned long pid) finding new threads. */ while ((dp = readdir (dir)) != NULL) { + unsigned long lwp; + ptid_t ptid; + /* Fetch one lwp. */ lwp = strtoul (dp->d_name, NULL, 10); + ptid = ptid_build (pid, lwp, 0); + /* Is this a new thread? */ - if (lwp - && find_thread_ptid (ptid_build (pid, lwp, 0)) == NULL) + if (lwp != 0 && find_thread_ptid (ptid) == NULL) { - linux_attach_lwp_1 (lwp, 0); - new_threads_found++; + int err; if (debug_threads) - debug_printf ("Found and attached to new lwp %ld\n", - lwp); + debug_printf ("Found new lwp %ld\n", lwp); + + err = linux_attach_lwp (ptid); + if (err != 0) + warning ("Cannot attach to lwp %ld: %s", + lwp, + linux_attach_fail_reason_string (ptid, err)); + + new_threads_found++; } } diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h index 7459710dacf..cd3001dec95 100644 --- a/gdb/gdbserver/linux-low.h +++ b/gdb/gdbserver/linux-low.h @@ -343,7 +343,16 @@ struct lwp_info int linux_pid_exe_is_elf_64_file (int pid, unsigned int *machine); -void linux_attach_lwp (unsigned long pid); +/* Attach to PTID. Returns 0 on success, non-zero otherwise (an + errno). */ +int linux_attach_lwp (ptid_t ptid); + +/* Return the reason an attach failed, in string form. ERR is the + error returned by linux_attach_lwp (an errno). This string should + be copied into a buffer by the client if the string will not be + immediately used, or if it must persist. */ +char *linux_attach_fail_reason_string (ptid_t ptid, int err); + struct lwp_info *find_lwp_pid (ptid_t ptid); void linux_stop_lwp (struct lwp_info *lwp); diff --git a/gdb/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c index f63e39e8297..ae0d191aba3 100644 --- a/gdb/gdbserver/thread-db.c +++ b/gdb/gdbserver/thread-db.c @@ -323,27 +323,33 @@ find_one_thread (ptid_t ptid) static int attach_thread (const td_thrhandle_t *th_p, td_thrinfo_t *ti_p) { + struct process_info *proc = current_process (); + int pid = pid_of (proc); + ptid_t ptid = ptid_build (pid, ti_p->ti_lid, 0); struct lwp_info *lwp; + int err; if (debug_threads) debug_printf ("Attaching to thread %ld (LWP %d)\n", ti_p->ti_tid, ti_p->ti_lid); - linux_attach_lwp (ti_p->ti_lid); - lwp = find_lwp_pid (pid_to_ptid (ti_p->ti_lid)); - if (lwp == NULL) + err = linux_attach_lwp (ptid); + if (err != 0) { - warning ("Could not attach to thread %ld (LWP %d)\n", - ti_p->ti_tid, ti_p->ti_lid); + warning ("Could not attach to thread %ld (LWP %d): %s\n", + ti_p->ti_tid, ti_p->ti_lid, + linux_attach_fail_reason_string (ptid, err)); return 0; } + lwp = find_lwp_pid (ptid); + gdb_assert (lwp != NULL); lwp->thread_known = 1; lwp->th = *th_p; if (thread_db_use_events) { td_err_e err; - struct thread_db *thread_db = current_process ()->private->thread_db; + struct thread_db *thread_db = proc->private->thread_db; err = thread_db->td_thr_event_enable_p (th_p, 1); if (err != TD_OK) diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index d08cb1357cc..e84ee952783 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -1320,13 +1320,16 @@ linux_nat_attach (struct target_ops *ops, char *args, int from_tty) make_cleanup (xfree, message); buffer_init (&buffer); - linux_ptrace_attach_warnings (pid, &buffer); + linux_ptrace_attach_fail_reason (pid, &buffer); buffer_grow_str0 (&buffer, ""); buffer_s = buffer_finish (&buffer); make_cleanup (xfree, buffer_s); - throw_error (ex.error, "%s%s", buffer_s, message); + if (*buffer_s != '\0') + throw_error (ex.error, "warning: %s\n%s", buffer_s, message); + else + throw_error (ex.error, "%s", message); } /* The ptrace base target adds the main thread with (pid,0,0) diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 0b24bd22c4f..ba39114a97f 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2014-04-25 Simon Marchi + Pedro Alves + + PR server/16255 + * gdb.multi/multi-attach.c: New file. + * gdb.multi/multi-attach.exp: New file. + 2014-04-25 Pedro Alves * gdb.base/cond-eval-mode.exp (warning): Move trailing \r\n to diff --git a/gdb/testsuite/gdb.multi/multi-attach.c b/gdb/testsuite/gdb.multi/multi-attach.c new file mode 100644 index 00000000000..0fb0a64af15 --- /dev/null +++ b/gdb/testsuite/gdb.multi/multi-attach.c @@ -0,0 +1,48 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2007-2014 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 . */ + +/* This program is intended to be started outside of gdb, and then + attached to by gdb. It loops for a while, but not forever. */ + +#include +#include + +static void * +thread_func (void *arg) +{ + int i; + + for (i = 0; i < 120; i++) + sleep (1); + + return NULL; +} + +int main () +{ + int i; + pthread_t thread; + + pthread_create (&thread, NULL, thread_func, NULL); + + for (i = 0; i < 120; i++) + sleep (1); + + pthread_join (thread, NULL); + + return 0; +} diff --git a/gdb/testsuite/gdb.multi/multi-attach.exp b/gdb/testsuite/gdb.multi/multi-attach.exp new file mode 100644 index 00000000000..e933520e61b --- /dev/null +++ b/gdb/testsuite/gdb.multi/multi-attach.exp @@ -0,0 +1,60 @@ +# This testcase is part of GDB, the GNU debugger. + +# Copyright 2007-2014 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 . + +# Test attaching to multiple threaded programs. + +standard_testfile + +# We need to use TCL's exec to get the pid. +if [is_remote target] then { + return 0 +} + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug additional_flags=-lpthread}]} { + return -1 +} + +# Start the programs running and then wait for a bit, to be sure that +# they can be attached to. +set testpid1 [eval exec $binfile &] +set testpid2 [eval exec $binfile &] +exec sleep 2 +if { [istarget "*-*-cygwin*"] } { + # testpid{1,2} are the Cygwin PID, GDB uses the Windows PID, which might be + # different due to the way fork/exec works. + set testpid1 [ exec ps -e | gawk "{ if (\$1 == $testpid1) print \$4; }" ] + set testpid2 [ exec ps -e | gawk "{ if (\$1 == $testpid2) print \$4; }" ] +} + +gdb_test "attach $testpid1" \ + "Attaching to program: .*, process $testpid1.*(in|at).*" \ + "attach to program 1" +gdb_test "backtrace" ".*main.*" "backtrace 1" + +gdb_test "add-inferior -exec $binfile" \ + "Added inferior 2.*" \ + "add second inferior" +gdb_test "inferior 2" ".*Switching to inferior 2.*" "switch to second inferior" + +gdb_test "attach $testpid2" \ + "Attaching to program: .*, process $testpid2.*(in|at).*" \ + "attach to program 2" +gdb_test "backtrace" ".*main.*" "backtrace 2" + +gdb_test "kill" "" "kill inferior 2" "Kill the program being debugged.*" "y" +gdb_test "inferior 1" ".*Switching to inferior 1.*" +gdb_test "kill" "" "kill inferior 1" "Kill the program being debugged.*" "y" -- 2.30.2