Re: [PATCH v1 2/2] gdb: rely on the first non-exited thread TPID when reading Linux procfs files

Simon Marchi <[email protected]>
Newsgroups gmane.comp.gdb.patches
Message-ID <[email protected]>
On 7/28/26 10:33 AM, Matthieu Longo wrote:
> diff --git a/gdb/inferior.h b/gdb/inferior.h
> index 5c7a52319e7..91dc4e636fd 100644
> --- a/gdb/inferior.h
> +++ b/gdb/inferior.h
> @@ -513,6 +513,17 @@ class inferior : public refcounted_object,
>    /* Find (non-exited) thread PTID of this inferior.  */
>    thread_info *find_thread (ptid_t ptid);
>  
> +  /* Return the first non-exited thread of this inferior.
> +
> +     This is only a best-effort choice of a thread that is expected to still
> +     exist in the target.  A thread may have exited after GDB last updated its
> +     thread list, or GDB/gdbserver may have observed the exit but not yet
> +     propagated it through all layers.  Therefore the returned thread is not
> +     guaranteed to still be alive when it is later accessed.  This avoids the
> +     common case where the current thread has exited, but callers must still be
> +     prepared for the selected thread to no longer exist.  */
> +  ptid_t first_non_exited_thread () const;

I think it would be more consistent with the other "find thread" methods
to return the `thread_info *`

> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index 8c53ffd5e89..9bdcc55a0e1 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -457,6 +457,44 @@ linux_has_shared_address_space (struct gdbarch *gdbarch)
>    return linux_is_uclinux ();
>  }
>  
> +/* Return a PTID that identifies the current process and can be used to
> +   access procfs safely.
> +
> +   The returned PTID is that of the thread-group leader whenever it is
> +   still alive.  If the leader has already exited, the PTID of the first
> +   non-exited thread in the current inferior is returned instead.
> +   This ensures that the returned PTID always refers to a live thread
> +   whose procfs entries are present and populated.  */
> +static ptid_t
> +get_process_reference_ptid (bool verbose = false)
> +{
> +  /* Get the current thread.  */
> +  thread_info *thr = inferior_thread ();
> +
> +  /* Construct the PTID of the thread-group leader.  On Linux,
> +     the leader's LWP ID is equal to the process ID.  */
> +  ptid_t leader_ptid (thr->ptid.pid (), thr->ptid.pid ());
> +
> +  /* Use the thread-group leader if it is still alive.  Otherwise, use
> +     the first thread that has not exited.  */
> +  thread_info *leader_thr
> +    = current_inferior ()->find_thread (leader_ptid);
> +  ptid_t ptid = (leader_thr == nullptr
> +		 ? current_inferior ()->first_non_exited_thread ()
> +		 : leader_ptid);
> +
> +  if (!verbose)
> +    return ptid;
> +
> +  if (leader_thr != nullptr)
> +    gdb_printf (_("process %d\n"), leader_ptid.pid ());
> +  else
> +    gdb_printf (_("process %d [Note: information where gathered from LWP %ld " \
> +		  "as the thread-group leader (LWP=%ld) already exited.]\n"),
> +		ptid.pid (), ptid.lwp (), leader_ptid.lwp ());

I don't think the printfs should be part of this getter function.  Move
it to a separate function called by whoever needs it.  Or just inline
it, it looks like there is just a single caller that needs it
(linux_info_proc).

And really (IIUC), this function only really needs to return a tid (the
id of one LWP), not a ptid.

Simon
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.