Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simark@simark.ca>
To: Matthieu Longo <matthieu.longo@arm.com>, gdb-patches@sourceware.org
Cc: Luis Machado <luis.machado@amd.com>,
	Luis Machado <luis.machado.foss@gmail.com>,
	Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Subject: Re: [PATCH v1 2/2] gdb: rely on the first non-exited thread TPID when reading Linux procfs files
Date: Thu, 20 Aug 2026 11:48:20 -0400	[thread overview]
Message-ID: <e816857d-2c02-4987-b49c-b6c4debb24ed@simark.ca> (raw)
In-Reply-To: <20260728143317.245389-3-matthieu.longo@arm.com>

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

  parent reply	other threads:[~2026-08-20 15:48 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-28 14:33 [PATCH v1 0/2] gdb: fix for bug 31207 Matthieu Longo
2026-07-28 14:33 ` [PATCH v1 1/2] gdb: rename any_thread_of_inferior to any_non_exited_thread_of_inferior Matthieu Longo
2026-08-20 15:37   ` Simon Marchi
2026-08-24 14:17     ` Matthieu Longo
2026-07-28 14:33 ` [PATCH v1 2/2] gdb: rely on the first non-exited thread TPID when reading Linux procfs files Matthieu Longo
2026-08-03  4:48   ` Thiago Jung Bauermann
2026-08-13 15:13     ` Matthieu Longo
2026-08-20 15:48   ` Simon Marchi [this message]
2026-08-24 15:29     ` Matthieu Longo
2026-08-24 16:25       ` Matthieu Longo
2026-08-24 16:27         ` Matthieu Longo
2026-08-20 11:09 ` [PATCH v1 0/2] gdb: fix for bug 31207 Matthieu Longo
2026-08-21 15:39   ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e816857d-2c02-4987-b49c-b6c4debb24ed@simark.ca \
    --to=simark@simark.ca \
    --cc=gdb-patches@sourceware.org \
    --cc=luis.machado.foss@gmail.com \
    --cc=luis.machado@amd.com \
    --cc=matthieu.longo@arm.com \
    --cc=thiago.bauermann@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox