From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1551) id 20A6A3858C50; Mon, 27 Feb 2023 19:14:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 20A6A3858C50 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1677525287; bh=fVu6rz7mhUvEYUUUwzjTSGaY932PHtjCel00uTCzy1k=; h=From:To:Subject:Date:From; b=IyQeVPa1rFQxD62cvKmdk+ZlEc6NL0JFOraO8DV/QyJzJJJgSAQF5iBQgfLHK37yA dTQlQNwUfF4+bQpZtny/iMv+v6s+3qcZi3qjVP3kPMAUglS3ovn+8ZWXTTqXQyhkjG ObR9212DBz4XDmtcYBnuPpS0GFisSmVdI6QPIGxo= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Pedro Alves To: gdb-cvs@sourceware.org Subject: [binutils-gdb] all-stop "follow-fork parent" and selecting another thread X-Act-Checkin: binutils-gdb X-Git-Author: Pedro Alves X-Git-Refname: refs/heads/master X-Git-Oldrev: 3505d4c4f7e2156837c7bfcbecf0a03b202e7ffb X-Git-Newrev: bd9482bca71e7bf4149a319a95d6fec4589c3758 Message-Id: <20230227191447.20A6A3858C50@sourceware.org> Date: Mon, 27 Feb 2023 19:14:47 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Dbd9482bca71e= 7bf4149a319a95d6fec4589c3758 commit bd9482bca71e7bf4149a319a95d6fec4589c3758 Author: Pedro Alves Date: Fri Nov 25 16:20:22 2022 +0000 all-stop "follow-fork parent" and selecting another thread =20 With: =20 - catch a fork in thread 1 - select thread 2 - set follow-fork child - next =20 ... follow_fork notices that thread 1 had last stopped for a fork which hasn't been followed yet, and because thread 1 is not the current thread, GDB aborts the execution command, presenting the stop in thread 1. =20 That makes sense, as only the forking thread (thread 1) survives in the child, so better stop and let the user decide how to proceed. =20 However, with: =20 - catch a fork in thread 1 - select thread 2 - set follow-fork parent << note difference here - next =20 ... GDB does the same: follow_fork notices that thread 1 had last stopped for a fork which hasn't been followed yet, and because thread 1 is not the current thread, GDB aborts the execution command, presenting the stop in thread 1. =20 Aborting/stopping in this case doesn't make sense to me. As we're following the parent, thread 2 will still continue to exist in the parent. What the child does after we've followed the parent shouldn't matter -- it can go on running free, be detached, etc., depending on "set schedule-multiple", "set detach-on-fork", etc. That does not influence the execution command that the user issued for the parent thread. =20 So this patch changes GDB in that direction -- in follow_fork, if following the parent, and we've switched threads meanwhile, switch back to the unfollowed thread, follow it (stay with the parent), and don't abort/stop. If we're following a fork (as opposed to vfork), then switch back again to the thread that the user was trying to resume. If following a vfork, however, stay with the vforking-thread selected, as we will need to see a vfork_done event first, before we can resume any other thread. =20 As I was working on this, I managed to end up calling target_resume for a solo-thread resume (to collect the vfork_done event), with scope_ptid pointing at the vfork parent thread, and inferior_ptid pointing to the vfork child. For a solo-thread resume, the scope_ptid argument to target_resume must the same as inferior_ptid. The mistake was caught by the assertion in target_resume, like so: =20 ... [infrun] resume_1: step=3D0, signal=3DGDB_SIGNAL_0, trap_expected=3D0= , current thread [1722839.1722839.0] at 0x5555555553c3 [infrun] do_target_resume: resume_ptid=3D1722839.1722939.0, step=3D0,= sig=3DGDB_SIGNAL_0 ../../src/gdb/target.c:2661: internal-error: target_resume: Assertion `= inferior_ptid.matches (scope_ptid)' failed. ... =20 but I think it doesn't hurt to catch such a mistake earlier, hence the change in internal_resume_ptid. =20 Change-Id: I896705506a16d2488b1bfb4736315dd966f4e412 Diff: --- gdb/infrun.c | 88 +++++++++-- gdb/testsuite/gdb.threads/foll-fork-other-thread.c | 84 ++++++++++ .../gdb.threads/foll-fork-other-thread.exp | 172 +++++++++++++++++= ++++ 3 files changed, 335 insertions(+), 9 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index 04d76895f5e..beb9ca79389 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -757,13 +757,56 @@ follow_fork () if (tp =3D=3D cur_thr) continue; =20 - if (tp->pending_follow.kind () !=3D TARGET_WAITKIND_SPURIOUS) + /* follow_fork_inferior clears tp->pending_follow, and below + we'll need the value after the follow_fork_inferior + call. */ + target_waitkind kind =3D tp->pending_follow.kind (); + + if (kind !=3D TARGET_WAITKIND_SPURIOUS) { infrun_debug_printf ("need to follow-fork [%s] first", tp->ptid.to_string ().c_str ()); =20 switch_to_thread (tp); - should_resume =3D false; + + /* Set up inferior(s) as specified by the caller, and + tell the target to do whatever is necessary to follow + either parent or child. */ + if (follow_child) + { + /* The thread that started the execution command + won't exist in the child. Abort the command and + immediately stop in this thread, in the child, + inside fork. */ + should_resume =3D false; + } + else + { + /* Following the parent, so let the thread fork its + child freely, it won't influence the current + execution command. */ + if (follow_fork_inferior (follow_child, detach_fork)) + { + /* Target refused to follow, or there's some + other reason we shouldn't resume. */ + switch_to_thread (cur_thr); + set_last_target_status_stopped (cur_thr); + return false; + } + + /* If we're following a vfork, when we need to leave + the just-forked thread as selected, as we need to + solo-resume it to collect the VFORK_DONE event. + If we're following a fork, however, switch back + to the original thread that we continue stepping + it, etc. */ + if (kind !=3D TARGET_WAITKIND_VFORKED) + { + gdb_assert (kind =3D=3D TARGET_WAITKIND_FORKED); + switch_to_thread (cur_thr); + } + } + break; } } @@ -2201,6 +2244,29 @@ user_visible_resume_target (ptid_t resume_ptid) : current_inferior ()->process_target ()); } =20 +/* Find a thread from the inferiors that we'll resume that is waiting + for a vfork-done event. */ + +static thread_info * +find_thread_waiting_for_vfork_done () +{ + gdb_assert (!target_is_non_stop_p ()); + + if (sched_multi) + { + for (inferior *inf : all_non_exited_inferiors ()) + if (inf->thread_waiting_for_vfork_done !=3D nullptr) + return inf->thread_waiting_for_vfork_done; + } + else + { + inferior *cur_inf =3D current_inferior (); + if (cur_inf->thread_waiting_for_vfork_done !=3D nullptr) + return cur_inf->thread_waiting_for_vfork_done; + } + return nullptr; +} + /* Return a ptid representing the set of threads that we will resume, in the perspective of the target, assuming run control handling does not require leaving some threads stopped (e.g., stepping past @@ -2241,14 +2307,18 @@ internal_resume_ptid (int user_step) Since we don't have that flexibility (we can only pass one ptid), just resume the first thread waiting for a vfork-done event we find (e.g. = thread 2.1). */ - if (sched_multi) - { - for (inferior *inf : all_non_exited_inferiors ()) - if (inf->thread_waiting_for_vfork_done !=3D nullptr) - return inf->thread_waiting_for_vfork_done->ptid; + thread_info *thr =3D find_thread_waiting_for_vfork_done (); + if (thr !=3D nullptr) + { + /* If we have a thread that is waiting for a vfork-done event, + then we should have switched to it earlier. Calling + target_resume with thread scope is only possible when the + current thread matches the thread scope. */ + gdb_assert (thr->ptid =3D=3D inferior_ptid); + gdb_assert (thr->inf->process_target () + =3D=3D inferior_thread ()->inf->process_target ()); + return thr->ptid; } - else if (current_inferior ()->thread_waiting_for_vfork_done !=3D nullptr) - return current_inferior ()->thread_waiting_for_vfork_done->ptid; =20 return user_visible_resume_ptid (user_step); } diff --git a/gdb/testsuite/gdb.threads/foll-fork-other-thread.c b/gdb/tests= uite/gdb.threads/foll-fork-other-thread.c new file mode 100644 index 00000000000..47e8b1c7389 --- /dev/null +++ b/gdb/testsuite/gdb.threads/foll-fork-other-thread.c @@ -0,0 +1,84 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2022 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . = */ + +#include +#include +#include +#include +#include +#include +#include + +/* Set by GDB. */ +volatile int stop_looping =3D 0; + +static void * +gdb_forker_thread (void *arg) +{ + int ret; + int stat; + pid_t pid =3D FORK_FUNC (); + + if (pid =3D=3D 0) + _exit (0); + + assert (pid > 0); + + /* Wait for child to exit. */ + do + { + ret =3D waitpid (pid, &stat, 0); + } + while (ret =3D=3D -1 && errno =3D=3D EINTR); + + assert (ret =3D=3D pid); + assert (WIFEXITED (stat)); + assert (WEXITSTATUS (stat) =3D=3D 0); + + stop_looping =3D 1; + + return NULL; +} + +static void +sleep_a_bit (void) +{ + usleep (1000 * 50); +} + +int +main (void) +{ + int i; + int ret; + pthread_t thread; + + alarm (60); + + ret =3D pthread_create (&thread, NULL, gdb_forker_thread, NULL); + assert (ret =3D=3D 0); + + while (!stop_looping) /* while loop */ + { + sleep_a_bit (); /* break here */ + sleep_a_bit (); /* other line */ + } + + pthread_join (thread, NULL); + + return 0; /* exiting here */ +} diff --git a/gdb/testsuite/gdb.threads/foll-fork-other-thread.exp b/gdb/tes= tsuite/gdb.threads/foll-fork-other-thread.exp new file mode 100644 index 00000000000..ed62bfc54d9 --- /dev/null +++ b/gdb/testsuite/gdb.threads/foll-fork-other-thread.exp @@ -0,0 +1,172 @@ +# Copyright 2022 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test catching a vfork/fork in one thread, and then doing a "next" in +# another thread, in different combinations of "set follow-fork +# parent/child", and other execution modes. + +standard_testfile + +# Line where to stop the main thread. +set break_here_line [gdb_get_line_number "break here"] + +# Build executables, one for each fork flavor. +foreach_with_prefix fork_func {fork vfork} { + set opts [list debug pthreads additional_flags=3D-DFORK_FUNC=3D${fork_= func}] + if { [build_executable "failed to prepare" \ + ${testfile}-${fork_func} ${srcfile} $opts] } { + return + } +} + +# Run the test with the given parameters: +# +# - FORK_FUNC: fork flavor, "fork" or "vfork". +# - FOLLOW: "set follow-fork" value, either "parent" or "child". +# - TARGET-NON-STOP: "maintenance set target-non-stop" value, "auto", "o= n" or +# "off". +# - NON-STOP: "set non-stop" value, "on" or "off". +# - DISPLACED-STEPPING: "set displaced-stepping" value, "auto", "on" or = "off". + +proc do_test { fork_func follow target-non-stop non-stop displaced-steppin= g } { + save_vars { ::GDBFLAGS } { + append ::GDBFLAGS " -ex \"maintenance set target-non-stop ${target-non-st= op}\"" + append ::GDBFLAGS " -ex \"set non-stop ${non-stop}\"" + clean_restart ${::binfile}-${fork_func} + } + + gdb_test_no_output "set displaced-stepping ${displaced-stepping}" + + if { ![runto_main] } { + return + } + + delete_breakpoints + + gdb_test "catch $fork_func" "Catchpoint .*" + + # Verify that the catchpoint is mentioned in an "info breakpoints", + # and further that the catchpoint mentions no process id. + gdb_test "info breakpoints" \ + ".*catchpoint.*keep y.*fork\[\r\n\]+" \ + "info breakpoints before fork" + + gdb_test "continue" \ + "Catchpoint \[0-9\]* \\(.?forked process \[0-9\]*\\),.*" \ + "explicit child follow, catch fork" + + # Verify that the catchpoint is mentioned in an "info breakpoints", + # and further that the catchpoint managed to capture a process id. + gdb_test "info breakpoints" \ + ".*catchpoint.*keep y.*fork, process.*" \ + "info breakpoints after fork" + + gdb_test "thread 1" "Switching to .*" + + gdb_test_no_output "set scheduler-locking on" + + # Advance the next-ing thread to the point where we'll execute the + # next. + gdb_test "break $::srcfile:$::break_here_line" "Breakpoint $::decimal = at $::hex.*" + gdb_test "continue" "hit Breakpoint $::decimal, main.*" + + # Disable schedlock and step. The pending fork should no longer + # be pending afterwards. + + gdb_test "set scheduler-locking off" + + # Make sure GDB doesn't try to step over the breakpoint at PC + # first, we want to make sure that GDB doesn't lose focus of the + # step/next in this thread. A breakpoint would make GDB switch + # focus anyhow, thus hide a potential bug. + delete_breakpoints + + gdb_test_no_output "set follow-fork $follow" + + set any "\[^\r\n\]*" + + if {$follow =3D=3D "child"} { + + # For fork, GDB detaches from the parent at follow-fork time. + # For vfork, GDB detaches from the parent at child exit/exec + # time. + if {$fork_func =3D=3D "fork"} { + set detach_parent \ + [multi_line \ + "\\\[Detaching after $fork_func from parent process $any\\\]" \ + "\\\[Inferior 1 $any detached\\\]"] + } else { + set detach_parent "" + } + + gdb_test "next" \ + [multi_line \ + "\\\[Attaching after $any $fork_func to child $any\\\]" \ + "\\\[New inferior 2 $any\\\]" \ + "$detach_parent.*warning: Not resuming: switched threads before followi= ng fork child\\." \ + "\\\[Switching to $any\\\]" \ + ".*"] \ + "next aborts resumption" + + # The child should be stopped inside the fork implementation + # in the runtime. Exactly at which instruction/function is + # system dependent, but we can check that our + # "gdb_forker_thread" function appears in the backtrace. + gdb_test "bt" " in gdb_forker_thread ${any} at ${any}${::srcfile}:.*" + + # The child is now thread 1. + gdb_test "print \$_thread" " =3D 1" + + if {$fork_func =3D=3D "fork"} { + gdb_test "continue" \ + [multi_line \ + "Continuing." \ + "\\\[Inferior 2 \\\(process $any\\\) exited normally\\\]"] \ + "continue to exit" + } else { + gdb_test "continue" \ + [multi_line \ + "Continuing." \ + "\\\[Detaching vfork parent process $any after child exit\\\]" \ + "\\\[Inferior 1 \\\(process $any\\\) detached\\\]" \ + "\\\[Inferior 2 \\\(process $any\\\) exited normally\\\]"] \ + "continue to exit" + } + } else { + gdb_test "next" \ + "\\\[Detaching after $fork_func from child process ${any}\\\].* other= line .*" \ + "next to other line" + + gdb_test "print \$_thread" " =3D 1" + + gdb_test "continue" \ + [multi_line \ + "Continuing." \ + "\\\[Inferior 1 \\\(process $any\\\) exited normally\\\]"] \ + "continue to exit" + } +} + +foreach_with_prefix fork_func {fork vfork} { + foreach_with_prefix follow {child} { + foreach_with_prefix target-non-stop {auto on off} { + foreach_with_prefix non-stop {off} { + foreach_with_prefix displaced-stepping {auto on off} { + do_test ${fork_func} ${follow} ${target-non-stop} ${non-stop} ${disp= laced-stepping} + } + } + } + } +}