This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] gdbserver: Get the pid from /proc when attaching to a non-initial lwp
- From: Pedro Alves <palves at redhat dot com>
- To: Simon Marchi <simon dot marchi at ericsson dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Fri, 25 Apr 2014 17:00:04 +0100
- Subject: Re: [PATCH] gdbserver: Get the pid from /proc when attaching to a non-initial lwp
- Authentication-results: sourceware.org; auth=none
- References: <1398187705-17237-1-git-send-email-simon dot marchi at ericsson dot com>
On 04/22/2014 06:28 PM, Simon Marchi wrote:
> gdbserver fails to attach to a second inferior that is multi-threaded.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16255
>
> For the second inferior (and so on), current_inferior is still set to the
> first inferior when this code is ran. As a result, non-initial lwps of
> the second inferior get assigned the pid of the first inferior.
>
> One solution could be to switch current_inferior temporarily. Another
> one (which I chose) is to go get the pid (tgid in the linux terminology)
> in /proc.
Thanks Simon. After you poked me earlier about this, I had an idea
for a different solution, which I think ends up being more complete,
and I've coded it now. See the description in the patch below.
> I augmented the gdb.server/ext-attach.exp test case to attach to two
> inferiors simultaneously and made the test program multi-threaded.
Thanks. That's very useful. Let's use it, but make it independent of
gdbserver. There's really nothing gdbserver specific about attaching
to two inferiors. All (multi-process capable) targets should be
able to do this.
Let me know what you think.
---------
>From 1705e97537ea392af4e4c2dd0c6e0ec1136f2e39 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 25 Apr 2014 15:23:44 +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 <palves@redhat.com>
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 <palves@redhat.com>
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 <simon.marchi@ericsson.com>
Pedro Alves <palves@redhat.com>
PR server/16255
* gdb.multi/multi-attach.c: New file.
* gdb.multi/multi-attach.exp: New file.
---
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/gdb.multi/multi-attach.c | 48 ++++++++++++++
gdb/testsuite/gdb.multi/multi-attach.exp | 60 ++++++++++++++++++
8 files changed, 199 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/common/linux-ptrace.c b/gdb/common/linux-ptrace.c
index 7c1b78a..efbd1ea 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 38bb9ea..881d9c9 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 c847c62..0ef8d60 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 7459710..cd3001d 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 f63e39e..ae0d191 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 d08cb13..e84ee95 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/gdb.multi/multi-attach.c b/gdb/testsuite/gdb.multi/multi-attach.c
new file mode 100644
index 0000000..0fb0a64
--- /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 <http://www.gnu.org/licenses/>. */
+
+/* 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 <unistd.h>
+#include <pthread.h>
+
+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 0000000..e933520
--- /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 <http://www.gnu.org/licenses/>.
+
+# 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"
--
1.7.11.7