Make follow_fork not rely on get_last_target_status

Currently, if

  - you're in all-stop mode,
  - the inferior last stopped because of a fork catchpoint,

when you next resume the program, gdb checks whether it had last
stopped for a fork/vfork, and if so,

 a) if the current thread is the one that forked, gdb follows the
   parent/child, depending on "set follow-fork" mode.

 b) if the current thread is some other thread (because you switched
   threads meanwhile), gdb switches back to that thread, gdb follows
   the parent/child, and stops the resumption command.

There's a problem in b), however -- if you have "set schedule-multiple
off", which is the default, or "set scheduler-locking on", gdb will
still switch back to the forking thread, even if you didn't want to
resume it.  For example, with:

  (gdb) catch fork
  (gdb) c
  * thread 1 stops for fork
  (gdb) thread 2
  (gdb) set scheduler-locking on
  (gdb) c

gdb switches back to thread 1, and follows the fork.

Or with:

  (gdb) add-inferior -exec prog
  (gdb) inferior 2
  (gdb) start
  (gdb) inferior 1
  (gdb) catch fork
  (gdb) c
  * thread 1.1 stops for fork
  (gdb) inferior 2
  (gdb) set schedule-multiple off # this is the default
  (gdb) c

gdb switches back to thread 1.1, and follows the fork.

Another issue is that, because follow_fork relies on
get_last_target_status to find the thread that has a pending fork, it
is possible to confuse it.  For example, "run" or "start" call
init_wait_for_inferior, which clears the last target status, so this:

  (gdb) catch fork
  (gdb) c
  * thread 1 stops for fork
  (gdb) add-inferior -exec prog
  (gdb) inferior 2
  (gdb) start
  (gdb) set follow-fork child
  (gdb) inferior 1
  (gdb) n

... does not follow to the fork child of inferior 1, because the
get_last_target_status call in follow_fork doesn't return a
TARGET_WAITKIND_FORKED.  Thanks to Simon for this example.

All of the above are fixed by this patch.  It changes follow_fork to
not look at get_last_target_status, but to instead iterate over the
set of threads that the user is resuming, and find the one that has a
pending_follow kind of fork/vfork.

gdb.base/foll-fork.exp is augmented to exercise the last "start"
scenario described above.  The other cases will be exercised in the
testcase added by the following patch.

Change-Id: Ifcca77e7b2456277387f40660ef06cec2b93b97e
This commit is contained in:
Pedro Alves
2022-11-17 18:25:36 +00:00
parent 8e958b8349
commit 43fac29628
2 changed files with 71 additions and 32 deletions

View File

