diff options
author | Kevin Buettner <kevinb@redhat.com> | 2025-02-05 11:27:00 -0700 |
---|---|---|
committer | Kevin Buettner <kevinb@redhat.com> | 2025-02-05 11:28:29 -0700 |
commit | e5501dd4321a6b63f306b292347e5d22058c3ed2 (patch) | |
tree | 998b9c50070f28c6101d8326f7a3b2a3ab854d1a /gdb/linux-nat.c | |
parent | b425859021d17adf62f06fb904797cf8642986ad (diff) | |
download | binutils-e5501dd4321a6b63f306b292347e5d22058c3ed2.zip binutils-e5501dd4321a6b63f306b292347e5d22058c3ed2.tar.gz binutils-e5501dd4321a6b63f306b292347e5d22058c3ed2.tar.bz2 |
Make linux checkpoints work with multiple inferiors
The current linux checkpoint code, most of which may be found in
linux-fork.c, is quite broken when attempting to use more than
one inferior. Running GDB will show internal errors when starting
two inferiors, placing a checkpoint in one, then switching to
the other and doing one of the following commands, "restart",
"detach", "kill", or continue (to program exit). Test cases
for two of those scenarios may be found in this bug:
https://sourceware.org/bugzilla/show_bug.cgi?id=31065
I've tested for each of the scenarios and many more in the new
test case, gdb.multi/checkpoint-multi.exp.
I started off with the goal of fixing just those problems, and was
mostly successful with a much smaller patch, but doing "info
checkpoints" with more than one inferior didn't work correctly due to
some of the inferiors being in the wrong program space. That led me
to making the linux-fork code fully inferior-aware.
Prior to this commit, the list of forks was being maintained in a
global named named 'fork_list'. I turned this into a per-inferior
data structure. There was also global named 'highest_fork_num' which
is also now part of the per-inferior struct. A registry key named
'checkpoint_inferior_data_key' along with function
'get_checkpoint_inferior_data' is used to access the per-inferior
data. This new function, get_checkpoint_inferior_data, is only
called by the new functions 'fork_list', 'reset_highest_fork_num',
and increment_highest_fork_num, each of which is passed a pointer to
the inferior. Most occurrences referring to the (previously) global
'fork_list' have been replaced by 'fork_list (inf)'. In some
functions, where the 'fork_list' is referenced multiple times, a local
named 'fork_list' is declared and initialized instead, like this:
auto &fork_list = ::fork_list (inf);
The constructor for 'struct fork_info' has gained an additional
parameter. In addition to passing the pid of the new fork, we now
also pass the fork identifier, fork_num, to the constructor. This
integer is shown to the user in the "info checkpoints" command and
is provided by the user, perhaps in conjunction with the inferior
number, in commands which manipulate checkpoints, e.g. 'restart' and
'delete checkpoint'.
When checkpoints are used in only one inferior, this commit will
present information to the user and will accept checkpoint identifiers
to commands in much the same way as the code did before this commit.
Per Pedro Alves's recommendations, the "info checkpoints" command has
been changed somewhat. "info checkpoints" used to display "(main
process)" for the first process in the checkpoint list. This is no
longer done because it does not provide useful information. It also
used to display "<running>", when the process is running and no useful
frame information may be displayed. This has been changed to
"(running)" in order to be more consistent with the output of the
"info threads" command. A new column has been added to the output for
showing the active process in the output from "info checkpoints".
This column will display 'y' for the active process and 'n' for the
others. For the active inferior a '*' is also printed preceding the
checkpoint identifier. Here's what things look(ed) like before and
after for just one inferior:
Before:
(gdb) info checkpoints
* 0 Thread 0x7ffff7cd3740 (LWP 84201) (main process) at 0x40114a, file hello.c, line 28
1 process 84205 at 0x401199, file hello.c, line 51
2 process 84206 at 0x4011a3, file hello.c, line 53
After:
(gdb) info checkpoints
Id Active Target Id Frame
* 0 y process 551311 at 0x40114a, file hello.c, line 28
1 n process 551314 at 0x401199, file hello.c, line 51
2 n process 551315 at 0x4011a3, file hello.c, line 53
(The Thread versus process distinction is handled by another
patch - the "After" example assumes that patch is applied too.)
When there are multiple inferiors, the "info checkpoints" output looks
like this:
(gdb) info checkpoints
Id Active Target Id Frame
1.0 y process 535276 at 0x401199, file hello.c, line 51
1.1 n process 535283 at 0x401199, file hello.c, line 51
1.2 n process 535288 at 0x401199, file hello.c, line 51
2.1 n process 535280 at 0x401258, file goodbye.c, line 62
2.2 y process 535284 at 0x401258, file goodbye.c, line 62
* 3.0 y process 535285 at 0x40115c, file hangout.c, line 31
3.2 n process 535287 at 0x40115c, file hangout.c, line 31
A new function named 'parse_checkpoint_id' has been added. As its
name suggests, it's responsible for parsing a string representing a
checkpoint identifier. These identifiers may be either a decimal
number representing the checkpoint number in the current inferior or
two decimal numbers separated by '.', in which case the first is the
inferior number and the second is the checkpoint number in that
inferior. It is called by delete_checkpoint_command,
detach_checkpoint_command, info_checkpoints_command, and
restart_command. Calls to 'parse_checkpoint_id' replace calls to
'parse_and_eval_long', plus error checking and error reporting code
near the calls to 'parse_and_eval_long'. As such, error checking and
reporting has been consolidated into a single function and the
messages output are more uniform, though this has necessitated changes
to the existing test case gdb.base/checkpoint.exp.
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 pointer to a fork_info struct just as 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
identifiers incorporating an inferior number may be printed.
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.
gdb/linux-nat.c has a few changes. All but one of them are related
to passing the inferior to one of the linux-fork functions. But
one of the tests in linux_nat_target::detach has also changed in
a non-obvious way. In attempting to determine whether to call
linux_fork_detach(), that code used to do:
if (pid == inferior_ptid.pid () && forks_exist_p ())
It's been simplified to:
if (forks_exist_p (inf))
I had added the 'pid == inferior_ptid.pid ()' condition in late 2023
while working on a detach bug. It was kind of a hack to prevent
calling linux_fork_detach() when in a different inferior. That's no
longer needed since the call to forks_exist_p does this directly -
i.e. it is now inferior-aware.
Finally, the header file 'linux-fork.h' has been updated to reflect
the fact that add_fork, linux_fork_killall, linux_fork_detach, and
forks_exist_p all now require that a pointer to an inferior be passed
to these functions. Additionally (as mentioned earlier),
find_fork_pid now returns std::pair<fork_info *, inferior *> instead
'of fork_info *'.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31065
Reviewed-By: Tom Tromey <tom@tromey.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
Diffstat (limited to 'gdb/linux-nat.c')
-rw-r--r-- | gdb/linux-nat.c | 18 |
1 files changed, 11 insertions, 7 deletions
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 3f25237..b16f9f9 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -1558,13 +1558,13 @@ linux_nat_target::detach (inferior *inf, int from_tty) gdb_assert (num_lwps (pid) == 1 || (target_is_non_stop_p () && num_lwps (pid) == 0)); - if (pid == inferior_ptid.pid () && forks_exist_p ()) + if (forks_exist_p (inf)) { /* Multi-fork case. The current inferior_ptid is being detached from, but there are other viable forks to debug. Detach from the current fork, and context-switch to the first available. */ - linux_fork_detach (from_tty, find_lwp_pid (ptid_t (pid))); + linux_fork_detach (from_tty, find_lwp_pid (ptid_t (pid)), inf); } else { @@ -2082,8 +2082,12 @@ linux_handle_extended_wait (struct lwp_info *lp, int status) detach_breakpoints (ptid_t (new_pid, new_pid)); /* Retain child fork in ptrace (stopped) state. */ - if (!find_fork_pid (new_pid)) - add_fork (new_pid); + if (find_fork_pid (new_pid).first == nullptr) + { + struct inferior *inf = find_inferior_ptid (linux_target, + lp->ptid); + add_fork (new_pid, inf); + } /* Report as spurious, so that infrun doesn't want to follow this fork. We're actually doing an infcall in @@ -3729,8 +3733,8 @@ linux_nat_target::kill () parent will be sleeping if this is a vfork. */ iterate_over_lwps (pid_ptid, kill_unfollowed_child_callback); - if (forks_exist_p ()) - linux_fork_killall (); + if (forks_exist_p (current_inferior ())) + linux_fork_killall (current_inferior ()); else { /* Stop all threads before killing them, since ptrace requires @@ -3761,7 +3765,7 @@ linux_nat_target::mourn_inferior () close_proc_mem_file (pid); - if (! forks_exist_p ()) + if (! forks_exist_p (current_inferior ())) /* Normal case, no other forks available. */ inf_ptrace_target::mourn_inferior (); else |