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

Hello Matthieu,

Matthieu Longo <[email protected]> writes:

> On Linux, /proc/<pid> is keyed by the thread-group leader PID. When
> the leader has exited, some /proc/<pid>/... entries become unavailable
> even though another thread is still alive. This can happen, for instance,
> when the main thread calls pthread_exit() and another thread continues
> the execution (existing test: gcore-stale-thread).
>
> This causes GDB to fail to read procfs entries such as cmdline, cwd,
> exe, maps, and smaps when it uses 'current_inferior ()->pid' after the
> thread-group leader has exited.
>
> Fix this by adding inferior::first_non_exited_thread(), which returns
> the PTID of the first non-exited thread of the inferior. Use its LWP ID
> when accessing procfs entries that only need a representative live LWP
> belonging to the process.
>
> This is a best-effort choice of a thread that is expected to still exist
> in the target. Since GDB's view of the threads list may be stale, the
> selected thread may already have exited by the time it is accessed.
> Callers must therefore still be prepared to handle that case.
>
> Update the following functions:
>  - linux_info_proc
>  - linux_process_address_in_memtag_page
>  - linux_find_memory_regions_full
>  - linux_fill_prpsinfo
>  - linux_address_in_shadow_stack_mem_range
> to use the first non-exited thread's LWP ID instead of the inferior PID
> when constructing procfs paths.
>
> Add a new test in gdb.threads.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31207
> ---
>  gdb/inferior.c                                | 12 +++
>  gdb/inferior.h                                | 11 +++
>  gdb/linux-tdep.c                              | 90 +++++++++++++------
>  ...access-procfs-while-thread-leader-exited.c | 48 ++++++++++
>  ...cess-procfs-while-thread-leader-exited.exp | 78 ++++++++++++++++
>  5 files changed, 211 insertions(+), 28 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.threads/access-procfs-while-thread-leader-exited.c
>  create mode 100644 gdb/testsuite/gdb.threads/access-procfs-while-thread-leader-exited.exp

Reviewed-by: Thiago Jung Bauermann <[email protected]>

Just one nit:

> 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 " \

s/where/was/

Though actually IMHO it reads just as well with s/where //.

> +		  "as the thread-group leader (LWP=%ld) already exited.]\n"),
> +		ptid.pid (), ptid.lwp (), leader_ptid.lwp ());
> +  return ptid;
> +}
> +
>  /* This is how we want PTIDs from core files to be printed.  */
>  
>  static std::string

-- 
Thiago
(he/him)