Re: [PATCH] Warn users about mismatched PID namespaces

[Pedro Alves <palves@redhat.com>]

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