Re: [PATCH v4 1/4] Make linux checkpoints work with multiple inferiors

[Pedro Alves <pedro@palves.net>]

Hi Kevin!

Trying v4 before looking at the code, I saw one behavior that
surprised me.  That is when you have two inferiors, and checkpoints
in only one inferior.  Like so:

 (gdb) info checkpoints 
 No checkpoints.
 (gdb) checkpoint 
 Checkpoint 1: fork returned pid 66326.
 (gdb) info checkpoints 
 *  0 A  process 66173 at 0x555555555258, file threads.c, line 53
    1    process 66326 at 0x555555555258, file threads.c, line 53
 (gdb) add-inferior 
 [New inferior 2]
 Added inferior 2 on connection 1 (native)

Now in the following command, I was expecting to see 1.0 and 1.1
as checkpoint numbers, since we now have two inferiors, but we only
get the 0 and 1:

 (gdb) info checkpoints 
 *  0 A  process 66173 at 0x555555555258, file threads.c, line 53
    1    process 66326 at 0x555555555258, file threads.c, line 53


If I switch to inferior 2, and then list checkpoints, I do get
the fully-qualified checkpoint ids:

 (gdb) info inferiors 
   Num  Description       Connection           Executable        
 * 1    process 66173     1 (native)           /net/cascais.nfs/brno/pedro/gdb/tests/threads 
   2    <null>            1 (native)                             
 (gdb) inferior 2
 [Switching to inferior 2 [<null>] (<noexec>)]
 (gdb) info checkpoints 
    1.0 A  process 66173 at 0x55555258, file threads.c, line 53
    1.1    process 66326 at 0x55555258, file threads.c, line 53

I was expecting to see the same output when inferior 1 is the
one selected.  Again, going back to inferior 1:

 (gdb) inferior 1
 [Switching to inferior 1 [process 66173] (/net/cascais.nfs/brno/pedro/gdb/tests/threads)]
 [Switching to thread 1.1 (Thread 0x7ffff7f93740 (LWP 66173))]
 #0  main () at threads.c:53
 53      int main() {
 (gdb) info checkpoints 
 *  0 A  process 66173 at 0x555555555258, file threads.c, line 53
    1    process 66326 at 0x555555555258, file threads.c, line 53
 (gdb) 

Now that I look at the code, I see that the predicate used to
determine whether to show the inferior number is checking if
there are multiple inferiors with checkpoints, so it seems
intentional.  But FYI, coming at this with a lot of context
from previous discussion swapped out, I found it confusing.


On the "R" state, thinking some more (since our last discussion),
I wonder if we really need it.  In "info threads", we show that
the thread is running in the "frame" column, like:

 (gdb) info threads 
   Id   Target Id                                    Frame 
 * 1    Thread 0x7ffff7f8e740 (LWP 439463) "infloop" (running)
                                                     ^^^^^^^^^

Maybe we should just reuse the frame-printing code from "info threads".

I tried to reproduce the "R" state here locally, and saw that
it's broken, for trying to fetch registers of a running thread:

 (gdb) start
 ...
 (gdb) checkpoint
 ...
 (gdb) info checkpoints 
 *  0 A  process 439463 at 0x7ffff7ce578a, file ../sysdeps/unix/sysv/linux/clock_nanosleep.c, line 78
    1    process 439827 at 0x7ffff7ce578a, file ../sysdeps/unix/sysv/linux/clock_nanosleep.c, line 78
 (gdb) c&
 Continuing.
 (gdb) info checkpoints 
 *  0 AR process 439463 at Couldn't get registers: No such process.


BTW, the documentation patch says that you can only see the "R" state
in non-stop mode:

 +one which @value{GDBN} is currently debugging.  @code{R} indicates a
 +running process; this status letter can only appear when @value{GDBN}
 +is running in non-stop mode.  

But that's not correct.  You can also use the "c&" command in
all-stop mode.

I noticed this because I wanted to try what it say here:

                               If neither @code{A} or @code{R} are present,
 +this indicates a checkpoint which can be switched to using the
 +@code{restart} command or deleted using the @code{delete checkpoint}
 +command.

... specifically, I wanted to try switching to a checkpoint in the "R"
state, check that GDB behaves properly.

Other than the issues above, this all LGTM.  Some trivial typos
pointed out below.

Pedro Alves

On 2024-06-26 02:55, Kevin Buettner wrote:

> The functions 'find_fork_ptid' and 'find_fork_pid' used to return a
> pointer to a fork_info struct.  They now return a pair consisting of
> the pointer to a fork_info struct in addition to a pointer to the
> inferior containing that checkpoint.
> 
> 'find_fork_id' returns a a pointer to a fork_info struct just as

Typo, double "a a".


> it did before, but it's now gained a new parameter, 'inf', which
> is the inferior in which to look.
> 
> info_checkpoints_command used to simply iterate over the list of
> forks (checkpoints), printing each one out.  It now needs to iterate
> over all inferiors and, for those which have checkpoints, it needs
> to iterate over the list of checkpoints in that inferior.  As noted
> earlier, the format of the output has been changed so that checkpoint
> identifers incorporating an inferior number may be printed.

identifers -> identifiers

> 
> linux_fork_context, called by restart_command, now contains code to
> switch inferiors when the fork being restarted is in an inferior which
> is different from the current one.  The scoped_switch_fork_info class
> now also contains code for switching inferiors in both the constructor
> and destructor.
>