public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Make `linux_info_proc` prefer using the LWP over the PID
@ 2024-01-06  2:45 Matheus Branco Borella
  2024-01-08 15:50 ` Simon Marchi
  2024-01-19 16:49 ` [PATCH v2] " Matheus Branco Borella
  0 siblings, 2 replies; 4+ messages in thread
From: Matheus Branco Borella @ 2024-01-06  2:45 UTC (permalink / raw)
  To: gdb-patches; +Cc: Matheus Branco Borella

Fixes: https://sourceware.org/bugzilla/show_bug.cgi?id=31207

Normally, `linux_info_proc` would use the PID to determine which subfolder in
`/proc` to read information from. While this is usually fine, it breaks down
after the main thread exits, at which point the information in `/proc/$pid`
becomes become unreliable, if it is available at all. While it is the case
that most programs terminate after their main thread exits, some may continue
running from detached threads, in which case `info proc` will start misbehaving.

This patch addresses this by making it so that the LWP - the Lightweight Process
ID, that, in the case of GNU/Linux is the number of the process backing up the
thread[1] - is prefered over the PID. By doing this, `linux_info_proc` will
always access valid procfs information, even after the main thread exits.

[1]: https://man7.org/linux/man-pages/man2/clone.2.html
---
 gdb/linux-tdep.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 82e8bc3db3c..2c91e298d45 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -840,7 +840,14 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
       if (current_inferior ()->fake_pid_p)
 	error (_("Can't determine the current process's PID: you must name one."));
 
-      pid = current_inferior ()->pid;
+      /* Seeing as, when the main thread exits, the information in /proc/$pid
+       * becomes unreliable, we should prefer using the current TID, whenever
+       * possible. */
+      pid = inferior_ptid.lwp ();
+
+      /* And fall back to the actual PID only when the TID is not available. */
+      if (pid == 0)
+	pid = current_inferior ()->pid;
     }
 
   args = skip_spaces (args);
-- 
2.40.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Make `linux_info_proc` prefer using the LWP over the PID
  2024-01-06  2:45 [PATCH] Make `linux_info_proc` prefer using the LWP over the PID Matheus Branco Borella
@ 2024-01-08 15:50 ` Simon Marchi
  2024-01-19 16:52   ` Matheus Branco Borella
  2024-01-19 16:49 ` [PATCH v2] " Matheus Branco Borella
  1 sibling, 1 reply; 4+ messages in thread
From: Simon Marchi @ 2024-01-08 15:50 UTC (permalink / raw)
  To: Matheus Branco Borella, gdb-patches



On 2024-01-05 21:45, Matheus Branco Borella wrote:
> Fixes: https://sourceware.org/bugzilla/show_bug.cgi?id=31207

We use `Bug:` for these.  Also, move it at the end of the commit
message, like standard git trailers
(https://git-scm.com/docs/git-interpret-trailers).

> Normally, `linux_info_proc` would use the PID to determine which subfolder in
> `/proc` to read information from. While this is usually fine, it breaks down
> after the main thread exits, at which point the information in `/proc/$pid`
> becomes become unreliable, if it is available at all. While it is the case
> that most programs terminate after their main thread exits, some may continue
> running from detached threads, in which case `info proc` will start misbehaving.
> 
> This patch addresses this by making it so that the LWP - the Lightweight Process
> ID, that, in the case of GNU/Linux is the number of the process backing up the
> thread[1] - is prefered over the PID. By doing this, `linux_info_proc` will
> always access valid procfs information, even after the main thread exits.
> 
> [1]: https://man7.org/linux/man-pages/man2/clone.2.html
> ---
>  gdb/linux-tdep.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index 82e8bc3db3c..2c91e298d45 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -840,7 +840,14 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
>        if (current_inferior ()->fake_pid_p)
>  	error (_("Can't determine the current process's PID: you must name one."));
>  
> -      pid = current_inferior ()->pid;
> +      /* Seeing as, when the main thread exits, the information in /proc/$pid
> +       * becomes unreliable, we should prefer using the current TID, whenever
> +       * possible. */
> +      pid = inferior_ptid.lwp ();
> +
> +      /* And fall back to the actual PID only when the TID is not available. */
> +      if (pid == 0)
> +	pid = current_inferior ()->pid;

I would suggest trying to use the any_live_thread_of_inferior function
to get a non-exited thread.   This way, if the current thread has
exited, it will find another that should be suitable for reading the
proc information.

I can imagine another case where thing would go wrong.  There might be
threads which have exited, but for which we have not processed the
"exited" event yet.  The exited state will not yet be reflected in the
thread_info structure, so we might pick it thinking it's a live thread.
But I would ignore that problem for now, what you propose is already a
good improvement over the current state.

Simon

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] Make `linux_info_proc` prefer using the LWP over the PID
  2024-01-06  2:45 [PATCH] Make `linux_info_proc` prefer using the LWP over the PID Matheus Branco Borella
  2024-01-08 15:50 ` Simon Marchi
@ 2024-01-19 16:49 ` Matheus Branco Borella
  1 sibling, 0 replies; 4+ messages in thread
From: Matheus Branco Borella @ 2024-01-19 16:49 UTC (permalink / raw)
  To: gdb-patches; +Cc: simark, Matheus Branco Borella

Normally, `linux_info_proc` would use the PID to determine which subfolder in
`/proc` to read information from. While this is usually fine, it breaks down
after the main thread exits, at which point the information in `/proc/$pid`
becomes become unreliable, if it is available at all. While it is the case
that most programs terminate after their main thread exits, some may continue
running from detached threads, in which case `info proc` will start misbehaving.

This patch addresses this by making it so that the LWP - the Lightweight Process
ID, that, in the case of GNU/Linux is the number of the process backing up the
thread[1] - is prefered over the PID. By doing this, `linux_info_proc` will
always access valid procfs information, even after the main thread exits.

[1]: https://man7.org/linux/man-pages/man2/clone.2.html

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31207
---
 gdb/linux-tdep.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 82e8bc3db3c..4fa7a98adde 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -840,7 +840,17 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
       if (current_inferior ()->fake_pid_p)
 	error (_("Can't determine the current process's PID: you must name one."));
 
-      pid = current_inferior ()->pid;
+      /* Seeing as, when the main thread exits, the information in /proc/$pid
+       * becomes unreliable, we should prefer using the current TID, whenever
+       * possible. */
+      pid = 0;
+      struct thread_info *info = any_live_thread_of_inferior (current_inferior ());
+      if (info != nullptr)
+	pid = info->ptid.lwp ();
+
+      /* And fall back to the actual PID only when the TID is not available. */
+      if (pid == 0)
+	pid = current_inferior ()->pid;
     }
 
   args = skip_spaces (args);
-- 
2.40.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Make `linux_info_proc` prefer using the LWP over the PID
  2024-01-08 15:50 ` Simon Marchi
@ 2024-01-19 16:52   ` Matheus Branco Borella
  0 siblings, 0 replies; 4+ messages in thread
From: Matheus Branco Borella @ 2024-01-19 16:52 UTC (permalink / raw)
  To: gdb-patches; +Cc: dark.ryu.550, simark

I've sent in a v2 that should address your points. Thanks for your time.


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-01-19 16:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-06  2:45 [PATCH] Make `linux_info_proc` prefer using the LWP over the PID Matheus Branco Borella
2024-01-08 15:50 ` Simon Marchi
2024-01-19 16:52   ` Matheus Branco Borella
2024-01-19 16:49 ` [PATCH v2] " Matheus Branco Borella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).