From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3898 invoked by alias); 9 Nov 2014 21:23:22 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 3873 invoked by uid 89); 9 Nov 2014 21:23:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: dancol.org Received: from dancol.org (HELO dancol.org) (96.126.100.184) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Sun, 09 Nov 2014 21:23:20 +0000 Received: from [199.201.65.2] (helo=[172.30.20.166]) by dancol.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84) (envelope-from ) id 1XnZx5-0004C0-Lt; Sun, 09 Nov 2014 13:23:17 -0800 Message-ID: <545FDB3C.6060709@dancol.org> Date: Sun, 09 Nov 2014 21:23:00 -0000 From: Daniel Colascione User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Pedro Alves , gdb-patches@sourceware.org Subject: Re: [PATCH] Warn users about mismatched PID namespaces References: <5451AB7E.40709@dancol.org> <545CBFEB.7000907@redhat.com> In-Reply-To: <545CBFEB.7000907@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-11/txt/msg00156.txt.bz2 -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Second version follows. On 11/07/2014 12:49 PM, Pedro Alves wrote: > On 10/30/2014 03:07 AM, Daniel Colascione wrote: > >> diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c >> index 352fac1..4089417 100644 >> - --- a/gdb/linux-thread-db.c >> +++ b/gdb/linux-thread-db.c >> @@ -1223,6 +1223,25 @@ thread_db_new_objfile (struct objfile *objfile) >> static void >> thread_db_inferior_created (struct target_ops *target, int from_tty) >> { > > This hook is called even if the current target isn't a native process. > E.g., you may have loaded a core, in which case looking at > getpid or /proc doesn't make sense. Or you may be debugging with > "target sim", or a remote process with gdbserver [1], etc. > > We need this same check that thread_db_load does: > > /* Don't attempt to use thread_db for remote targets. */ > if (!(target_can_run (¤t_target) || core_bfd)) > return 0; > > [1] BTW, could I interest in giving gdbserver/thread-db.c the > same treatment? Do warnings from gdbserver even get propagated to remotes? I don't see any obvious equivalent to the on-attach hook there. >> + /* If the child is in a different PID namespace, its idea of its PID >> + will differ from our idea of its PID. When we scan the child's >> + thread list, we'll mistakenly think it has no threads since the >> + thread PID fields won't match the PID we give to >> + libthread_db. */ >> + char *our_pid_ns = linux_proc_pid_get_ns (getpid (), "pid"); >> + char *inferior_pid_ns = linux_proc_pid_get_ns ( >> + ptid_get_pid (inferior_ptid), "pid"); >> + >> + if (our_pid_ns != NULL && inferior_pid_ns != NULL && > > Put '&&' at the beginning of the next line. Done > >> + strcmp (our_pid_ns, inferior_pid_ns) != 0) > > >> + { >> + warning (_ ("Target and debugger are in different PID namespaces; " >> + "thread lists and other data are likely unreliable")); >> + } >> + >> + xfree (our_pid_ns); >> + xfree (inferior_pid_ns); > > Please factor this new code to a function; Having it in a function > makes it easier to move the caller around if necessary. Done. > >> + >> check_for_thread_db (); >> } >> >> diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c >> index 30797da..8efccba 100644 >> - --- a/gdb/nat/linux-procfs.c >> +++ b/gdb/nat/linux-procfs.c >> @@ -113,3 +113,22 @@ linux_proc_pid_is_zombie (pid_t pid) >> { >> return linux_proc_pid_has_state (pid, "Z (zombie)"); >> } >> + >> +/* See linux-procfs.h declaration. */ >> + >> +char* >> +linux_proc_pid_get_ns (pid_t pid, const char *ns) >> +{ >> + char buf[100]; >> + char nsval[64]; >> + int ret; >> + snprintf (buf, sizeof (buf), "/proc/%d/ns/%s", (int) pid, ns); > > Use xsnprintf Done. >> >> +/* Return an opaque string identifying PID's NS namespace or NULL if >> + * the information is unavailable. The returned string must be >> + * released with xfree. */ >> + >> +extern char* linux_proc_pid_get_ns (pid_t pid, const char *ns); > > Space between char and *. What do you mean? Lots of other functions in GDB use the style I used in my patch. diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c index 352fac1..bd3635e 100644 - --- a/gdb/linux-thread-db.c +++ b/gdb/linux-thread-db.c @@ -1217,12 +1217,41 @@ thread_db_new_objfile (struct objfile *objfile) check_for_thread_db (); } +static void +check_pid_namespace_match () +{ + /* Check is only relevant for local targets targets. */ + if (target_can_run (¤t_target)) + { + /* If the child is in a different PID namespace, its idea of its + PID will differ from our idea of its PID. When we scan the + child's thread list, we'll mistakenly think it has no threads + since the thread PID fields won't match the PID we give to + libthread_db. */ + char *our_pid_ns = linux_proc_pid_get_ns (getpid (), "pid"); + char *inferior_pid_ns = linux_proc_pid_get_ns ( + ptid_get_pid (inferior_ptid), "pid"); + + if (our_pid_ns != NULL && inferior_pid_ns != NULL + && strcmp (our_pid_ns, inferior_pid_ns) != 0) + { + warning (_ ("Target and debugger are in different PID " + "namespaces; thread lists and other data are " + "likely unreliable")); + } + + xfree (our_pid_ns); + xfree (inferior_pid_ns); + } +} + /* This function is called via the inferior_created observer. This handles the case of debugging statically linked executables. */ static void thread_db_inferior_created (struct target_ops *target, int from_tty) { + check_pid_namespace_match (); check_for_thread_db (); } diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c index 30797da..8efccba 100644 - --- a/gdb/nat/linux-procfs.c +++ b/gdb/nat/linux-procfs.c @@ -113,3 +113,22 @@ linux_proc_pid_is_zombie (pid_t pid) { return linux_proc_pid_has_state (pid, "Z (zombie)"); } + +/* See linux-procfs.h declaration. */ + +char* +linux_proc_pid_get_ns (pid_t pid, const char *ns) +{ + char buf[100]; + char nsval[64]; + int ret; + snprintf (buf, sizeof (buf), "/proc/%d/ns/%s", (int) pid, ns); + ret = readlink (buf, nsval, sizeof (nsval)); + if (0 < ret && ret < sizeof (nsval)) + { + nsval[ret] = '\0'; + return xstrdup (nsval); + } + + return NULL; +} diff --git a/gdb/nat/linux-procfs.h b/gdb/nat/linux-procfs.h index d13fff7..e4f301f 100644 - --- a/gdb/nat/linux-procfs.h +++ b/gdb/nat/linux-procfs.h @@ -40,4 +40,10 @@ extern int linux_proc_pid_is_stopped (pid_t pid); extern int linux_proc_pid_is_zombie (pid_t pid); +/* Return an opaque string identifying PID's NS namespace or NULL if + * the information is unavailable. The returned string must be + * released with xfree. */ + +extern char* linux_proc_pid_get_ns (pid_t pid, const char *ns); + #endif /* COMMON_LINUX_PROCFS_H */ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCAAGBQJUX9s8AAoJEN4WImmbpWBlApwP/2KmKdnBIh55ZDUggMOL63Ui uFDcuccoQUH+WiLgVw36SJFLFOlDl4wab3ipg37eUiChR/iBXnAuBqHRIVTO5VfF AkjXRAJw5obNcW0b62aD645gjQd4hOsvaYRDcuV6W+vbn1FIPxRxNGAHwtZQ7PNE 06vJu//OWbjQZVogGgVojDK7j1oWEm66nR2Se5UPOk+ddxqoFDtSCNAiutnhlstt w4+Dh3NCDSJUnoL3qPQf8ex8qymojvwshTvJLOMVX7XzXnuGNXZg/8Au0SHAnTeI CEwdrgw+SG3vXdp9BnzPn2EYzIiyu6lAfHp4HoRojmmMXhtk2ES+qg/W/Ty6Os6Q wkPVa5W5TOygh6Ux213LyjJlnYl3fOFa3Hmg20Yu3AwmvIy4UDsgQjBqUouDhv2n 2FZPXZ+4XiRuK4zuPrXgfdP9ywYpshY/Yxaj4a+blHOMkViu3G8sWxKLDa90juAl xjiYuevQHmTe6EOomuIfC/gzKszQhz9SnCpycgCtB9UCruHrgoFnHJYQwi760ynf t7anmvUV7jWIUrH+2OQBEa+kEl3uM9fkQJa6x71bMfH6Ob8MYq7ciJv78uM38nfo 3KyGSsb4RR6aiLyLsBJIesmLfYKxMGm/uGd4IJJrvbf3mCH2YoNLPZNPCSKrUXfv tD41GDUaXGSEYKBHYkSg =bck+ -----END PGP SIGNATURE-----