Re: [PATCH] Warn users about mismatched PID namespaces

Pedro Alves
2014-11-07T12:49:57Z

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 (&current_target) || core_bfd))
return 0;

[1] BTW, could I interest in giving gdbserver/thread-db.c the
same treatment?

> + /* 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.

> + 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.

> +
> 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 .

>
> +/* 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 *.

Thanks,
Pedro Alves