Re: [PATCH v3 3/3] Make thread_db_target::pid_to_str checkpoint-aware

[Pedro Alves <pedro@palves.net>]

On 2024-04-14 20:44, Kevin Buettner wrote:
> This commit prevents thread_db_target::pid_to_str from considering
> a checkpoint as a thread.  The reason for doing this is that pids
> associated with checkpoints can never be a thread due to the fact
> that checkpoints (which are implemented by forking a process) can
> only work with single-threaded processes.

Well, that isn't strictly true, as a checkpoint can spawn a new
thread/clone.   Or even exec...  But let's not go there...

> 
> Without this commit, many of the "info checkpoints" commands
> in gdb.multi/checkpoint-multi.exp will incorrectly show some
> of the checkpoints as threads.  E.g...
> 
> *  1.0 A  Thread 0x7ffff7cd3740 (LWP 128534) at 0x401199, file hello.c, line 51
>    1.2    process 128546 at 0x401199, file hello.c, line 51
>    1.3    process 128547 at 0x401199, file hello.c, line 51
>    2.1    process 128538 at 0x401258, file goodbye.c, line 62
>    2.2 A  Thread 0x7ffff7cd3740 (LWP 128542) at 0x401258, file goodbye.c, line 62
>    3.0 A  Thread 0x7ffff7cd3740 (LWP 128543) at 0x40115c, file hangout.c, line 31
>    3.2    process 128545 at 0x40115c, file hangout.c, line 31
> 
> With this commit in place, the output looks like this instead:
> 
> *  1.0 A  process 129961 at 0x401199, file hello.c, line 51
>    1.2    process 129974 at 0x401199, file hello.c, line 51
>    1.3    process 129975 at 0x401199, file hello.c, line 51
>    2.1    process 129965 at 0x401258, file goodbye.c, line 62
>    2.2 A  process 129969 at 0x401258, file goodbye.c, line 62
>    3.0 A  process 129970 at 0x40115c, file hangout.c, line 31
>    3.2    process 129972 at 0x40115c, file hangout.c, line 31
> 
> (For brevity, I've removed the directory elements in each of the paths
> above.)
> 
> The testcase, gdb.multi/checkpoint-multi.exp, has been updated to
> reflect the fact that only "process" should now appear in output
> from "info checkpoints".
> ---
>  gdb/linux-thread-db.c                        | 4 +++-
>  gdb/testsuite/gdb.multi/checkpoint-multi.exp | 2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
> index 65bf4a79fdf..488a7086acf 100644
> --- a/gdb/linux-thread-db.c
> +++ b/gdb/linux-thread-db.c
> @@ -48,6 +48,7 @@
>  #include "gdbsupport/pathstuff.h"
>  #include "valprint.h"
>  #include "cli/cli-style.h"
> +#include "linux-fork.h"
>  
>  /* GNU/Linux libthread_db support.
>  
> @@ -1657,7 +1658,8 @@ thread_db_target::pid_to_str (ptid_t ptid)
>  {
>    thread_info *thread_info = current_inferior ()->find_thread (ptid);
>  
> -  if (thread_info != NULL && thread_info->priv != NULL)
> +  if (thread_info != NULL && thread_info->priv != NULL
> +      && !forks_exist_p (current_inferior ()))
>      {
>        thread_db_thread_info *priv = get_thread_db_thread_info (thread_info);
>  

I think that it's better to change linux-fork.c, avoid special casing checkpoints
if we can.  If linux-fork.c wants to print a process, then it should pass down a
process ptid.  Like:

diff --git c/gdb/linux-fork.c w/gdb/linux-fork.c
index e35e438cabc..9003b43dc82 100644
--- c/gdb/linux-fork.c
+++ w/gdb/linux-fork.c
@@ -807,7 +807,8 @@ info_checkpoints_command (const char *arg, int from_tty)
          else
            gdb_printf ("   ");
 
-         gdb_printf ("%s", target_pid_to_str (fi.ptid).c_str ());
+         ptid_t pid_ptid (fi.ptid.pid ());
+         gdb_printf ("%s", target_pid_to_str (pid_ptid).c_str ());
 
          gdb_printf (_(" at "));
          ULONGEST pc