@@ -705,6 +705,15 @@ holding the child stopped. Try \"set detach-on-fork\" or \
return false;
}
/* Set the last target status as TP having stopped. */
static void
set_last_target_status_stopped (thread_info *tp)
{
set_last_target_status (tp->inf->process_target (), tp->ptid,
target_waitstatus {}.set_stopped (GDB_SIGNAL_0));
}
/* Tell the target to follow the fork we're stopped at. Returns true
if the inferior should be resumed; false, if the target for some
reason decided it's best not to resume. */
@@ -729,32 +738,33 @@ follow_fork ()
if (!non_stop)
{
process_stratum_target *wait_target;
ptid_t wait_ptid;
struct target_waitstatus wait_status;
thread_info *cur_thr = inferior_thread ();
/* Get the last target status returned by target_wait(). */
get_last_target_status (&wait_target, &wait_ptid, &wait_status);
ptid_t resume_ptid
= user_visible_resume_ptid (cur_thr->control.stepping_command);
process_stratum_target *resume_target
= user_visible_resume_target (resume_ptid);
/* If not stopped at a fork event, then there's nothing else to
do. */
if (wait_status.kind () != TARGET_WAITKIND_FORKED
&& wait_status.kind () != TARGET_WAITKIND_VFORKED)
return 1;
/* Check if we switched over from WAIT_PTID, since the event was
reported. */
if (wait_ptid != minus_one_ptid
&& (current_inferior ()->process_target () != wait_target
|| inferior_ptid != wait_ptid))
/* Check if there's a thread that we're about to resume, other
than the current, with an unfollowed fork/vfork. If so,
switch back to it, to tell the target to follow it (in either
direction). We'll afterwards refuse to resume, and inform
the user what happened. */
for (thread_info *tp : all_non_exited_threads (resume_target,
resume_ptid))
{
/* We did. Switch back to WAIT_PTID thread, to tell the
target to follow it (in either direction). We'll
afterwards refuse to resume, and inform the user what
happened. */
thread_info *wait_thread = find_thread_ptid (wait_target, wait_ptid);
switch_to_thread (wait_thread);
should_resume = false;
if (tp == cur_thr)
continue;
if (tp->pending_follow.kind () != TARGET_WAITKIND_SPURIOUS)
{
infrun_debug_printf ("need to follow-fork [%s] first",
tp->ptid.to_string ().c_str ());
switch_to_thread (tp);
should_resume = false;
break;
}
}
}
@@ -818,21 +828,16 @@ follow_fork ()
}
else
{
/* This makes sure we don't try to apply the "Switched
over from WAIT_PID" logic above. */
nullify_last_target_wait_ptid ();
/* If we followed the child, switch to it... */
if (follow_child)
{
thread_info *child_thr = find_thread_ptid (parent_targ, child);
switch_to_thread (child_thr);
tp = find_thread_ptid (parent_targ, child);
switch_to_thread (tp);
/* ... and preserve the stepping state, in case the
user was stepping over the fork call. */
if (should_resume)
{
tp = inferior_thread ();
tp->control.step_resume_breakpoint
= step_resume_breakpoint;
tp->control.step_range_start = step_range_start;
@@ -871,6 +876,8 @@ follow_fork ()
break;
}
if (!should_resume)
set_last_target_status_stopped (tp);
return should_resume;
}

View File

@@ -212,15 +212,22 @@ set reading_in_symbols_re {(?:\r\nReading in symbols for [^\r\n]*)?}
# Test the ability to catch a fork, specify that the child be
# followed, and continue. Make the catchpoint permanent.
proc_with_prefix catch_fork_child_follow {} {
proc_with_prefix catch_fork_child_follow {second_inferior} {
global gdb_prompt
global srcfile
global reading_in_symbols_re
if { $second_inferior && [use_gdb_stub] } {
return
}
if { ![setup] } {
return
}
# Get rid of the breakpoint at "main".
delete_breakpoints
set bp_after_fork [gdb_get_line_number "set breakpoint here"]
gdb_test "catch fork" \
@@ -249,6 +256,29 @@ proc_with_prefix catch_fork_child_follow {} {
"Temporary breakpoint.*, line $bp_after_fork.*" \
"set follow-fork child, tbreak"
if {$second_inferior} {
gdb_test "add-inferior" "Added inferior 2.*" "add inferior 2"
gdb_test "inferior 2" "Switching to inferior 2.*"
gdb_load $::binfile
# Start it. This should not affect inferior 1, given "set
# schedule-multiple off" (default). GDB used to have a bug
# where "start" would clear the pending follow fork
# information of inferior 1.
gdb_test "start" "Starting program.*Temporary breakpoint .*"
gdb_test "inferior 1" "Switching to inferior 1.*"
# Verify that the catchpoint is still mentioned in an "info
# breakpoints", and further that the catchpoint still shows
# the captured process id.
gdb_test "info breakpoints" \
".*catchpoint.*keep y.*fork, process.*" \
"info breakpoints, after starting second inferior"
}
set expected_re "\\\[Attaching after.* fork to.*\\\[Detaching after fork from"
append expected_re ".* at .*$bp_after_fork.*"
gdb_test "continue" $expected_re "set follow-fork child, hit tbreak"
@@ -431,6 +461,8 @@ foreach_with_prefix follow-fork-mode {"parent" "child"} {
# Catchpoint tests.
catch_fork_child_follow
foreach_with_prefix second_inferior {false true} {
catch_fork_child_follow $second_inferior
}
catch_fork_unpatch_child
tcatch_fork_parent_follow