From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id C566B3857C6A for ; Wed, 24 Nov 2021 20:04:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C566B3857C6A X-ASG-Debug-ID: 1637784285-0c856e2e472af080001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id 0dke8skaDZ65WrEc (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 24 Nov 2021 15:04:46 -0500 (EST) X-Barracuda-Envelope-From: simon.marchi@efficios.com X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from smarchi-efficios.internal.efficios.com (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) by smtp.ebox.ca (Postfix) with ESMTP id DB4FA441D66; Wed, 24 Nov 2021 15:04:45 -0500 (EST) From: Simon Marchi X-Barracuda-RBL-IP: 192.222.180.24 X-Barracuda-Effective-Source-IP: 192-222-180-24.qc.cable.ebox.net[192.222.180.24] X-Barracuda-Apparent-Source-IP: 192.222.180.24 To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH 3/3] gdb, gdbserver: detach fork child when detaching from fork parent Date: Wed, 24 Nov 2021 15:04:44 -0500 X-ASG-Orig-Subj: [PATCH 3/3] gdb, gdbserver: detach fork child when detaching from fork parent Message-Id: <20211124200444.614978-4-simon.marchi@efficios.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20211124200444.614978-1-simon.marchi@efficios.com> References: <20211029203332.69894-1-simon.marchi@polymtl.ca> <20211124200444.614978-1-simon.marchi@efficios.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1637784286 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 14348 X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.50 X-Barracuda-Spam-Status: No, SCORE=0.50 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=8.0 tests=BSF_RULE7568M X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.94175 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.50 BSF_RULE7568M Custom Rule 7568M X-Spam-Status: No, score=-19.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Nov 2021 20:04:55 -0000 While working with pending fork events, I wondered what would happen if the user detached an inferior while a thread of that inferior had a pending fork event. What happens with the fork child, which is ptrace-attached by the GDB process (or by GDBserver), but not known to the core? Sure enough, neither the core of GDB or the target detach the child process, so GDB (or GDBserver) just stays ptrace-attached to the process. The result is that the fork child process is stuck, while you would expect it to be detached and run. Make GDBserver detach of fork children it knows about. That is done in the generic handle_detach function. Since a process_info already exists for the child, we can simply call detach_inferior on it. GDB-side, make the linux-nat and remote targets detach of fork children known because of pending fork events. These pending fork events can be stored in thread_info::pending_waitstatus, if the core has consumed the event but then saved it for later. They can also be stored, for the linux-nat target, in lwp_info::status and lwp_info::waitstatus. For the remote target, I suppose that a pending fork event could maybe be in the stop reply queue, but I could generate a case where would could detach while having pending events there, so I didn't handle that case in this patch. Update the gdb.threads/pending-fork-event.exp to try to detach the process while a thread has a pending fork event. In order to verify that the fork child process is correctly detached and resumes execution outside of GDB's control, make that process create a file in the test output directory, and make the test wait $timeout seconds for that file to appear (it happens instantly if everything goes well). The test catches a bug in linux-nat.c, also reported as PR 28512 ("waitstatus.h:300: internal-error: gdb_signal target_waitstatus::sig() const: Assertion `m_kind == TARGET_WAITKIND_STOPPED || m_kind == TARGET_WAITKIND_SIGNALLED' failed.). When detaching a thread with a pending event, get_detach_signal unconditionally fetches the signal stored in the waitstatus (`tp->pending_waitstatus ().sig ()`). However, that is only valid if the pending event is of type TARGET_WAITKIND_STOPPED, and this is now enforced using assertions (iit would also be valid for TARGET_WAITKIND_SIGNALLED, but that would mean the thread does not exist anymore, so we wouldn't be detaching it). Add a condition in get_detach_signal to access the signal number only if the wait status is of kind TARGET_WAITKIND_STOPPED, and use GDB_SIGNAL_0 instead (since the thread was not stopped with a signal to begin with). There is a known limitation: we don't remove breakpoints from the children before detaching it. So the children, could hit a trap instruction after being detached and crash. I know this is wrong, and it should be fixed, but I would like to handle that later. The current patch doesn't fix everything, but it's a step in the right direction. Change-Id: I6d811a56f520e3cb92d5ea563ad38976f92e93dd Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28512 --- gdb/linux-nat.c | 48 +++++++++++- gdb/remote.c | 14 ++++ .../pending-fork-event-touch-file.c | 26 +++++++ .../gdb.threads/pending-fork-event.c | 6 +- .../gdb.threads/pending-fork-event.exp | 76 +++++++++++++++++-- gdbserver/server.cc | 29 +++++++ 6 files changed, 189 insertions(+), 10 deletions(-) create mode 100644 gdb/testsuite/gdb.threads/pending-fork-event-touch-file.c diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 85ae68d8a6fa..b9e09e4fc0ce 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -1315,7 +1315,16 @@ get_detach_signal (struct lwp_info *lp) if (target_is_non_stop_p () && !tp->executing ()) { if (tp->has_pending_waitstatus ()) - signo = tp->pending_waitstatus ().sig (); + { + /* If the thread has a pending event, and it was stopped with a + signal, use that signal to resume it. If it has a pending + event of another kind, it was not stopped with a signal, so + resume it without a signal. */ + if (tp->pending_waitstatus ().kind () == TARGET_WAITKIND_STOPPED) + signo = tp->pending_waitstatus ().sig (); + else + signo = GDB_SIGNAL_0; + } else signo = tp->stop_signal (); } @@ -1367,6 +1376,43 @@ detach_one_lwp (struct lwp_info *lp, int *signo_p) gdb_assert (lp->status == 0 || WIFSTOPPED (lp->status)); + /* If the lwp/thread we are about to detach has a pending fork event, + there is a process GDB is attached to that the core of GDB doesn't know + about. Detach from it. */ + + /* Check in lwp_info::status. */ + if (WIFSTOPPED (lp->status) && linux_is_extended_waitstatus (lp->status)) + { + int event = linux_ptrace_get_extended_event (lp->status); + + if (event == PTRACE_EVENT_FORK || event == PTRACE_EVENT_VFORK) + { + unsigned long child_pid; + int ret = ptrace (PTRACE_GETEVENTMSG, lp->ptid.lwp (), 0, &child_pid); + if (ret == 0) + detach_one_pid (child_pid, 0); + else + perror_warning_with_name (_("Failed to detach fork child")); + } + } + + /* Check in lwp_info::waitstatus. */ + if (lp->waitstatus.kind () == TARGET_WAITKIND_VFORKED + || lp->waitstatus.kind () == TARGET_WAITKIND_FORKED) + detach_one_pid (lp->waitstatus.child_ptid ().pid (), 0); + + + /* Check in thread_info::pending_waitstatus. */ + thread_info *tp = find_thread_ptid (linux_target, lp->ptid); + if (tp->has_pending_waitstatus ()) + { + const target_waitstatus &ws = tp->pending_waitstatus (); + + if (ws.kind () == TARGET_WAITKIND_VFORKED + || ws.kind () == TARGET_WAITKIND_FORKED) + detach_one_pid (ws.child_ptid ().pid (), 0); + } + if (lp->status != 0) linux_nat_debug_printf ("Pending %s for %s on detach.", strsignal (WSTOPSIG (lp->status)), diff --git a/gdb/remote.c b/gdb/remote.c index 724386e09164..677d54ccd5c9 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -5876,6 +5876,20 @@ remote_target::remote_detach_1 (inferior *inf, int from_tty) if (from_tty && !rs->extended && number_of_live_inferiors (this) == 1) puts_filtered (_("Ending remote debugging.\n")); + /* See if any thread of the inferior we are detaching has a pending fork + status. In that case, we must detach from the child resulting from + that fork. */ + for (thread_info *thread : inf->non_exited_threads ()) + { + if (!thread->has_pending_waitstatus ()) + continue; + + const target_waitstatus &ws = thread->pending_waitstatus (); + if (ws.kind () == TARGET_WAITKIND_FORKED + || ws.kind () == TARGET_WAITKIND_VFORKED) + remote_detach_pid (ws.child_ptid ().pid ()); + } + thread_info *tp = find_thread_ptid (this, inferior_ptid); /* Check to see if we are detaching a fork parent. Note that if we diff --git a/gdb/testsuite/gdb.threads/pending-fork-event-touch-file.c b/gdb/testsuite/gdb.threads/pending-fork-event-touch-file.c new file mode 100644 index 000000000000..5536381847b1 --- /dev/null +++ b/gdb/testsuite/gdb.threads/pending-fork-event-touch-file.c @@ -0,0 +1,26 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2021 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 + +int +main (void) +{ + FILE *f = fopen (TOUCH_FILE_PATH, "w"); + fclose (f); + return 0; +} diff --git a/gdb/testsuite/gdb.threads/pending-fork-event.c b/gdb/testsuite/gdb.threads/pending-fork-event.c index a39ca75a49ac..bab4a99e5786 100644 --- a/gdb/testsuite/gdb.threads/pending-fork-event.c +++ b/gdb/testsuite/gdb.threads/pending-fork-event.c @@ -32,18 +32,18 @@ break_here (void) static void do_fork (void) { - pthread_barrier_wait (&barrier); - while (!release_forking_thread); if (FORK_FUNCTION () == 0) - _exit (0); + execl (TOUCH_FILE_BIN, TOUCH_FILE_BIN, NULL); } static void * thread_func (void *p) { + pthread_barrier_wait (&barrier); + #if defined(MAIN_THREAD_FORKS) break_here (); #elif defined(OTHER_THREAD_FORKS) diff --git a/gdb/testsuite/gdb.threads/pending-fork-event.exp b/gdb/testsuite/gdb.threads/pending-fork-event.exp index 6574b9abf5b7..cbe55c2f5e7c 100644 --- a/gdb/testsuite/gdb.threads/pending-fork-event.exp +++ b/gdb/testsuite/gdb.threads/pending-fork-event.exp @@ -28,11 +28,32 @@ # for it. Once fixed, the child process' thread is hidden by whoever holds the # pending fork event. -standard_testfile +standard_testfile .c -touch-file.c -proc do_test { target-non-stop who_forks fork_function } { +set touch_file_bin $binfile-touch-file +set touch_file_path [standard_output_file some-file] - set opts [list debug "additional_flags=-DFORK_FUNCTION=$fork_function"] +set opts [list debug "additional_flags=-DTOUCH_FILE_PATH=\"$touch_file_path\""] +if { [gdb_compile "$srcdir/$subdir/$srcfile2" $touch_file_bin executable $opts] != "" } { + return 0 +} + +# Run until execution is stopped and a thread has a pending fork event. +# +# WHO_FORKS tells which of the thread program threads does the fork ("main" or +# "other"). +# +# FORK_FUNCTION tells which function to call to fork ("fork" or "vfork"). +# +# Return 1 on success, 0 otherwise. + +proc setup { target-non-stop who_forks fork_function } { + set this_binfile ${::binfile}-${fork_function} + + set opts [list \ + debug \ + "additional_flags=-DFORK_FUNCTION=$fork_function" \ + "additional_flags=-DTOUCH_FILE_BIN=\"$::touch_file_bin\""] # WHO_FORKS says which of the main or other thread calls (v)fork. The # thread that does not call (v)fork is the one who tries to step. @@ -47,7 +68,7 @@ proc do_test { target-non-stop who_forks fork_function } { } if { [gdb_compile_pthreads "$::srcdir/$::subdir/$::srcfile" $this_binfile executable $opts] != "" } { - return + return 0 } save_vars { ::GDBFLAGS } { @@ -57,7 +78,7 @@ proc do_test { target-non-stop who_forks fork_function } { if {![runto_main]} { fail "could not run to main" - return + return 0 } # Run until breakpoint in the second thread. @@ -75,12 +96,55 @@ proc do_test { target-non-stop who_forks fork_function } { # Make sure there's still a single inferior. gdb_test "info inferior" {\* 1 [^\r\n]+} + + return 1 +} + +# Return 1 if file ::TOUCH_FILE_PATH exists, else 0. + +proc file_exists {} { + return [file exists $::touch_file_path] +} + +# Wait up to ::TIMEOUT seconds for file ::TOUCH_FILE_PATH to exist. Return +# 1 if it does exist, 0 otherwise. + +proc file_exists_with_timeout {} { + for {set i 0} {$i < $::timeout} {incr i} { + if { [file_exists] } { + return 1 + } + + sleep 1 + } + + return 0 +} + +# Run until there exists (at least, likely exists) a pending fork event. +# Detach the fork parent. Verify that the fork child (which does not appear in +# GDB, since the fork event is pending) is detached as well. + +proc_with_prefix do_test_detach { target-non-stop who_forks fork_function } { + file delete $::touch_file_path + gdb_assert { ![file_exists] } "file does not exist before test" + + if { ![setup ${target-non-stop} $who_forks $fork_function] } { + return + } + + gdb_test "detach" + + # After being detached, the fork child creates file ::TOUCH_FILE_PATH. + # Seeing this file tells us the fork child was detached and executed + # successfully. + gdb_assert { [file_exists_with_timeout] } "file exists after detach" } foreach_with_prefix target-non-stop { auto on off } { foreach_with_prefix who_forks { main other } { foreach_with_prefix fork_function { fork vfork } { - do_test ${target-non-stop} $who_forks $fork_function + do_test_detach ${target-non-stop} $who_forks $fork_function } } } diff --git a/gdbserver/server.cc b/gdbserver/server.cc index 7963f2faf267..2f3d5341758f 100644 --- a/gdbserver/server.cc +++ b/gdbserver/server.cc @@ -1250,6 +1250,35 @@ handle_detach (char *own_buf) /* We'll need this after PROCESS has been destroyed. */ int pid = process->pid; + /* If this process has an unreported fork child, that child is not known to + GDB, so detach it as well. + + Here, we specifically don't want to use "safe iteration", as detaching + another process might delete the next thread in the iteration, which is + the one saved by the safe iterator. We will never delete the currently + iterated on thread, so standard iteration should be safe. */ + for (thread_info *thread : all_threads) + { + /* Only threads that are of the process we are detaching. */ + if (thread->id.pid () != pid) + continue; + + /* Only threads that have a pending fork event. */ + if (thread->fork_child == nullptr) + continue; + + process_info *fork_child_process + = find_process_pid (thread->fork_child->id.pid ()); + gdb_assert (fork_child_process != nullptr); + + int fork_child_pid = fork_child_process->pid; + + if (detach_inferior (fork_child_process) != 0) + warning (_("Failed to detach fork child %s, child of %s"), + target_pid_to_str (ptid_t (fork_child_pid)).c_str (), + target_pid_to_str (thread->id).c_str ()); + } + if (detach_inferior (process) != 0) write_enn (own_buf); else -- 2.26.2