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