* [PATCH v3 0/3] Follow fork improvements
@ 2021-06-10 14:33 Simon Marchi
2021-06-10 14:33 ` [PATCH v3 1/3] gdb: call post_create_inferior at end of follow_fork_inferior Simon Marchi
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Simon Marchi @ 2021-06-10 14:33 UTC (permalink / raw)
To: gdb-patches
This is just a slightly non-trivial rebase of:
https://sourceware.org/pipermail/gdb-patches/2021-June/179454.html
There were conflicts due to:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=122373f7f25946cfc51de9e19ba1d173195f9910
I think the combined result is nice, since we now set
inferior::in_initial_library_scan in a single place, in
post_create_inferior.
Simon Marchi (3):
gdb: call post_create_inferior at end of follow_fork_inferior
gdb: pass child_ptid and fork kind to target_ops::follow_fork
gdb: follow-fork: push target and add thread in target_follow_fork
gdb/NEWS | 3 +
gdb/doc/gdb.texinfo | 4 +
gdb/exec.c | 6 +-
gdb/exec.h | 9 +-
gdb/fbsd-nat.c | 12 +-
gdb/fbsd-nat.h | 2 +-
gdb/infrun.c | 199 ++++++++------------
gdb/jit.c | 28 +++
gdb/linux-nat.c | 45 ++---
gdb/linux-nat.h | 5 +-
gdb/linux-thread-db.c | 2 +-
gdb/obsd-nat.c | 15 +-
gdb/obsd-nat.h | 2 +-
gdb/process-stratum-target.c | 15 ++
gdb/process-stratum-target.h | 9 +
gdb/remote.c | 22 +--
gdb/target-debug.h | 2 +
gdb/target-delegates.c | 26 ++-
gdb/target.c | 22 ++-
gdb/target.h | 16 +-
gdb/testsuite/gdb.base/jit-elf-fork-main.c | 129 +++++++++++++
gdb/testsuite/gdb.base/jit-elf-fork-solib.c | 25 +++
gdb/testsuite/gdb.base/jit-elf-fork.exp | 186 ++++++++++++++++++
23 files changed, 576 insertions(+), 208 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/jit-elf-fork-main.c
create mode 100644 gdb/testsuite/gdb.base/jit-elf-fork-solib.c
create mode 100644 gdb/testsuite/gdb.base/jit-elf-fork.exp
--
2.32.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 1/3] gdb: call post_create_inferior at end of follow_fork_inferior
2021-06-10 14:33 [PATCH v3 0/3] Follow fork improvements Simon Marchi
@ 2021-06-10 14:33 ` Simon Marchi
2021-07-02 17:54 ` Pedro Alves
2021-06-10 14:33 ` [PATCH v3 2/3] gdb: pass child_ptid and fork kind to target_ops::follow_fork Simon Marchi
2021-06-10 14:33 ` [PATCH v3 3/3] gdb: follow-fork: push target and add thread in target_follow_fork Simon Marchi
2 siblings, 1 reply; 12+ messages in thread
From: Simon Marchi @ 2021-06-10 14:33 UTC (permalink / raw)
To: gdb-patches
GDB doesn't handle well the case of an inferior using the JIT interface
to register JIT-ed objiles and forking. If an inferior registers a code
object using the JIT interface and then forks, the child process
conceptually has the same code object loaded, so GDB should look it up
and learn about it (it currently doesn't).
To achieve this, I think it would make sense to have the
inferior_created observable called when an inferior is created due to a
fork in follow_fork_inferior. The inferior_created observable is
currently called both after starting a new inferior and after attaching
to an inferior, allowing various sub-components to learn about that new
executing inferior. We can see handling a fork child just like
attaching to it, so any work done when attaching should also be done in
the case of a fork child.
Instead of just calling the inferior_created observable, this patch
makes follow_fork_inferior call the whole post_create_inferior function.
This way, the attach and follow-fork code code paths are more alike.
Given that post_create_inferior calls solib_create_inferior_hook,
follow_fork_inferior doesn't need to do it itself, so those calls to
solib_create_inferior_hook are removed.
One question you may have: why not just call post_create_inferior at the
places where solib_create_inferior_hook is currently called, instead of
after target_follow_fork?
- there's something fishy for the second solib_create_inferior_hook
call site: at this point we have switched the current program space
to the child's, but not the current inferior nor the current thread.
So solib_create_inferior_hook (and everything under, including
check_for_thread_db, for example) is called with inferior 1 as the
current inferior and inferior 2's program space as the current
program space. I think that's wrong, because at this point we are
setting up inferior 2, and all that code relies on the current
inferior. We could just add a switch_to_thread call before it to
make inferior 2 the current one, but there are other problems (see
below).
- solib_create_inferior_hook is currently not called on the
`follow_child && detach_fork` path. I think we need to call it,
because we still get a new inferior in that case (even though we
detach the parent). If we only call post_create_inferior where
solib_create_inferior_hook used to be called, then the JIT
subcomponent doesn't get informed about the new inferior, and that
introduces a failure in the new gdb.base/jit-elf-fork.exp test.
- if we try to put the post_create_inferior just after the
switch_to_thread that was originally at line 662, or just before the
call to target_follow_fork, we introduce a subtle failure in
gdb.threads/fork-thread-pending.exp. What happens then is that
libthread_db gets loaded (somewhere under post_create_inferior)
before the linux-nat target learns about the LWPs (which happens in
linux_nat_target::follow_fork). As a result, the ALL_LWPS loop in
try_thread_db_load_1 doesn't see the child LWP, and the thread-db
target doesn't have the chance to fill in thread_info::priv. A bit
later, when the test does "info threads", and
thread_db_target::pid_to_str is called, the thread-db target doesn't
recognize the thread as one of its own, and delegates the request to
the target below. Because the pid_to_str output is not the expected
one, the test fails.
This tells me that we need to call the process target's follow_fork
first, to make the process target create the necessary LWP and thread
structures. Then, we can call post_create_inferior to let the other
components of GDB do their thing.
But then you may ask: check_for_thread_db is already called today,
somewhere under solib_create_inferior_hook, and that is before
target_follow_fork, why don't we see this ordering problem!? Well,
because of the first bullet point: when check_for_thread_db /
thread_db_load are called, the current inferior is (erroneously)
inferior 1, the parent. Because libthread_db is already loaded for
the parent, thread_db_load early returns. check_for_thread_db later
gets called by linux_nat_target::follow_fork. At this point, the
current inferior is the correct one and the child's LWP exists, so
all is well.
Since we now call post_create_inferior after target_follow_fork, which
calls the inferior_created observable, which calls check_for_thread_db,
I don't think linux_nat_target needs to explicitly call
check_for_thread_db itself, so that is removed.
In terms of testing, this patch adds a new gdb.base/jit-elf-fork.exp
test. It makes an inferior register a JIT code object and then fork.
It then verifies that whatever the detach-on-fork and follow-fork-child
parameters are, GDB knows about the JIT code object in all the inferiors
that survive the fork. It verifies that the inferiors can unload that
code object.
There isn't currently a way to get visibility into GDB's idea of the JIT
code objects for each inferior. For the purpose of this test, add the
"maintenance info jit" command. There isn't much we can print about the
JIT code objects except their load address. So the output looks a bit
bare, but it's good enough for the test.
gdb/ChangeLog:
* NEWS: Mention "maint info jit" command.
* infrun.c (follow_fork_inferior): Don't call
solib_create_inferior_hook, call post_create_inferior if a new
inferior was created.
* jit.c (maint_info_jit_cmd): New.
(_initialize_jit): Register new command.
* linux-nat.c (linux_nat_target::follow_fork): Don't call
check_for_thread_db.
* linux-nat.h (check_for_thread_db): Remove declaration.
* linux-thread-db.c (check_thread_signals): Make static.
gdb/doc/ChangeLog:
* gdb.texinfo (Maintenance Commands): Mention "maint info jit".
gdb/testsuite/ChangeLog:
* gdb.base/jit-elf-fork-main.c: New test.
* gdb.base/jit-elf-fork-solib.c: New test.
* gdb.base/jit-elf-fork.exp: New test.
Change-Id: I9a192e55b8a451c00e88100669283fc9ca60de5c
---
gdb/NEWS | 3 +
gdb/doc/gdb.texinfo | 4 +
gdb/infrun.c | 38 ++--
gdb/jit.c | 28 +++
gdb/linux-nat.c | 15 --
gdb/linux-nat.h | 3 -
gdb/linux-thread-db.c | 2 +-
gdb/testsuite/gdb.base/jit-elf-fork-main.c | 129 ++++++++++++++
gdb/testsuite/gdb.base/jit-elf-fork-solib.c | 25 +++
gdb/testsuite/gdb.base/jit-elf-fork.exp | 186 ++++++++++++++++++++
10 files changed, 391 insertions(+), 42 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/jit-elf-fork-main.c
create mode 100644 gdb/testsuite/gdb.base/jit-elf-fork-solib.c
create mode 100644 gdb/testsuite/gdb.base/jit-elf-fork.exp
diff --git a/gdb/NEWS b/gdb/NEWS
index 56743fc9aeaa..47fb4424a699 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -101,6 +101,9 @@ maintenance flush dcache
maintenance info target-sections
Print GDB's internal target sections table.
+maintenance info jit
+ Print the JIT code objects in the inferior known to GDB.
+
memory-tag show-logical-tag POINTER
Print the logical tag for POINTER.
memory-tag with-logical-tag POINTER TAG
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d09b86cda959..6cfdd3153394 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -38931,6 +38931,10 @@ buffer.
Control whether @value{GDBN} will skip PAD packets when computing the
packet history.
+@kindex maint info jit
+@item maint info jit
+Print information about JIT code objects loaded in the current inferior.
+
@kindex set displaced-stepping
@kindex show displaced-stepping
@cindex displaced stepping support
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4bd21fde5907..791ccee6ea0b 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -429,6 +429,8 @@ holding the child stopped. Try \"set detach-on-fork\" or \
return true;
}
+ thread_info *child_thr = nullptr;
+
if (!follow_child)
{
/* Detach new forked process? */
@@ -478,8 +480,8 @@ holding the child stopped. Try \"set detach-on-fork\" or \
switch_to_no_thread ();
child_inf->symfile_flags = SYMFILE_NO_READ;
child_inf->push_target (parent_inf->process_target ());
- thread_info *child_thr
- = add_thread_silent (child_inf->process_target (), child_ptid);
+ child_thr = add_thread_silent (child_inf->process_target (),
+ child_ptid);
/* If this is a vfork child, then the address-space is
shared with the parent. */
@@ -514,17 +516,6 @@ holding the child stopped. Try \"set detach-on-fork\" or \
/* solib_create_inferior_hook relies on the current
thread. */
switch_to_thread (child_thr);
-
- /* Let the shared library layer (e.g., solib-svr4) learn
- about this new process, relocate the cloned exec, pull
- in shared libraries, and install the solib event
- breakpoint. If a "cloned-VM" event was propagated
- better throughout the core, this wouldn't be
- required. */
- scoped_restore restore_in_initial_library_scan
- = make_scoped_restore (&child_inf->in_initial_library_scan,
- true);
- solib_create_inferior_hook (0);
}
}
@@ -633,7 +624,7 @@ holding the child stopped. Try \"set detach-on-fork\" or \
child_inf->push_target (target);
}
- thread_info *child_thr = add_thread_silent (target, child_ptid);
+ child_thr = add_thread_silent (target, child_ptid);
/* If this is a vfork child, then the address-space is shared
with the parent. If we detached from the parent, then we can
@@ -653,15 +644,6 @@ holding the child stopped. Try \"set detach-on-fork\" or \
child_inf->symfile_flags = SYMFILE_NO_READ;
set_current_program_space (child_inf->pspace);
clone_program_space (child_inf->pspace, parent_pspace);
-
- /* Let the shared library layer (e.g., solib-svr4) learn
- about this new process, relocate the cloned exec, pull in
- shared libraries, and install the solib event breakpoint.
- If a "cloned-VM" event was propagated better throughout
- the core, this wouldn't be required. */
- scoped_restore restore_in_initial_library_scan
- = make_scoped_restore (&child_inf->in_initial_library_scan, true);
- solib_create_inferior_hook (0);
}
switch_to_thread (child_thr);
@@ -669,6 +651,16 @@ holding the child stopped. Try \"set detach-on-fork\" or \
target_follow_fork (follow_child, detach_fork);
+ /* If we ended up creating a new inferior, call post_create_inferior to inform
+ the various subcomponents. */
+ if (child_thr != nullptr)
+ {
+ scoped_restore_current_thread restore;
+ switch_to_thread (child_thr);
+
+ post_create_inferior (0);
+ }
+
return false;
}
diff --git a/gdb/jit.c b/gdb/jit.c
index 1de785b8de08..1838fd966bf8 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -74,6 +74,30 @@ show_jit_debug (struct ui_file *file, int from_tty,
fprintf_filtered (file, _("JIT debugging is %s.\n"), value);
}
+/* Implementation of the "maintenance info jit" command. */
+
+static void
+maint_info_jit_cmd (const char *args, int from_tty)
+{
+ inferior *inf = current_inferior ();
+ bool printed_header = false;
+
+ /* Print a line for each JIT-ed objfile. */
+ for (objfile *obj : inf->pspace->objfiles ())
+ {
+ if (obj->jited_data == nullptr)
+ continue;
+
+ if (!printed_header)
+ {
+ printf_filtered ("Base address of known JIT-ed objfiles:\n");
+ printed_header = true;
+ }
+
+ printf_filtered (" - %s\n", paddress (obj->arch (), obj->jited_data->addr));
+ }
+}
+
struct jit_reader
{
jit_reader (struct gdb_reader_funcs *f, gdb_dlhandle_up &&h)
@@ -1247,6 +1271,10 @@ _initialize_jit ()
show_jit_debug,
&setdebuglist, &showdebuglist);
+ add_cmd ("jit", class_maintenance, maint_info_jit_cmd,
+ _("Print information about JIT-ed code objects."),
+ &maintenanceinfolist);
+
gdb::observers::inferior_created.attach (jit_inferior_created_hook, "jit");
gdb::observers::inferior_execd.attach (jit_inferior_created_hook, "jit");
gdb::observers::inferior_exit.attach (jit_inferior_exit_hook, "jit");
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 34a2aee41d78..d394262c2af3 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -519,18 +519,6 @@ linux_nat_target::follow_fork (bool follow_child, bool detach_fork)
ptrace (PTRACE_DETACH, child_pid, 0, signo);
}
}
- else
- {
- /* Switching inferior_ptid is not enough, because then
- inferior_thread () would crash by not finding the thread
- in the current inferior. */
- scoped_restore_current_thread restore_current_thread;
- thread_info *child = find_thread_ptid (this, child_ptid);
- switch_to_thread (child);
-
- /* Let the thread_db layer learn about this new process. */
- check_for_thread_db ();
- }
if (has_vforked)
{
@@ -607,9 +595,6 @@ linux_nat_target::follow_fork (bool follow_child, bool detach_fork)
child_lp = add_lwp (inferior_ptid);
child_lp->stopped = 1;
child_lp->last_resume_kind = resume_stop;
-
- /* Let the thread_db layer learn about this new process. */
- check_for_thread_db ();
}
}
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 5426a5c69001..feeaddafe991 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -294,9 +294,6 @@ extern enum tribool have_ptrace_getregset;
(LP) != NULL; \
(LP) = (LP)->next)
-/* Attempt to initialize libthread_db. */
-void check_for_thread_db (void);
-
/* Called from the LWP layer to inform the thread_db layer that PARENT
spawned CHILD. Both LWPs are currently stopped. This function
does whatever is required to have the child LWP under the
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index d1e8c22ac962..c748163b1ece 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -1254,7 +1254,7 @@ check_thread_signals (void)
an inferior is created (or otherwise acquired, e.g. attached to)
and when new shared libraries are loaded into a running process. */
-void
+static void
check_for_thread_db (void)
{
/* Do nothing if we couldn't load libthread_db.so.1. */
diff --git a/gdb/testsuite/gdb.base/jit-elf-fork-main.c b/gdb/testsuite/gdb.base/jit-elf-fork-main.c
new file mode 100644
index 000000000000..7431200b05b2
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-elf-fork-main.c
@@ -0,0 +1,129 @@
+/* This test program is part of GDB, the GNU debugger.
+
+ Copyright 2011-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 <http://www.gnu.org/licenses/>. */
+
+/* Simulate loading of JIT code. */
+
+#include <elf.h>
+#include <link.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include "jit-protocol.h"
+#include "jit-elf-util.h"
+
+static void
+usage (void)
+{
+ fprintf (stderr, "Usage: jit-elf-main libraries...\n");
+ exit (1);
+}
+
+/* Must be defined by .exp file when compiling to know
+ what address to map the ELF binary to. */
+#ifndef LOAD_ADDRESS
+#error "Must define LOAD_ADDRESS"
+#endif
+#ifndef LOAD_INCREMENT
+#error "Must define LOAD_INCREMENT"
+#endif
+
+int
+main (int argc, char *argv[])
+{
+ int i;
+ alarm (300);
+ /* Used as backing storage for GDB to populate argv. */
+ char *fake_argv[10];
+
+ if (argc < 2)
+ {
+ usage ();
+ exit (1);
+ }
+
+ for (i = 1; i < argc; ++i)
+ {
+ size_t obj_size;
+ void *load_addr = (void *) (size_t) (LOAD_ADDRESS + (i - 1) * LOAD_INCREMENT);
+ printf ("Loading %s as JIT at %p\n", argv[i], load_addr);
+ void *addr = load_elf (argv[i], &obj_size, load_addr);
+
+ char name[32];
+ sprintf (name, "jit_function_%04d", i);
+ int (*jit_function) (void) = (int (*) (void)) load_symbol (addr, name);
+
+ /* Link entry at the end of the list. */
+ struct jit_code_entry *const entry = calloc (1, sizeof (*entry));
+ entry->symfile_addr = (const char *)addr;
+ entry->symfile_size = obj_size;
+ entry->prev_entry = __jit_debug_descriptor.relevant_entry;
+ __jit_debug_descriptor.relevant_entry = entry;
+
+ if (entry->prev_entry != NULL)
+ entry->prev_entry->next_entry = entry;
+ else
+ __jit_debug_descriptor.first_entry = entry;
+
+ /* Notify GDB. */
+ __jit_debug_descriptor.action_flag = JIT_REGISTER;
+ __jit_debug_register_code ();
+
+ if (jit_function () != 42)
+ {
+ fprintf (stderr, "unexpected return value\n");
+ exit (1);
+ }
+ }
+
+ i = 0; /* break before fork */
+
+ fork ();
+
+ i = 0; /* break after fork */
+
+ /* Now unregister them all in reverse order. */
+ while (__jit_debug_descriptor.relevant_entry != NULL)
+ {
+ struct jit_code_entry *const entry =
+ __jit_debug_descriptor.relevant_entry;
+ struct jit_code_entry *const prev_entry = entry->prev_entry;
+
+ if (prev_entry != NULL)
+ {
+ prev_entry->next_entry = NULL;
+ entry->prev_entry = NULL;
+ }
+ else
+ __jit_debug_descriptor.first_entry = NULL;
+
+ /* Notify GDB. */
+ __jit_debug_descriptor.action_flag = JIT_UNREGISTER;
+ __jit_debug_register_code ();
+
+ __jit_debug_descriptor.relevant_entry = prev_entry;
+ free (entry);
+ }
+
+ return 0; /* break before return */
+}
diff --git a/gdb/testsuite/gdb.base/jit-elf-fork-solib.c b/gdb/testsuite/gdb.base/jit-elf-fork-solib.c
new file mode 100644
index 000000000000..4215b2dce582
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-elf-fork-solib.c
@@ -0,0 +1,25 @@
+/* This test program is part of GDB, the GNU debugger.
+
+ Copyright 2011-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 <http://www.gnu.org/licenses/>. */
+
+/* This simulates a JIT library. The function name is supplied during
+ compilation as a macro. */
+
+#ifndef FUNCTION_NAME
+#error "Must define the FUNCTION_NAME macro to set a jited function name"
+#endif
+
+int FUNCTION_NAME() { return 42; }
diff --git a/gdb/testsuite/gdb.base/jit-elf-fork.exp b/gdb/testsuite/gdb.base/jit-elf-fork.exp
new file mode 100644
index 000000000000..c8ab69437c3e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-elf-fork.exp
@@ -0,0 +1,186 @@
+# Copyright 2011-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 <http://www.gnu.org/licenses/>.
+
+# Test fork handling of an inferior that has JIT-ed objfiles.
+
+if {[skip_shlib_tests]} {
+ untested "skipping shared library tests"
+ return -1
+}
+
+if {[get_compiler_info]} {
+ untested "could not get compiler info"
+ return 1
+}
+
+load_lib jit-elf-helpers.exp
+
+# The main code that loads and registers JIT objects.
+set main_basename "jit-elf-fork-main"
+set main_srcfile ${srcdir}/${subdir}/${main_basename}.c
+set main_binfile [standard_output_file ${main_basename}]
+
+# The shared library that gets loaded as JIT objects.
+set jit_solib_basename jit-elf-fork-solib
+set jit_solib_srcfile ${srcdir}/${subdir}/${jit_solib_basename}.c
+
+# Compile a shared library to use as JIT objects.
+set jit_solibs_target [compile_and_download_n_jit_so \
+ $jit_solib_basename $jit_solib_srcfile 1]
+if { $jit_solibs_target == -1 } {
+ return
+}
+
+# Compile the main code (which loads the JIT objects).
+if { [compile_jit_main ${main_srcfile} ${main_binfile} {}] != 0 } {
+ return
+}
+
+# Set up for the tests.
+#
+# detach-on-fork and follow-fork-mode are the values to use for the GDB
+# parameters of the same names.
+#
+# Upon return, execution is stopped at the breakpoint just after fork. Which
+# inferiors exist and which inferior is the current one depends on the
+# parameter.
+#
+# The only breakpoint left is one just before the return statement in main, so
+# that the callers can continue execution until there.
+
+proc do_setup { detach-on-fork follow-fork-mode } {
+ clean_restart ${::main_binfile}
+
+ gdb_test_no_output "set detach-on-fork ${detach-on-fork}"
+ gdb_test_no_output "set follow-fork-mode ${follow-fork-mode}"
+
+ if { ![runto_main] } {
+ fail "can't run to main"
+ return -1
+ }
+
+ # Poke desired values directly into inferior.
+ gdb_test_no_output "set var argc=2" "forging argc"
+ gdb_test_no_output "set var argv=fake_argv" "forging argv"
+ set jit_solib_target [lindex $::jit_solibs_target 0]
+ gdb_test_no_output "set var argv\[1]=\"${jit_solib_target}\"" {forging argv[1]}
+
+ # Put a breakpoint before the fork, continue there.
+ gdb_breakpoint [gdb_get_line_number "break before fork" $::main_srcfile]
+ gdb_continue_to_breakpoint "continue to before fork" ".*break before fork.*"
+
+ # We should have one JIT object loaded.
+ gdb_test "maint info jit" " - ${::hex}" "jit-ed objfiles before fork"
+
+ # Put a breakpoint just after the fork, continue there.
+ gdb_breakpoint [gdb_get_line_number "break after fork" $::main_srcfile]
+ gdb_continue_to_breakpoint "continue to after fork" ".*break after fork.*"
+
+ # We should still have one JIT object loaded in whatever inferior we are
+ # currently stopped in, regardless of the mode.
+ gdb_test "maint info jit" " - ${::hex}" "jit-ed objfiles after fork"
+
+ # Delete our breakpoints.
+ delete_breakpoints
+
+ # Put a breakpoint before return, for the convenience of our callers.
+ gdb_breakpoint [gdb_get_line_number "break before return" $::main_srcfile]
+}
+
+proc_with_prefix test_detach_on_fork_off_follow_fork_mode_parent { } {
+ if { [do_setup off parent] == -1 } {
+ return -1
+ }
+
+ # We are stopped in the parent.
+ gdb_test "inferior" "Current inferior is 1.*" "current inferior is parent"
+
+ # Switch to the child, verify there is a JIT-ed objfile.
+ gdb_test "inferior 2" "Switching to inferior 2.*"
+ gdb_test "maint info jit" " - ${::hex}" "jit-ed objfile in child"
+
+ # Continue child past JIT unload, verify there are no more JIT-ed objfiles.
+ gdb_continue_to_breakpoint "continue to before return - child" ".*break before return.*"
+ gdb_test_no_output "maint info jit" "no more jit-ed objfiles in child"
+
+ # Go back to parent, the JIT-ed objfile should still be there.
+ gdb_test "inferior 1" "Switching to inferior 1.*"
+ gdb_test "maint info jit" " - ${::hex}" "jit-ed objfile in parent"
+
+ # Continue parent past JIT unload, verify there are no more JIT-ed objfiles.
+ gdb_continue_to_breakpoint "continue to before return - parent" ".*break before return.*"
+ gdb_test_no_output "maint info jit" "no more jit-ed objfiles in parent"
+}
+
+proc_with_prefix test_detach_on_fork_off_follow_fork_mode_child { } {
+ if { [do_setup off child] == -1 } {
+ return -1
+ }
+
+ # We are stopped in the child. This is the exact same thing as
+ # test_detach_on_fork_off_follow_fork_mode_parent, except that we are
+ # stopped in the child.
+ gdb_test "inferior" "Current inferior is 2.*" "current inferior is child"
+
+ # Switch to the parent, verify there is a JIT-ed objfile.
+ gdb_test "inferior 1" "Switching to inferior 1.*"
+ gdb_test "maint info jit" " - ${::hex}" "jit-ed objfile in parent"
+
+ # Continue parent past JIT unload, verify there are no more JIT-ed objfiles.
+ gdb_continue_to_breakpoint "continue to before return - parent" ".*break before return.*"
+ gdb_test_no_output "maint info jit" "no more jit-ed objfiles in parent"
+
+ # Go back to child, the JIT-ed objfile should still be there.
+ gdb_test "inferior 2" "Switching to inferior 2.*"
+ gdb_test "maint info jit" " - ${::hex}" "jit-ed objfile in child"
+
+ # Continue child past JIT unload, verify there are no more JIT-ed objfiles.
+ gdb_continue_to_breakpoint "continue to before return - child" ".*break before return.*"
+ gdb_test_no_output "maint info jit" "no more jit-ed objfiles in child"
+}
+
+proc_with_prefix test_detach_on_fork_on_follow_fork_mode_parent { } {
+ if { [do_setup on parent] == -1 } {
+ return -1
+ }
+
+ # We are stopped in the parent, child is detached.
+ gdb_test "inferior" "Current inferior is 1.*" "current inferior is parent"
+ gdb_test "inferior 2" "Inferior ID 2 not known." "no inferior 2"
+
+ # Continue past JIT unload, verify there are no more JIT-ed objfiles.
+ gdb_continue_to_breakpoint "continue to before return" ".*break before return.*"
+ gdb_test_no_output "maint info jit" "no more jit-ed objfiles"
+}
+
+proc_with_prefix test_detach_on_fork_on_follow_fork_mode_child { } {
+ if { [do_setup on child] == -1 } {
+ return -1
+ }
+
+ # We are stopped in the child, parent is detached.
+ gdb_test "inferior" "Current inferior is 2.*" "current inferior is child"
+ gdb_test "inferior 1" "Switching to inferior 1 \\\[<null>\\\].*"
+ gdb_test "inferior 2" "Switching to inferior 2.*"
+
+ # Continue past JIT unload, verify there are no more JIT-ed objfiles.
+ gdb_continue_to_breakpoint "continue to before return" ".*break before return.*"
+ gdb_test_no_output "maint info jit" "no more jit-ed objfiles"
+}
+
+test_detach_on_fork_off_follow_fork_mode_parent
+test_detach_on_fork_off_follow_fork_mode_child
+test_detach_on_fork_on_follow_fork_mode_parent
+test_detach_on_fork_on_follow_fork_mode_child
--
2.32.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 2/3] gdb: pass child_ptid and fork kind to target_ops::follow_fork
2021-06-10 14:33 [PATCH v3 0/3] Follow fork improvements Simon Marchi
2021-06-10 14:33 ` [PATCH v3 1/3] gdb: call post_create_inferior at end of follow_fork_inferior Simon Marchi
@ 2021-06-10 14:33 ` Simon Marchi
2021-07-02 17:55 ` Pedro Alves
2021-06-10 14:33 ` [PATCH v3 3/3] gdb: follow-fork: push target and add thread in target_follow_fork Simon Marchi
2 siblings, 1 reply; 12+ messages in thread
From: Simon Marchi @ 2021-06-10 14:33 UTC (permalink / raw)
To: gdb-patches
This is a small cleanup I think would be nice, that I spotted while
doing the following patch.
gdb/ChangeLog:
* target.h (struct target_ops) <follow_fork>: Add ptid and
target_waitkind parameters.
(target_follow_fork): Likewise.
* target.c (default_follow_fork): Likewise.
(target_follow_fork): Likewise.
* fbsd-nat.h (class fbsd_nat_target) <follow_fork>: Likewise.
* fbsd-nat.c (fbsd_nat_target::follow_fork): Likewise.
* linux-nat.h (class linux_nat_target) <follow_fork>: Likewise.
* linux-nat.c (linux_nat_target::follow_fork): Likewise.
* obsd-nat.h (class obsd_nat_target) <follow_fork>: Likewise.
* obsd-nat.c (obsd_nat_target::follow_fork): Likewise.
* remote.c (class remote_target) <follow_fork>: Likewise.
* target-debug.h (target_debug_print_target_waitkind): New.
* target-delegates.c: Re-generate.
Change-Id: I5421a542f2e19100a22b74cc333d2b235d0de3c8
---
gdb/fbsd-nat.c | 8 ++++----
gdb/fbsd-nat.h | 2 +-
gdb/infrun.c | 15 +++++++--------
gdb/linux-nat.c | 19 +++++++------------
gdb/linux-nat.h | 2 +-
gdb/obsd-nat.c | 8 +++-----
gdb/obsd-nat.h | 2 +-
gdb/remote.c | 18 ++++++------------
gdb/target-debug.h | 2 ++
gdb/target-delegates.c | 24 ++++++++++++++----------
gdb/target.c | 8 +++++---
gdb/target.h | 15 ++++++++-------
12 files changed, 59 insertions(+), 64 deletions(-)
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 581c04d5f83d..42c95834e50d 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -1406,12 +1406,12 @@ fbsd_nat_target::supports_stopped_by_sw_breakpoint ()
the ptid of the followed inferior. */
void
-fbsd_nat_target::follow_fork (bool follow_child, bool detach_fork)
+fbsd_nat_target::follow_fork (ptid_t child_ptid, target_waitkind fork_kind,
+ bool follow_child, bool detach_fork)
{
if (!follow_child && detach_fork)
{
- struct thread_info *tp = inferior_thread ();
- pid_t child_pid = tp->pending_follow.value.related_pid.pid ();
+ pid_t child_pid = child_ptid.pid ();
/* Breakpoints have already been detached from the child by
infrun.c. */
@@ -1420,7 +1420,7 @@ fbsd_nat_target::follow_fork (bool follow_child, bool detach_fork)
perror_with_name (("ptrace"));
#ifndef PTRACE_VFORK
- if (tp->pending_follow.kind == TARGET_WAITKIND_VFORKED)
+ if (fork_kind == TARGET_WAITKIND_VFORKED)
{
/* We can't insert breakpoints until the child process has
finished with the shared memory region. The parent
diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
index 772655d320e6..0f8b30df225b 100644
--- a/gdb/fbsd-nat.h
+++ b/gdb/fbsd-nat.h
@@ -80,7 +80,7 @@ class fbsd_nat_target : public inf_ptrace_target
#endif
#ifdef TDP_RFPPWAIT
- void follow_fork (bool, bool) override;
+ void follow_fork (ptid_t, target_waitkind, bool, bool) override;
int insert_fork_catchpoint (int) override;
int remove_fork_catchpoint (int) override;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 791ccee6ea0b..608fc29c078c 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -404,13 +404,12 @@ show_follow_fork_mode_string (struct ui_file *file, int from_tty,
static bool
follow_fork_inferior (bool follow_child, bool detach_fork)
{
- int has_vforked;
- ptid_t parent_ptid, child_ptid;
-
- has_vforked = (inferior_thread ()->pending_follow.kind
- == TARGET_WAITKIND_VFORKED);
- parent_ptid = inferior_ptid;
- child_ptid = inferior_thread ()->pending_follow.value.related_pid;
+ target_waitkind fork_kind = inferior_thread ()->pending_follow.kind;
+ gdb_assert (fork_kind == TARGET_WAITKIND_FORKED
+ || fork_kind == TARGET_WAITKIND_VFORKED);
+ bool has_vforked = fork_kind == TARGET_WAITKIND_VFORKED;
+ ptid_t parent_ptid = inferior_ptid;
+ ptid_t child_ptid = inferior_thread ()->pending_follow.value.related_pid;
if (has_vforked
&& !non_stop /* Non-stop always resumes both branches. */
@@ -649,7 +648,7 @@ holding the child stopped. Try \"set detach-on-fork\" or \
switch_to_thread (child_thr);
}
- target_follow_fork (follow_child, detach_fork);
+ target_follow_fork (child_ptid, fork_kind, follow_child, detach_fork);
/* If we ended up creating a new inferior, call post_create_inferior to inform
the various subcomponents. */
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index d394262c2af3..b6fb2fa4a4b6 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -447,24 +447,19 @@ typedef std::unique_ptr<struct lwp_info, lwp_deleter> lwp_info_up;
unchanged. */
void
-linux_nat_target::follow_fork (bool follow_child, bool detach_fork)
+linux_nat_target::follow_fork (ptid_t child_ptid, target_waitkind fork_kind,
+ bool follow_child, bool detach_fork)
{
if (!follow_child)
{
- struct lwp_info *child_lp = NULL;
- int has_vforked;
- ptid_t parent_ptid, child_ptid;
- int parent_pid, child_pid;
-
- has_vforked = (inferior_thread ()->pending_follow.kind
- == TARGET_WAITKIND_VFORKED);
- parent_ptid = inferior_ptid;
+ bool has_vforked = fork_kind == TARGET_WAITKIND_VFORKED;
+ ptid_t parent_ptid = inferior_ptid;
child_ptid = inferior_thread ()->pending_follow.value.related_pid;
- parent_pid = parent_ptid.lwp ();
- child_pid = child_ptid.lwp ();
+ int parent_pid = parent_ptid.lwp ();
+ int child_pid = child_ptid.lwp ();
/* We're already attached to the parent, by default. */
- child_lp = add_lwp (child_ptid);
+ lwp_info *child_lp = add_lwp (child_ptid);
child_lp->stopped = 1;
child_lp->last_resume_kind = resume_stop;
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index feeaddafe991..ee36c56519b2 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -133,7 +133,7 @@ class linux_nat_target : public inf_ptrace_target
void post_attach (int) override;
- void follow_fork (bool, bool) override;
+ void follow_fork (ptid_t, target_waitkind, bool, bool) override;
std::vector<static_tracepoint_marker>
static_tracepoint_markers_by_strid (const char *id) override;
diff --git a/gdb/obsd-nat.c b/gdb/obsd-nat.c
index a8164ddbad15..46fdc0676eab 100644
--- a/gdb/obsd-nat.c
+++ b/gdb/obsd-nat.c
@@ -194,17 +194,15 @@ obsd_nat_target::post_startup_inferior (ptid_t pid)
the ptid of the followed inferior. */
void
-obsd_nat_target::follow_fork (bool follow_child, bool detach_fork)
+obsd_nat_target::follow_fork (ptid_t child_ptid, target_waitkind fork_kind,
+ bool follow_child, bool detach_fork)
{
if (!follow_child)
{
- struct thread_info *tp = inferior_thread ();
- pid_t child_pid = tp->pending_follow.value.related_pid.pid ();
-
/* Breakpoints have already been detached from the child by
infrun.c. */
- if (ptrace (PT_DETACH, child_pid, (PTRACE_TYPE_ARG3)1, 0) == -1)
+ if (ptrace (PT_DETACH, child_ptid.pid (), (PTRACE_TYPE_ARG3)1, 0) == -1)
perror_with_name (("ptrace"));
}
}
diff --git a/gdb/obsd-nat.h b/gdb/obsd-nat.h
index 60b078fd0d30..ddd4baf77614 100644
--- a/gdb/obsd-nat.h
+++ b/gdb/obsd-nat.h
@@ -30,7 +30,7 @@ class obsd_nat_target : public inf_ptrace_target
ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) override;
#ifdef PT_GET_PROCESS_STATE
- void follow_fork (bool, bool) override;
+ void follow_fork (ptid_t, target_waitkind, bool, bool) override;
int insert_fork_catchpoint (int) override;
diff --git a/gdb/remote.c b/gdb/remote.c
index 39bdd2e4ab08..ecdf834093be 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -682,7 +682,7 @@ class remote_target : public process_stratum_target
const struct btrace_config *btrace_conf (const struct btrace_target_info *) override;
bool augmented_libraries_svr4_read () override;
- void follow_fork (bool, bool) override;
+ void follow_fork (ptid_t, target_waitkind, bool, bool) override;
void follow_exec (inferior *, ptid_t, const char *) override;
int insert_fork_catchpoint (int) override;
int remove_fork_catchpoint (int) override;
@@ -5899,13 +5899,13 @@ extended_remote_target::detach (inferior *inf, int from_tty)
remote target as well. */
void
-remote_target::follow_fork (bool follow_child, bool detach_fork)
+remote_target::follow_fork (ptid_t child_ptid, target_waitkind fork_kind,
+ bool follow_child, bool detach_fork)
{
struct remote_state *rs = get_remote_state ();
- enum target_waitkind kind = inferior_thread ()->pending_follow.kind;
- if ((kind == TARGET_WAITKIND_FORKED && remote_fork_event_p (rs))
- || (kind == TARGET_WAITKIND_VFORKED && remote_vfork_event_p (rs)))
+ if ((fork_kind == TARGET_WAITKIND_FORKED && remote_fork_event_p (rs))
+ || (fork_kind == TARGET_WAITKIND_VFORKED && remote_vfork_event_p (rs)))
{
/* When following the parent and detaching the child, we detach
the child here. For the case of following the child and
@@ -5916,13 +5916,7 @@ remote_target::follow_fork (bool follow_child, bool detach_fork)
if (detach_fork && !follow_child)
{
/* Detach the fork child. */
- ptid_t child_ptid;
- pid_t child_pid;
-
- child_ptid = inferior_thread ()->pending_follow.value.related_pid;
- child_pid = child_ptid.pid ();
-
- remote_detach_pid (child_pid);
+ remote_detach_pid (child_ptid.pid ());
}
}
}
diff --git a/gdb/target-debug.h b/gdb/target-debug.h
index c3004c2c229b..5949441bc66c 100644
--- a/gdb/target-debug.h
+++ b/gdb/target-debug.h
@@ -174,6 +174,8 @@
target_debug_do_print (host_address_to_string (X.data ()))
#define target_debug_print_gdb_unique_xmalloc_ptr_char(X) \
target_debug_do_print (X.get ())
+#define target_debug_print_target_waitkind(X) \
+ target_debug_do_print (pulongest (X))
static void
target_debug_print_struct_target_waitstatus_p (struct target_waitstatus *status)
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 3e759a2f80ea..fa4cc5bb2dfc 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -56,7 +56,7 @@ struct dummy_target : public target_ops
int remove_fork_catchpoint (int arg0) override;
int insert_vfork_catchpoint (int arg0) override;
int remove_vfork_catchpoint (int arg0) override;
- void follow_fork (bool arg0, bool arg1) override;
+ void follow_fork (ptid_t arg0, target_waitkind arg1, bool arg2, bool arg3) override;
int insert_exec_catchpoint (int arg0) override;
int remove_exec_catchpoint (int arg0) override;
void follow_exec (inferior *arg0, ptid_t arg1, const char *arg2) override;
@@ -231,7 +231,7 @@ struct debug_target : public target_ops
int remove_fork_catchpoint (int arg0) override;
int insert_vfork_catchpoint (int arg0) override;
int remove_vfork_catchpoint (int arg0) override;
- void follow_fork (bool arg0, bool arg1) override;
+ void follow_fork (ptid_t arg0, target_waitkind arg1, bool arg2, bool arg3) override;
int insert_exec_catchpoint (int arg0) override;
int remove_exec_catchpoint (int arg0) override;
void follow_exec (inferior *arg0, ptid_t arg1, const char *arg2) override;
@@ -1519,26 +1519,30 @@ debug_target::remove_vfork_catchpoint (int arg0)
}
void
-target_ops::follow_fork (bool arg0, bool arg1)
+target_ops::follow_fork (ptid_t arg0, target_waitkind arg1, bool arg2, bool arg3)
{
- this->beneath ()->follow_fork (arg0, arg1);
+ this->beneath ()->follow_fork (arg0, arg1, arg2, arg3);
}
void
-dummy_target::follow_fork (bool arg0, bool arg1)
+dummy_target::follow_fork (ptid_t arg0, target_waitkind arg1, bool arg2, bool arg3)
{
- default_follow_fork (this, arg0, arg1);
+ default_follow_fork (this, arg0, arg1, arg2, arg3);
}
void
-debug_target::follow_fork (bool arg0, bool arg1)
+debug_target::follow_fork (ptid_t arg0, target_waitkind arg1, bool arg2, bool arg3)
{
fprintf_unfiltered (gdb_stdlog, "-> %s->follow_fork (...)\n", this->beneath ()->shortname ());
- this->beneath ()->follow_fork (arg0, arg1);
+ this->beneath ()->follow_fork (arg0, arg1, arg2, arg3);
fprintf_unfiltered (gdb_stdlog, "<- %s->follow_fork (", this->beneath ()->shortname ());
- target_debug_print_bool (arg0);
+ target_debug_print_ptid_t (arg0);
fputs_unfiltered (", ", gdb_stdlog);
- target_debug_print_bool (arg1);
+ target_debug_print_target_waitkind (arg1);
+ fputs_unfiltered (", ", gdb_stdlog);
+ target_debug_print_bool (arg2);
+ fputs_unfiltered (", ", gdb_stdlog);
+ target_debug_print_bool (arg3);
fputs_unfiltered (")\n", gdb_stdlog);
}
diff --git a/gdb/target.c b/gdb/target.c
index 6babfc562563..c5468d332ec2 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2700,7 +2700,8 @@ target_program_signals (gdb::array_view<const unsigned char> program_signals)
}
static void
-default_follow_fork (struct target_ops *self, bool follow_child,
+default_follow_fork (struct target_ops *self, ptid_t child_ptid,
+ target_waitkind fork_kind, bool follow_child,
bool detach_fork)
{
/* Some target returned a fork event, but did not know how to follow it. */
@@ -2711,11 +2712,12 @@ default_follow_fork (struct target_ops *self, bool follow_child,
/* See target.h. */
void
-target_follow_fork (bool follow_child, bool detach_fork)
+target_follow_fork (ptid_t child_ptid, target_waitkind fork_kind,
+ bool follow_child, bool detach_fork)
{
target_ops *target = current_inferior ()->top_target ();
- return target->follow_fork (follow_child, detach_fork);
+ return target->follow_fork (child_ptid, fork_kind, follow_child, detach_fork);
}
/* See target.h. */
diff --git a/gdb/target.h b/gdb/target.h
index e22f9038197c..3395ac333169 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -636,7 +636,7 @@ struct target_ops
TARGET_DEFAULT_RETURN (1);
virtual int remove_vfork_catchpoint (int)
TARGET_DEFAULT_RETURN (1);
- virtual void follow_fork (bool, bool)
+ virtual void follow_fork (ptid_t, target_waitkind, bool, bool)
TARGET_DEFAULT_FUNC (default_follow_fork);
virtual int insert_exec_catchpoint (int)
TARGET_DEFAULT_RETURN (1);
@@ -1713,13 +1713,14 @@ extern int target_insert_vfork_catchpoint (int pid);
extern int target_remove_vfork_catchpoint (int pid);
-/* If the inferior forks or vforks, this function will be called at
- the next resume in order to perform any bookkeeping and fiddling
- necessary to continue debugging either the parent or child, as
- requested, and releasing the other. Information about the fork
- or vfork event is available via get_last_target_status (). */
+/* Call the follow_fork method on the current target stack.
-void target_follow_fork (bool follow_child, bool detach_fork);
+ This function is called when the inferior or vforks, to perform any
+ bookkeeping and fiddling necessary to continue debugging either the parent,
+ the child or both. */
+
+void target_follow_fork (ptid_t child_ptid, target_waitkind fork_kind,
+ bool follow_child, bool detach_fork);
/* Handle the target-specific bookkeeping required when the inferior makes an
exec call.
--
2.32.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 3/3] gdb: follow-fork: push target and add thread in target_follow_fork
2021-06-10 14:33 [PATCH v3 0/3] Follow fork improvements Simon Marchi
2021-06-10 14:33 ` [PATCH v3 1/3] gdb: call post_create_inferior at end of follow_fork_inferior Simon Marchi
2021-06-10 14:33 ` [PATCH v3 2/3] gdb: pass child_ptid and fork kind to target_ops::follow_fork Simon Marchi
@ 2021-06-10 14:33 ` Simon Marchi
2021-07-02 17:55 ` Pedro Alves
2 siblings, 1 reply; 12+ messages in thread
From: Simon Marchi @ 2021-06-10 14:33 UTC (permalink / raw)
To: gdb-patches
In the context of ROCm-gdb [1], the ROCm target sits on top of the
linux-nat target. when a process forks, it needs to carry over some
data from the forking inferior to the fork child inferior. Ideally, the
ROCm target would implement the follow_fork target_ops method, but there
are some small problems. This patch fixes these, which helps the ROCm
target, but also makes things more consistent and a bit nicer in
general, I believe.
The main problem is: when follow-fork-mode is "parent",
target_follow_fork is called with the parent as the current inferior.
When it's "child", target_follow_fork is called with the child as the
current inferior. This means that target_follow_fork is sometimes
called on the parent's target stack and sometimes on the child's target
stack.
The parent's target stack may contain targets above the process target,
such as the ROCm target. So if follow-fork-child is "parent", the ROCm
target would get notified of the fork and do whatever is needed. But
the child's target stack, at that moment, only contains the exec and
process target copied over from the parent. The child's target stack is
set up by follow_fork_inferior, before calling target_follow_fork. In
that case, the ROCm target wouldn't get notified of the fork.
For consistency, I think it would be good to always call
target_follow_fork on the parent inferior's target stack. I think it
makes sense as a way to indicate "this inferior has called fork, do
whatever is needed". The desired outcome of the fork (whether an
inferior is created for the child, do we need to detach from the child)
can be indicated by passed parameter.
I therefore propose these changes:
- make follow_fork_inferior always call target_follow_fork with the
parent as the current inferior. That lets all targets present on the
parent's target stack do some fork-related handling and push
themselves on the fork child's target stack if needed.
For this purpose, pass the child inferior down to target_follow_fork
and follow_fork implementations. This is nullptr if no inferior is
created for the child, because we want to detach from it.
- as a result, in follow_fork_inferior, detach from the parent inferior
(if needed) only after the target_follow_fork call. This is needed
because we want to call target_follow_fork before the parent's
starget stack is torn down.
- hand over to the targets in the parent's target stack (including the
process target) the responsibility to push themselves, if needed, to
the child's target stack. Also hand over the responsibility to the
process target, at the same time, to create the child's initial
thread (just like we do for follow_exec).
- pass the child inferior to exec_on_vfork, so we don't need to swap
the current inferior between parent and child. Nothing in
exec_on_vfork depends on the current inferior, after this change.
Although this could perhaps be replaced with just having the exec
target implement follow_fork and push itself in the child's target
stack, like the process target does... We would just need to make
sure the process target calls beneath()->follow_fork(...). I'm not
sure about this one.
gdb/ChangeLog:
* target.h (struct target_ops) <follow_fork>: Add inferior*
parameter.
(target_follow_fork): Likewise.
* target.c (default_follow_fork): Likewise.
(target_follow_fork): Likewise.
* fbsd-nat.h (class fbsd_nat_target) <follow_fork>: Likewise.
(fbsd_nat_target::follow_fork): Likewise, and call
inf_ptrace_target::follow_fork.
* linux-nat.h (class linux_nat_target) <follow_fork>: Likewise.
* linux-nat.c (linux_nat_target::follow_fork): Likewise, and
call inf_ptrace_target::follow_fork.
* obsd-nat.h (obsd_nat_target) <follow_fork>: Likewise.
* obsd-nat.c (obsd_nat_target::follow_fork): Likewise, and call
inf_ptrace_target::follow_fork.
* remote.c (class remote_target) <follow_fork>: Likewise.
(remote_target::follow_fork): Likewise, and call
process_stratum_target::follow_fork.
* process-stratum-target.h (class process_stratum_target)
<follow_fork>: New.
* process-stratum-target.c
(process_stratum_target::follow_fork): New.
* target-delegates.c: Re-generate.
[1] https://github.com/ROCm-Developer-Tools/ROCgdb
Change-Id: I460bd0af850f0485e8aed4b24c6d8262a4c69929
---
gdb/exec.c | 6 +-
gdb/exec.h | 9 +-
gdb/fbsd-nat.c | 8 +-
gdb/fbsd-nat.h | 2 +-
gdb/infrun.c | 166 +++++++++++++++--------------------
gdb/linux-nat.c | 15 ++--
gdb/linux-nat.h | 2 +-
gdb/obsd-nat.c | 9 +-
gdb/obsd-nat.h | 2 +-
gdb/process-stratum-target.c | 15 ++++
gdb/process-stratum-target.h | 9 ++
gdb/remote.c | 10 ++-
gdb/target-delegates.c | 24 ++---
gdb/target.c | 24 +++--
gdb/target.h | 7 +-
15 files changed, 166 insertions(+), 142 deletions(-)
diff --git a/gdb/exec.c b/gdb/exec.c
index ff0db82a69b9..07c8f3d42fda 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -679,10 +679,10 @@ program_space::remove_target_sections (void *owner)
/* See exec.h. */
void
-exec_on_vfork ()
+exec_on_vfork (inferior *vfork_child)
{
- if (!current_program_space->target_sections ().empty ())
- current_inferior ()->push_target (&exec_ops);
+ if (!vfork_child->pspace->target_sections ().empty ())
+ vfork_child->push_target (&exec_ops);
}
\f
diff --git a/gdb/exec.h b/gdb/exec.h
index 1119953dc8fb..c237f831d234 100644
--- a/gdb/exec.h
+++ b/gdb/exec.h
@@ -34,12 +34,11 @@ struct objfile;
extern target_section_table build_section_table (struct bfd *);
-/* The current inferior is a child vforked and its program space is
- shared with its parent. This pushes the exec target on the
- current/child inferior's target stack if there are sections in the
- program space's section table. */
+/* VFORK_CHILD is a child vforked and its program space is shared with its
+ parent. This pushes the exec target on that inferior's target stack if
+ there are sections in the program space's section table. */
-extern void exec_on_vfork ();
+extern void exec_on_vfork (inferior *vfork_child);
/* Read from mappable read-only sections of BFD executable files.
Return TARGET_XFER_OK, if read is successful. Return
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 42c95834e50d..148b49497af4 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -1406,9 +1406,13 @@ fbsd_nat_target::supports_stopped_by_sw_breakpoint ()
the ptid of the followed inferior. */
void
-fbsd_nat_target::follow_fork (ptid_t child_ptid, target_waitkind fork_kind,
- bool follow_child, bool detach_fork)
+fbsd_nat_target::follow_fork (inferior *child_inf, ptid_t child_ptid,
+ target_waitkind fork_kind, bool follow_child,
+ bool detach_fork)
{
+ inf_ptrace_target::follow_fork (child_inf, child_ptid, fork_kind,
+ follow_child, detach_fork);
+
if (!follow_child && detach_fork)
{
pid_t child_pid = child_ptid.pid ();
diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
index 0f8b30df225b..0c67b24c29f2 100644
--- a/gdb/fbsd-nat.h
+++ b/gdb/fbsd-nat.h
@@ -80,7 +80,7 @@ class fbsd_nat_target : public inf_ptrace_target
#endif
#ifdef TDP_RFPPWAIT
- void follow_fork (ptid_t, target_waitkind, bool, bool) override;
+ void follow_fork (inferior *, ptid_t, target_waitkind, bool, bool) override;
int insert_fork_catchpoint (int) override;
int remove_fork_catchpoint (int) override;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 608fc29c078c..16b3d9028f57 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -428,7 +428,8 @@ holding the child stopped. Try \"set detach-on-fork\" or \
return true;
}
- thread_info *child_thr = nullptr;
+ inferior *parent_inf = current_inferior ();
+ inferior *child_inf = nullptr;
if (!follow_child)
{
@@ -462,25 +463,15 @@ holding the child stopped. Try \"set detach-on-fork\" or \
}
else
{
- struct inferior *parent_inf, *child_inf;
-
/* Add process to GDB's tables. */
child_inf = add_inferior (child_ptid.pid ());
- parent_inf = current_inferior ();
child_inf->attach_flag = parent_inf->attach_flag;
copy_terminal_info (child_inf, parent_inf);
child_inf->gdbarch = parent_inf->gdbarch;
copy_inferior_target_desc_info (child_inf, parent_inf);
- scoped_restore_current_pspace_and_thread restore_pspace_thread;
-
- set_current_inferior (child_inf);
- switch_to_no_thread ();
child_inf->symfile_flags = SYMFILE_NO_READ;
- child_inf->push_target (parent_inf->process_target ());
- child_thr = add_thread_silent (child_inf->process_target (),
- child_ptid);
/* If this is a vfork child, then the address-space is
shared with the parent. */
@@ -489,7 +480,7 @@ holding the child stopped. Try \"set detach-on-fork\" or \
child_inf->pspace = parent_inf->pspace;
child_inf->aspace = parent_inf->aspace;
- exec_on_vfork ();
+ exec_on_vfork (child_inf);
/* The parent will be frozen until the child is done
with the shared region. Keep track of the
@@ -498,32 +489,18 @@ holding the child stopped. Try \"set detach-on-fork\" or \
child_inf->pending_detach = 0;
parent_inf->vfork_child = child_inf;
parent_inf->pending_detach = 0;
-
- /* Now that the inferiors and program spaces are all
- wired up, we can switch to the child thread (which
- switches inferior and program space too). */
- switch_to_thread (child_thr);
}
else
{
child_inf->aspace = new_address_space ();
child_inf->pspace = new program_space (child_inf->aspace);
child_inf->removable = 1;
- set_current_program_space (child_inf->pspace);
clone_program_space (child_inf->pspace, parent_inf->pspace);
-
- /* solib_create_inferior_hook relies on the current
- thread. */
- switch_to_thread (child_thr);
}
}
if (has_vforked)
{
- struct inferior *parent_inf;
-
- parent_inf = current_inferior ();
-
/* If we detached from the child, then we have to be careful
to not insert breakpoints in the parent until the child
is done with the shared memory region. However, if we're
@@ -538,8 +515,6 @@ holding the child stopped. Try \"set detach-on-fork\" or \
else
{
/* Follow the child. */
- struct inferior *parent_inf, *child_inf;
- struct program_space *parent_pspace;
if (print_inferior_events)
{
@@ -559,71 +534,12 @@ holding the child stopped. Try \"set detach-on-fork\" or \
child_inf = add_inferior (child_ptid.pid ());
- parent_inf = current_inferior ();
child_inf->attach_flag = parent_inf->attach_flag;
copy_terminal_info (child_inf, parent_inf);
child_inf->gdbarch = parent_inf->gdbarch;
copy_inferior_target_desc_info (child_inf, parent_inf);
- parent_pspace = parent_inf->pspace;
-
- process_stratum_target *target = parent_inf->process_target ();
-
- {
- /* Hold a strong reference to the target while (maybe)
- detaching the parent. Otherwise detaching could close the
- target. */
- auto target_ref = target_ops_ref::new_reference (target);
-
- /* If we're vforking, we want to hold on to the parent until
- the child exits or execs. At child exec or exit time we
- can remove the old breakpoints from the parent and detach
- or resume debugging it. Otherwise, detach the parent now;
- we'll want to reuse it's program/address spaces, but we
- can't set them to the child before removing breakpoints
- from the parent, otherwise, the breakpoints module could
- decide to remove breakpoints from the wrong process (since
- they'd be assigned to the same address space). */
-
- if (has_vforked)
- {
- gdb_assert (child_inf->vfork_parent == NULL);
- gdb_assert (parent_inf->vfork_child == NULL);
- child_inf->vfork_parent = parent_inf;
- child_inf->pending_detach = 0;
- parent_inf->vfork_child = child_inf;
- parent_inf->pending_detach = detach_fork;
- parent_inf->waiting_for_vfork_done = 0;
- }
- else if (detach_fork)
- {
- if (print_inferior_events)
- {
- /* Ensure that we have a process ptid. */
- ptid_t process_ptid = ptid_t (parent_ptid.pid ());
-
- target_terminal::ours_for_output ();
- fprintf_filtered (gdb_stdlog,
- _("[Detaching after fork from "
- "parent %s]\n"),
- target_pid_to_str (process_ptid).c_str ());
- }
-
- target_detach (parent_inf, 0);
- parent_inf = NULL;
- }
-
- /* Note that the detach above makes PARENT_INF dangling. */
-
- /* Add the child thread to the appropriate lists, and switch
- to this new thread, before cloning the program space, and
- informing the solib layer about this new process. */
-
- set_current_inferior (child_inf);
- child_inf->push_target (target);
- }
-
- child_thr = add_thread_silent (target, child_ptid);
+ program_space *parent_pspace = parent_inf->pspace;
/* If this is a vfork child, then the address-space is shared
with the parent. If we detached from the parent, then we can
@@ -633,7 +549,7 @@ holding the child stopped. Try \"set detach-on-fork\" or \
child_inf->pspace = parent_pspace;
child_inf->aspace = child_inf->pspace->aspace;
- exec_on_vfork ();
+ exec_on_vfork (child_inf);
}
else
{
@@ -641,22 +557,80 @@ holding the child stopped. Try \"set detach-on-fork\" or \
child_inf->pspace = new program_space (child_inf->aspace);
child_inf->removable = 1;
child_inf->symfile_flags = SYMFILE_NO_READ;
- set_current_program_space (child_inf->pspace);
clone_program_space (child_inf->pspace, parent_pspace);
}
-
- switch_to_thread (child_thr);
}
- target_follow_fork (child_ptid, fork_kind, follow_child, detach_fork);
+ gdb_assert (current_inferior () == parent_inf);
+
+ /* target_follow_fork is responsible for pushing the appropriate targets on
+ the new inferior's target stack and adding the initial thread (with ptid
+ CHILD_PTID). */
+ target_follow_fork (child_inf, child_ptid, fork_kind, follow_child,
+ detach_fork);
+
+ /* target_follow_fork must leave the parent as the current inferior. If we
+ want to follow the child, we make it the current one below. */
+ gdb_assert (current_inferior () == parent_inf);
+
+ /* If there is a child inferior, target_follow_fork must have created a thread
+ for it. */
+ if (child_inf != nullptr)
+ gdb_assert (child_inf->thread_list != nullptr);
+
+ /* Detach the parent if needed. */
+ if (follow_child)
+ {
+ /* If we're vforking, we want to hold on to the parent until
+ the child exits or execs. At child exec or exit time we
+ can remove the old breakpoints from the parent and detach
+ or resume debugging it. Otherwise, detach the parent now;
+ we'll want to reuse it's program/address spaces, but we
+ can't set them to the child before removing breakpoints
+ from the parent, otherwise, the breakpoints module could
+ decide to remove breakpoints from the wrong process (since
+ they'd be assigned to the same address space). */
+
+ if (has_vforked)
+ {
+ gdb_assert (child_inf->vfork_parent == NULL);
+ gdb_assert (parent_inf->vfork_child == NULL);
+ child_inf->vfork_parent = parent_inf;
+ child_inf->pending_detach = 0;
+ parent_inf->vfork_child = child_inf;
+ parent_inf->pending_detach = detach_fork;
+ parent_inf->waiting_for_vfork_done = 0;
+ }
+ else if (detach_fork)
+ {
+ if (print_inferior_events)
+ {
+ /* Ensure that we have a process ptid. */
+ ptid_t process_ptid = ptid_t (parent_ptid.pid ());
+
+ target_terminal::ours_for_output ();
+ fprintf_filtered (gdb_stdlog,
+ _("[Detaching after fork from "
+ "parent %s]\n"),
+ target_pid_to_str (process_ptid).c_str ());
+ }
+
+ target_detach (parent_inf, 0);
+ }
+ }
/* If we ended up creating a new inferior, call post_create_inferior to inform
the various subcomponents. */
- if (child_thr != nullptr)
+ if (child_inf != nullptr)
{
- scoped_restore_current_thread restore;
- switch_to_thread (child_thr);
+ /* If FOLLOW_CHILD, we leave CHILD_INF as the current inferior
+ (do not restore the parent as the current inferior). */
+ gdb::optional<scoped_restore_current_thread> maybe_restore;
+
+ if (!follow_child)
+ maybe_restore.emplace ();
+ switch_to_thread (*child_inf->threads ().begin ());
post_create_inferior (0);
}
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index b6fb2fa4a4b6..6c009b6a2970 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -442,19 +442,20 @@ struct lwp_deleter
typedef std::unique_ptr<struct lwp_info, lwp_deleter> lwp_info_up;
-/* Target hook for follow_fork. On entry inferior_ptid must be the
- ptid of the followed inferior. At return, inferior_ptid will be
- unchanged. */
+/* Target hook for follow_fork. */
void
-linux_nat_target::follow_fork (ptid_t child_ptid, target_waitkind fork_kind,
- bool follow_child, bool detach_fork)
+linux_nat_target::follow_fork (inferior *child_inf, ptid_t child_ptid,
+ target_waitkind fork_kind, bool follow_child,
+ bool detach_fork)
{
+ inf_ptrace_target::follow_fork (child_inf, child_ptid, fork_kind,
+ follow_child, detach_fork);
+
if (!follow_child)
{
bool has_vforked = fork_kind == TARGET_WAITKIND_VFORKED;
ptid_t parent_ptid = inferior_ptid;
- child_ptid = inferior_thread ()->pending_follow.value.related_pid;
int parent_pid = parent_ptid.lwp ();
int child_pid = child_ptid.lwp ();
@@ -587,7 +588,7 @@ linux_nat_target::follow_fork (ptid_t child_ptid, target_waitkind fork_kind,
{
struct lwp_info *child_lp;
- child_lp = add_lwp (inferior_ptid);
+ child_lp = add_lwp (child_ptid);
child_lp->stopped = 1;
child_lp->last_resume_kind = resume_stop;
}
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index ee36c56519b2..83bd6d4a6786 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -133,7 +133,7 @@ class linux_nat_target : public inf_ptrace_target
void post_attach (int) override;
- void follow_fork (ptid_t, target_waitkind, bool, bool) override;
+ void follow_fork (inferior *, ptid_t, target_waitkind, bool, bool) override;
std::vector<static_tracepoint_marker>
static_tracepoint_markers_by_strid (const char *id) override;
diff --git a/gdb/obsd-nat.c b/gdb/obsd-nat.c
index 46fdc0676eab..ab433524dadf 100644
--- a/gdb/obsd-nat.c
+++ b/gdb/obsd-nat.c
@@ -190,13 +190,16 @@ obsd_nat_target::post_startup_inferior (ptid_t pid)
perror_with_name (("ptrace"));
}
-/* Target hook for follow_fork. On entry and at return inferior_ptid is
- the ptid of the followed inferior. */
+/* Target hook for follow_fork. */
void
-obsd_nat_target::follow_fork (ptid_t child_ptid, target_waitkind fork_kind,
+obsd_nat_target::follow_fork (inferior *child_inf, ptid_t child_ptid,
+ target_waitkind fork_kind,
bool follow_child, bool detach_fork)
{
+ inf_ptrace_target::follow_fork (child_inf, child_ptid, fork_kind,
+ follow_child, detach_fork);
+
if (!follow_child)
{
/* Breakpoints have already been detached from the child by
diff --git a/gdb/obsd-nat.h b/gdb/obsd-nat.h
index ddd4baf77614..bb9f214278ac 100644
--- a/gdb/obsd-nat.h
+++ b/gdb/obsd-nat.h
@@ -30,7 +30,7 @@ class obsd_nat_target : public inf_ptrace_target
ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) override;
#ifdef PT_GET_PROCESS_STATE
- void follow_fork (ptid_t, target_waitkind, bool, bool) override;
+ void follow_fork (inferior *inf, ptid_t, target_waitkind, bool, bool) override;
int insert_fork_catchpoint (int) override;
diff --git a/gdb/process-stratum-target.c b/gdb/process-stratum-target.c
index c851090a7f2e..9933debde4c3 100644
--- a/gdb/process-stratum-target.c
+++ b/gdb/process-stratum-target.c
@@ -106,6 +106,21 @@ process_stratum_target::follow_exec (inferior *follow_inf, ptid_t ptid,
/* See process-stratum-target.h. */
+void
+process_stratum_target::follow_fork (inferior *child_inf, ptid_t child_ptid,
+ target_waitkind fork_kind,
+ bool follow_child,
+ bool detach_on_fork)
+{
+ if (child_inf != nullptr)
+ {
+ child_inf->push_target (this);
+ add_thread_silent (this, child_ptid);
+ }
+}
+
+/* See process-stratum-target.h. */
+
std::set<process_stratum_target *>
all_non_exited_process_targets ()
{
diff --git a/gdb/process-stratum-target.h b/gdb/process-stratum-target.h
index 31a97753db9c..22f42c3c4ef6 100644
--- a/gdb/process-stratum-target.h
+++ b/gdb/process-stratum-target.h
@@ -71,6 +71,15 @@ class process_stratum_target : public target_ops
void follow_exec (inferior *follow_inf, ptid_t ptid,
const char *execd_pathname) override;
+ /* Default implementation of follow_fork.
+
+ If a child inferior was created by infrun while following the fork
+ (CHILD_INF is non-nullptr), push this target on CHILD_INF's target stack
+ and add an initial thread with ptid CHILD_PTID. */
+ void follow_fork (inferior *child_inf, ptid_t child_ptid,
+ target_waitkind fork_kind, bool follow_child,
+ bool detach_on_fork) override;
+
/* True if any thread is, or may be executing. We need to track
this separately because until we fully sync the thread list, we
won't know whether the target is fully stopped, even if we see
diff --git a/gdb/remote.c b/gdb/remote.c
index ecdf834093be..fce99d42bdde 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -682,7 +682,7 @@ class remote_target : public process_stratum_target
const struct btrace_config *btrace_conf (const struct btrace_target_info *) override;
bool augmented_libraries_svr4_read () override;
- void follow_fork (ptid_t, target_waitkind, bool, bool) override;
+ void follow_fork (inferior *, ptid_t, target_waitkind, bool, bool) override;
void follow_exec (inferior *, ptid_t, const char *) override;
int insert_fork_catchpoint (int) override;
int remove_fork_catchpoint (int) override;
@@ -5899,9 +5899,13 @@ extended_remote_target::detach (inferior *inf, int from_tty)
remote target as well. */
void
-remote_target::follow_fork (ptid_t child_ptid, target_waitkind fork_kind,
- bool follow_child, bool detach_fork)
+remote_target::follow_fork (inferior *child_inf, ptid_t child_ptid,
+ target_waitkind fork_kind, bool follow_child,
+ bool detach_fork)
{
+ process_stratum_target::follow_fork (child_inf, child_ptid,
+ fork_kind, follow_child, detach_fork);
+
struct remote_state *rs = get_remote_state ();
if ((fork_kind == TARGET_WAITKIND_FORKED && remote_fork_event_p (rs))
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index fa4cc5bb2dfc..3fd2854955fe 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -56,7 +56,7 @@ struct dummy_target : public target_ops
int remove_fork_catchpoint (int arg0) override;
int insert_vfork_catchpoint (int arg0) override;
int remove_vfork_catchpoint (int arg0) override;
- void follow_fork (ptid_t arg0, target_waitkind arg1, bool arg2, bool arg3) override;
+ void follow_fork (inferior *arg0, ptid_t arg1, target_waitkind arg2, bool arg3, bool arg4) override;
int insert_exec_catchpoint (int arg0) override;
int remove_exec_catchpoint (int arg0) override;
void follow_exec (inferior *arg0, ptid_t arg1, const char *arg2) override;
@@ -231,7 +231,7 @@ struct debug_target : public target_ops
int remove_fork_catchpoint (int arg0) override;
int insert_vfork_catchpoint (int arg0) override;
int remove_vfork_catchpoint (int arg0) override;
- void follow_fork (ptid_t arg0, target_waitkind arg1, bool arg2, bool arg3) override;
+ void follow_fork (inferior *arg0, ptid_t arg1, target_waitkind arg2, bool arg3, bool arg4) override;
int insert_exec_catchpoint (int arg0) override;
int remove_exec_catchpoint (int arg0) override;
void follow_exec (inferior *arg0, ptid_t arg1, const char *arg2) override;
@@ -1519,30 +1519,32 @@ debug_target::remove_vfork_catchpoint (int arg0)
}
void
-target_ops::follow_fork (ptid_t arg0, target_waitkind arg1, bool arg2, bool arg3)
+target_ops::follow_fork (inferior *arg0, ptid_t arg1, target_waitkind arg2, bool arg3, bool arg4)
{
- this->beneath ()->follow_fork (arg0, arg1, arg2, arg3);
+ this->beneath ()->follow_fork (arg0, arg1, arg2, arg3, arg4);
}
void
-dummy_target::follow_fork (ptid_t arg0, target_waitkind arg1, bool arg2, bool arg3)
+dummy_target::follow_fork (inferior *arg0, ptid_t arg1, target_waitkind arg2, bool arg3, bool arg4)
{
- default_follow_fork (this, arg0, arg1, arg2, arg3);
+ default_follow_fork (this, arg0, arg1, arg2, arg3, arg4);
}
void
-debug_target::follow_fork (ptid_t arg0, target_waitkind arg1, bool arg2, bool arg3)
+debug_target::follow_fork (inferior *arg0, ptid_t arg1, target_waitkind arg2, bool arg3, bool arg4)
{
fprintf_unfiltered (gdb_stdlog, "-> %s->follow_fork (...)\n", this->beneath ()->shortname ());
- this->beneath ()->follow_fork (arg0, arg1, arg2, arg3);
+ this->beneath ()->follow_fork (arg0, arg1, arg2, arg3, arg4);
fprintf_unfiltered (gdb_stdlog, "<- %s->follow_fork (", this->beneath ()->shortname ());
- target_debug_print_ptid_t (arg0);
+ target_debug_print_inferior_p (arg0);
fputs_unfiltered (", ", gdb_stdlog);
- target_debug_print_target_waitkind (arg1);
+ target_debug_print_ptid_t (arg1);
fputs_unfiltered (", ", gdb_stdlog);
- target_debug_print_bool (arg2);
+ target_debug_print_target_waitkind (arg2);
fputs_unfiltered (", ", gdb_stdlog);
target_debug_print_bool (arg3);
+ fputs_unfiltered (", ", gdb_stdlog);
+ target_debug_print_bool (arg4);
fputs_unfiltered (")\n", gdb_stdlog);
}
diff --git a/gdb/target.c b/gdb/target.c
index c5468d332ec2..fdbb422fd866 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2700,9 +2700,9 @@ target_program_signals (gdb::array_view<const unsigned char> program_signals)
}
static void
-default_follow_fork (struct target_ops *self, ptid_t child_ptid,
- target_waitkind fork_kind, bool follow_child,
- bool detach_fork)
+default_follow_fork (struct target_ops *self, inferior *child_inf,
+ ptid_t child_ptid, target_waitkind fork_kind,
+ bool follow_child, bool detach_fork)
{
/* Some target returned a fork event, but did not know how to follow it. */
internal_error (__FILE__, __LINE__,
@@ -2712,12 +2712,24 @@ default_follow_fork (struct target_ops *self, ptid_t child_ptid,
/* See target.h. */
void
-target_follow_fork (ptid_t child_ptid, target_waitkind fork_kind,
- bool follow_child, bool detach_fork)
+target_follow_fork (inferior *child_inf, ptid_t child_ptid,
+ target_waitkind fork_kind, bool follow_child,
+ bool detach_fork)
{
target_ops *target = current_inferior ()->top_target ();
- return target->follow_fork (child_ptid, fork_kind, follow_child, detach_fork);
+ /* Check consistency between CHILD_INF, CHILD_PTID, FOLLOW_CHILD and
+ DETACH_FORK. */
+ if (child_inf != nullptr)
+ {
+ gdb_assert (follow_child || !detach_fork);
+ gdb_assert (child_inf->pid == child_ptid.pid ());
+ }
+ else
+ gdb_assert (!follow_child && detach_fork);
+
+ return target->follow_fork (child_inf, child_ptid, fork_kind, follow_child,
+ detach_fork);
}
/* See target.h. */
diff --git a/gdb/target.h b/gdb/target.h
index 3395ac333169..88bbf9e8aefe 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -636,7 +636,7 @@ struct target_ops
TARGET_DEFAULT_RETURN (1);
virtual int remove_vfork_catchpoint (int)
TARGET_DEFAULT_RETURN (1);
- virtual void follow_fork (ptid_t, target_waitkind, bool, bool)
+ virtual void follow_fork (inferior *, ptid_t, target_waitkind, bool, bool)
TARGET_DEFAULT_FUNC (default_follow_fork);
virtual int insert_exec_catchpoint (int)
TARGET_DEFAULT_RETURN (1);
@@ -1719,8 +1719,9 @@ extern int target_remove_vfork_catchpoint (int pid);
bookkeeping and fiddling necessary to continue debugging either the parent,
the child or both. */
-void target_follow_fork (ptid_t child_ptid, target_waitkind fork_kind,
- bool follow_child, bool detach_fork);
+void target_follow_fork (inferior *inf, ptid_t child_ptid,
+ target_waitkind fork_kind, bool follow_child,
+ bool detach_fork);
/* Handle the target-specific bookkeeping required when the inferior makes an
exec call.
--
2.32.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/3] gdb: call post_create_inferior at end of follow_fork_inferior
2021-06-10 14:33 ` [PATCH v3 1/3] gdb: call post_create_inferior at end of follow_fork_inferior Simon Marchi
@ 2021-07-02 17:54 ` Pedro Alves
2021-07-15 3:20 ` Simon Marchi
0 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2021-07-02 17:54 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 2021-06-10 3:33 p.m., Simon Marchi via Gdb-patches wrote:
> GDB doesn't handle well the case of an inferior using the JIT interface
> to register JIT-ed objiles and forking. If an inferior registers a code
typo: objiles -> objfiles
> There isn't currently a way to get visibility into GDB's idea of the JIT
> code objects for each inferior. For the purpose of this test, add the
> "maintenance info jit" command. There isn't much we can print about the
> JIT code objects except their load address. So the output looks a bit
> bare, but it's good enough for the test.
The patch looks good to me. I was just a little surprised with the output
of the "maintenance info jit" command. Specificially, the "-" looked odd to
me, because it looks like either an incomplete range (like start-end), or
a minus/negative sign.
(gdb) maint info jit
Base address of known JIT-ed objfiles:
- 0x5555555596b0
I would just drop the "-". But it's really no biggie and I'm fine with
leaving it.
Pedro Alves
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/3] gdb: pass child_ptid and fork kind to target_ops::follow_fork
2021-06-10 14:33 ` [PATCH v3 2/3] gdb: pass child_ptid and fork kind to target_ops::follow_fork Simon Marchi
@ 2021-07-02 17:55 ` Pedro Alves
2021-07-15 3:22 ` Simon Marchi
0 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2021-07-02 17:55 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 2021-06-10 3:33 p.m., Simon Marchi via Gdb-patches wrote:
> -void target_follow_fork (bool follow_child, bool detach_fork);
> + This function is called when the inferior or vforks, to perform any
when the inferior FORKS or vforks, I guess?
> + bookkeeping and fiddling necessary to continue debugging either the parent,
> + the child or both. */
> +
> +void target_follow_fork (ptid_t child_ptid, target_waitkind fork_kind,
> + bool follow_child, bool detach_fork);
>
Otherwise LGTM.
Pedro Alves
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 3/3] gdb: follow-fork: push target and add thread in target_follow_fork
2021-06-10 14:33 ` [PATCH v3 3/3] gdb: follow-fork: push target and add thread in target_follow_fork Simon Marchi
@ 2021-07-02 17:55 ` Pedro Alves
2021-07-15 3:44 ` Simon Marchi
0 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2021-07-02 17:55 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
Hi!
On 2021-06-10 3:33 p.m., Simon Marchi via Gdb-patches wrote:
> In the context of ROCm-gdb [1], the ROCm target sits on top of the
> linux-nat target. when a process forks, it needs to carry over some
> data from the forking inferior to the fork child inferior. Ideally, the
> ROCm target would implement the follow_fork target_ops method, but there
> are some small problems. This patch fixes these, which helps the ROCm
> target, but also makes things more consistent and a bit nicer in
> general, I believe.
>
> The main problem is: when follow-fork-mode is "parent",
> target_follow_fork is called with the parent as the current inferior.
> When it's "child", target_follow_fork is called with the child as the
> current inferior. This means that target_follow_fork is sometimes
> called on the parent's target stack and sometimes on the child's target
> stack.
>
> The parent's target stack may contain targets above the process target,
> such as the ROCm target. So if follow-fork-child is "parent", the ROCm
> target would get notified of the fork and do whatever is needed. But
> the child's target stack, at that moment, only contains the exec and
> process target copied over from the parent. The child's target stack is
> set up by follow_fork_inferior, before calling target_follow_fork. In
> that case, the ROCm target wouldn't get notified of the fork.
>
> For consistency, I think it would be good to always call
> target_follow_fork on the parent inferior's target stack. I think it
> makes sense as a way to indicate "this inferior has called fork, do
> whatever is needed". The desired outcome of the fork (whether an
> inferior is created for the child, do we need to detach from the child)
> can be indicated by passed parameter.
>
> I therefore propose these changes:
>
> - make follow_fork_inferior always call target_follow_fork with the
> parent as the current inferior. That lets all targets present on the
> parent's target stack do some fork-related handling and push
> themselves on the fork child's target stack if needed.
>
> For this purpose, pass the child inferior down to target_follow_fork
> and follow_fork implementations. This is nullptr if no inferior is
> created for the child, because we want to detach from it.
>
> - as a result, in follow_fork_inferior, detach from the parent inferior
> (if needed) only after the target_follow_fork call. This is needed
> because we want to call target_follow_fork before the parent's
> starget stack is torn down.
typo "starget" -> "target".
This is a nicer design for sure, though there's some historic baggage that explains
why we did what we did, and we may have a problem here. In current master,
when following a fork, while detaching the parent, we detach the parent first,
and then follow the child. This order was important before GDB gained support
for debugging more than one process at a time. It used to be the case
that some ports only supported follow-fork, but not "set detach-on-fork off".
"set detach-on-fork" was actually originally a linux-only command in linux-fork.c,
and holding on to the undetached parent or child was implemented back then by using
the fork save/restore machinery still in linux-fork.c nowadays for checkpoints.
This explains why we ended up with a "follow_fork" target method design -- everything
about following the fork was hidden behind that method, and core gdb
was not aware of multiple inferiors yet.
Now, I am not sure nowadays all targets that support following forks
handle debugging both parent and child at the same time and then detaching
one of them correctly. This is what we need to look out for.
Fortunately, we have muuuuuuch fewer targets than back in the day,
and only a few support following forks:
linux-nat.c, obsd-nat.c, remote.c, fbsd-nat.c
- linux-nat.c and remote.c surely handle this fine.
- fbsd-nat.c probably does too, since it explicitly handles detach_fork.
- obsd-nat.c does not handle "set detach-on-fork off" though. I think
this one deserves a double check.
> - target_follow_fork (child_ptid, fork_kind, follow_child, detach_fork);
> + gdb_assert (current_inferior () == parent_inf);
> +
> + /* target_follow_fork is responsible for pushing the appropriate targets on
> + the new inferior's target stack and adding the initial thread (with ptid
> + CHILD_PTID). */
And detaching the child if following the parent and detach_fork is on.
> + target_follow_fork (child_inf, child_ptid, fork_kind, follow_child,
> + detach_fork);
> +
Otherwise LGTM.
Pedro Alves
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/3] gdb: call post_create_inferior at end of follow_fork_inferior
2021-07-02 17:54 ` Pedro Alves
@ 2021-07-15 3:20 ` Simon Marchi
0 siblings, 0 replies; 12+ messages in thread
From: Simon Marchi @ 2021-07-15 3:20 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 2021-07-02 1:54 p.m., Pedro Alves wrote:
> On 2021-06-10 3:33 p.m., Simon Marchi via Gdb-patches wrote:
>> GDB doesn't handle well the case of an inferior using the JIT interface
>> to register JIT-ed objiles and forking. If an inferior registers a code
>
> typo: objiles -> objfiles
>
>> There isn't currently a way to get visibility into GDB's idea of the JIT
>> code objects for each inferior. For the purpose of this test, add the
>> "maintenance info jit" command. There isn't much we can print about the
>> JIT code objects except their load address. So the output looks a bit
>> bare, but it's good enough for the test.
>
> The patch looks good to me. I was just a little surprised with the output
> of the "maintenance info jit" command. Specificially, the "-" looked odd to
> me, because it looks like either an incomplete range (like start-end), or
> a minus/negative sign.
>
> (gdb) maint info jit
> Base address of known JIT-ed objfiles:
> - 0x5555555596b0
>
> I would just drop the "-". But it's really no biggie and I'm fine with
> leaving it.
>
> Pedro Alves
>
I fixed both and pushed this patch, thanks.
Simon
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/3] gdb: pass child_ptid and fork kind to target_ops::follow_fork
2021-07-02 17:55 ` Pedro Alves
@ 2021-07-15 3:22 ` Simon Marchi
0 siblings, 0 replies; 12+ messages in thread
From: Simon Marchi @ 2021-07-15 3:22 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 2021-07-02 1:55 p.m., Pedro Alves wrote:
> On 2021-06-10 3:33 p.m., Simon Marchi via Gdb-patches wrote:
>> -void target_follow_fork (bool follow_child, bool detach_fork);
>> + This function is called when the inferior or vforks, to perform any
>
> when the inferior FORKS or vforks, I guess?
Oh, right. Fixed and pushed this patch, thanks.
Simon
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 3/3] gdb: follow-fork: push target and add thread in target_follow_fork
2021-07-02 17:55 ` Pedro Alves
@ 2021-07-15 3:44 ` Simon Marchi
2021-07-15 19:31 ` Simon Marchi
0 siblings, 1 reply; 12+ messages in thread
From: Simon Marchi @ 2021-07-15 3:44 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 2021-07-02 1:55 p.m., Pedro Alves wrote:
> Hi!
>
> On 2021-06-10 3:33 p.m., Simon Marchi via Gdb-patches wrote:
>> In the context of ROCm-gdb [1], the ROCm target sits on top of the
>> linux-nat target. when a process forks, it needs to carry over some
>> data from the forking inferior to the fork child inferior. Ideally, the
>> ROCm target would implement the follow_fork target_ops method, but there
>> are some small problems. This patch fixes these, which helps the ROCm
>> target, but also makes things more consistent and a bit nicer in
>> general, I believe.
>>
>> The main problem is: when follow-fork-mode is "parent",
>> target_follow_fork is called with the parent as the current inferior.
>> When it's "child", target_follow_fork is called with the child as the
>> current inferior. This means that target_follow_fork is sometimes
>> called on the parent's target stack and sometimes on the child's target
>> stack.
>>
>> The parent's target stack may contain targets above the process target,
>> such as the ROCm target. So if follow-fork-child is "parent", the ROCm
>> target would get notified of the fork and do whatever is needed. But
>> the child's target stack, at that moment, only contains the exec and
>> process target copied over from the parent. The child's target stack is
>> set up by follow_fork_inferior, before calling target_follow_fork. In
>> that case, the ROCm target wouldn't get notified of the fork.
>>
>> For consistency, I think it would be good to always call
>> target_follow_fork on the parent inferior's target stack. I think it
>> makes sense as a way to indicate "this inferior has called fork, do
>> whatever is needed". The desired outcome of the fork (whether an
>> inferior is created for the child, do we need to detach from the child)
>> can be indicated by passed parameter.
>>
>> I therefore propose these changes:
>>
>> - make follow_fork_inferior always call target_follow_fork with the
>> parent as the current inferior. That lets all targets present on the
>> parent's target stack do some fork-related handling and push
>> themselves on the fork child's target stack if needed.
>>
>> For this purpose, pass the child inferior down to target_follow_fork
>> and follow_fork implementations. This is nullptr if no inferior is
>> created for the child, because we want to detach from it.
>>
>> - as a result, in follow_fork_inferior, detach from the parent inferior
>> (if needed) only after the target_follow_fork call. This is needed
>> because we want to call target_follow_fork before the parent's
>> starget stack is torn down.
>
> typo "starget" -> "target".
>
> This is a nicer design for sure, though there's some historic baggage that explains
> why we did what we did, and we may have a problem here. In current master,
> when following a fork, while detaching the parent, we detach the parent first,
> and then follow the child. This order was important before GDB gained support
> for debugging more than one process at a time. It used to be the case
> that some ports only supported follow-fork, but not "set detach-on-fork off".
> "set detach-on-fork" was actually originally a linux-only command in linux-fork.c,
> and holding on to the undetached parent or child was implemented back then by using
> the fork save/restore machinery still in linux-fork.c nowadays for checkpoints.
> This explains why we ended up with a "follow_fork" target method design -- everything
> about following the fork was hidden behind that method, and core gdb
> was not aware of multiple inferiors yet.
>
> Now, I am not sure nowadays all targets that support following forks
> handle debugging both parent and child at the same time and then detaching
> one of them correctly. This is what we need to look out for.
>
> Fortunately, we have muuuuuuch fewer targets than back in the day,
> and only a few support following forks:
>
> linux-nat.c, obsd-nat.c, remote.c, fbsd-nat.c
>
> - linux-nat.c and remote.c surely handle this fine.
>
> - fbsd-nat.c probably does too, since it explicitly handles detach_fork.
>
> - obsd-nat.c does not handle "set detach-on-fork off" though. I think
> this one deserves a double check.
Ok, thanks for the heads up. What could make OpenBSD not support it, if
a process on that OS wasn't capable of ptrace-ing two processes at the
same time? Or it would be something in our code?
There is an OpenBSD machine on the compile farm, I'll give it a try.
>> - target_follow_fork (child_ptid, fork_kind, follow_child, detach_fork);
>> + gdb_assert (current_inferior () == parent_inf);
>> +
>> + /* target_follow_fork is responsible for pushing the appropriate targets on
>> + the new inferior's target stack and adding the initial thread (with ptid
>> + CHILD_PTID). */
>
> And detaching the child if following the parent and detach_fork is on.
Here's what I ended up writing:
/* If we are setting up an inferior for the child, target_follow_fork is
responsible for pushing the appropriate targets on the new inferior's
target stack and adding the initial thread (with ptid CHILD_PTID).
If we are not setting up an inferior for the child (because following
the parent and detach_fork is true), it is responsible for detaching
from CHILD_PTID. */
Simon
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 3/3] gdb: follow-fork: push target and add thread in target_follow_fork
2021-07-15 3:44 ` Simon Marchi
@ 2021-07-15 19:31 ` Simon Marchi
2021-08-04 0:44 ` Simon Marchi
0 siblings, 1 reply; 12+ messages in thread
From: Simon Marchi @ 2021-07-15 19:31 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
>> Now, I am not sure nowadays all targets that support following forks
>> handle debugging both parent and child at the same time and then detaching
>> one of them correctly. This is what we need to look out for.
>>
>> Fortunately, we have muuuuuuch fewer targets than back in the day,
>> and only a few support following forks:
>>
>> linux-nat.c, obsd-nat.c, remote.c, fbsd-nat.c
>>
>> - linux-nat.c and remote.c surely handle this fine.
>>
>> - fbsd-nat.c probably does too, since it explicitly handles detach_fork.
>>
>> - obsd-nat.c does not handle "set detach-on-fork off" though. I think
>> this one deserves a double check.
>
> Ok, thanks for the heads up. What could make OpenBSD not support it, if
> a process on that OS wasn't capable of ptrace-ing two processes at the
> same time? Or it would be something in our code?
>
> There is an OpenBSD machine on the compile farm, I'll give it a try.
I built GDB on the OpenBSD machine, but it fails to start any program. I
tracked it down to x86_dr_low.get_status being nullptr here:
* thread #1, stop reason = step over
frame #0: 0x00000a116f00255d gdb`x86_dr_stopped_data_address(state=0x00000a13e5bdd790, addr_p=0x00007f7ffffe3ff8) at x86-dregs.c:610:12
607 registers. */
608 printf("1\n");
609 printf(" %p\n", x86_dr_low.get_status);
-> 610 status = x86_dr_low_get_status ();
^
611
612 printf("2\n");
613 ALL_DEBUG_ADDRESS_REGISTERS (i)
(lldb) print x86_dr_low.get_status
(unsigned long (*)()) $0 = 0x0000000000000000
(lldb) bt
* thread #1, stop reason = step over
* frame #0: 0x00000a116f00255d gdb`x86_dr_stopped_data_address(state=0x00000a13e5bdd790, addr_p=0x00007f7ffffe3ff8) at x86-dregs.c:610:12
frame #1: 0x00000a116f00274e gdb`x86_dr_stopped_by_watchpoint(state=0x00000a13e5bdd790) at x86-dregs.c:661:10
frame #2: 0x00000a116f331f99 gdb`x86_stopped_by_watchpoint() at x86-nat.c:209:10
frame #3: 0x00000a116eac3f4f gdb`x86_nat_target<obsd_nat_target>::stopped_by_watchpoint(this=0x00000a116f6de860) at x86-nat.h:102:11
frame #4: 0x00000a116f1cbefc gdb`target_stopped_by_watchpoint() at target.c:473:50
frame #5: 0x00000a116eb26fa5 gdb`watchpoints_triggered(ws=0x00007f7ffffe4bc8) at breakpoint.c:4791:32
frame #6: 0x00000a116ef17bdd gdb`handle_signal_stop(ecs=0x00007f7ffffe4ba0) at infrun.c:6084:29
frame #7: 0x00000a116ef0a357 gdb`handle_inferior_event(ecs=0x00007f7ffffe4ba0) at infrun.c:5694:7
frame #8: 0x00000a116ef08150 gdb`fetch_inferior_event() at infrun.c:4090:5
frame #9: 0x00000a116eedd8d1 gdb`inferior_event_handler(event_type=INF_REG_EVENT) at inf-loop.c:41:7
frame #10: 0x00000a116ef0e779 gdb`infrun_async_inferior_event_handler(data=0x0000000000000000) at infrun.c:9402:3
frame #11: 0x00000a116eae7cdf gdb`check_async_event_handlers() at async-event.c:335:4
frame #12: 0x00000a116f4fc947 gdb`gdb_do_one_event() at event-loop.cc:216:10
frame #13: 0x00000a116ef7ce51 gdb`start_event_loop() at main.c:421:13
frame #14: 0x00000a116ef7bf3a gdb`captured_command_loop() at main.c:481:3
frame #15: 0x00000a116ef79662 gdb`captured_main(data=0x00007f7ffffe4e70) at main.c:1353:4
frame #16: 0x00000a116ef79592 gdb`gdb_main(args=0x00007f7ffffe4e70) at main.c:1368:7
frame #17: 0x00000a116ea29707 gdb`main(argc=5, argv=0x00007f7ffffe4f18) at gdb.c:32:10
frame #18: 0x00000a116ea294a1 gdb`___start + 321
Indeed, _initialize_x86_bsd_nat sets it only when HAVE_PT_GETDBREGS is
defined, which it isn't on OpenBSD. I made the quick fix pasted below
(which I think would be a good fix to have in master), which allowed it
to go further, and hit:
(gdb) r
Starting program: /home/simark/build/binutils-gdb/gdb/a.out
Child process unexpectedly missing: No child processes.
/home/simark/src/binutils-gdb/gdb/inferior.c:303: internal-error: struct inferior *find_inferior_pid(process_stratum_target *, int): Assertion `pid != 0' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) Process 69075 stopped
* thread #1, stop reason = signal SIGSTOP
frame #0: 0x000004c42ebf2e5a libc.so.96.0`_thread_sys_poll at -:3
(lldb) bt
* thread #1, stop reason = signal SIGSTOP
* frame #0: 0x000004c42ebf2e5a libc.so.96.0`_thread_sys_poll at -:3
frame #1: 0x000004c42ec6867e libc.so.96.0`_libc_poll_cancel(fds=<unavailable>, nfds=<unavailable>, timeout=<unavailable>) at w_poll.c:27:8
frame #2: 0x000004c165907bb5 gdb`gdb_wait_for_event(block=1) at event-loop.cc:613:19
frame #3: 0x000004c1659079f8 gdb`gdb_do_one_event() at event-loop.cc:237:7
frame #4: 0x000004c165616565 gdb`gdb_readline_wrapper(prompt="/home/simark/src/binutils-gdb/gdb/inferior.c:303: internal-error: struct inferior *find_inferior_pid(process_stratum_target *, int): Assertion `pid != 0' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugging session? (y or n) ") at top.c:1122:10
frame #5: 0x000004c1656efb35 gdb`defaulted_query(ctlstr="%s\nQuit this debugging session? ", defchar='\0', args=0x00007f7ffffe06e0) at utils.c:889:18
frame #6: 0x000004c1656f007b gdb`query(ctlstr="%s\nQuit this debugging session? ") at utils.c:981:9
frame #7: 0x000004c1656ee7b5 gdb`internal_vproblem(problem=0x000004c165aed310, file="/home/simark/src/binutils-gdb/gdb/inferior.c", line=303, fmt="%s: Assertion `%s' failed.", ap=0x00007f7ffffe0a00) at utils.c:373:11
frame #8: 0x000004c1656ee402 gdb`internal_verror(file="/home/simark/src/binutils-gdb/gdb/inferior.c", line=303, fmt="%s: Assertion `%s' failed.", ap=0x00007f7ffffe0a00) at utils.c:439:3
frame #9: 0x000004c165907740 gdb`internal_error(file="/home/simark/src/binutils-gdb/gdb/inferior.c", line=303, fmt="%s: Assertion `%s' failed.") at errors.cc:55:3
frame #10: 0x000004c165301be3 gdb`find_inferior_pid(targ=0x000004c165ae98a0, pid=0) at inferior.c:303:3
frame #11: 0x000004c165301cd8 gdb`find_inferior_ptid(targ=0x000004c165ae98a0, ptid=(m_pid = 0, m_lwp = 0, m_tid = 0)) at inferior.c:317:10
frame #12: 0x000004c165605e43 gdb`find_thread_ptid(targ=0x000004c165ae98a0, ptid=(m_pid = 0, m_lwp = 0, m_tid = 0)) at thread.c:487:19
frame #13: 0x000004c165314556 gdb`handle_inferior_event(ecs=0x00007f7ffffe12c0) at infrun.c:5380:21
frame #14: 0x000004c1653131a0 gdb`fetch_inferior_event() at infrun.c:4090:5
frame #15: 0x000004c1652e8921 gdb`inferior_event_handler(event_type=INF_REG_EVENT) at inf-loop.c:41:7
obsd_nat_target::wait hits the error case (and prints the "Child process
unexpectedly missing" message) and returns the value of inferior_ptid,
which is null_ptid.
So, there seems to be a bit too much bit-rot for me to actually test
anything.
Simon
From e4314c31f9e804e4803d36286f6785e8d120cfc3 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Thu, 15 Jul 2021 15:12:50 -0400
Subject: [PATCH] fix
Change-Id: Ibbf6ec02469c80c1a7ab34e71d503779c9ae840c
---
gdb/nat/x86-dregs.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/gdb/nat/x86-dregs.c b/gdb/nat/x86-dregs.c
index cf8e517eb0..b61a551f81 100644
--- a/gdb/nat/x86-dregs.c
+++ b/gdb/nat/x86-dregs.c
@@ -82,6 +82,12 @@ x86_dr_low_get_control ()
return x86_dr_low.get_control ();
}
+static bool
+x86_dr_low_can_get_status ()
+{
+ return x86_dr_low.get_status != nullptr;
+}
+
/* Return the value of the inferior's DR6 debug status register. */
static unsigned long
@@ -604,6 +610,9 @@ int
x86_dr_stopped_data_address (struct x86_debug_reg_state *state,
CORE_ADDR *addr_p)
{
+ if (!x86_dr_low_can_get_status ())
+ return 0;
+
CORE_ADDR addr = 0;
int i;
int rc = 0;
@@ -693,6 +702,9 @@ x86_dr_stopped_by_watchpoint (struct x86_debug_reg_state *state)
int
x86_dr_stopped_by_hw_breakpoint (struct x86_debug_reg_state *state)
{
+ if (!x86_dr_low_can_get_status ())
+ return 0;
+
CORE_ADDR addr = 0;
int i;
int rc = 0;
--
2.26.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 3/3] gdb: follow-fork: push target and add thread in target_follow_fork
2021-07-15 19:31 ` Simon Marchi
@ 2021-08-04 0:44 ` Simon Marchi
0 siblings, 0 replies; 12+ messages in thread
From: Simon Marchi @ 2021-08-04 0:44 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
> I built GDB on the OpenBSD machine, but it fails to start any program. I
> tracked it down to x86_dr_low.get_status being nullptr here:
>
> * thread #1, stop reason = step over
> frame #0: 0x00000a116f00255d gdb`x86_dr_stopped_data_address(state=0x00000a13e5bdd790, addr_p=0x00007f7ffffe3ff8) at x86-dregs.c:610:12
> 607 registers. */
> 608 printf("1\n");
> 609 printf(" %p\n", x86_dr_low.get_status);
> -> 610 status = x86_dr_low_get_status ();
> ^
> 611
> 612 printf("2\n");
> 613 ALL_DEBUG_ADDRESS_REGISTERS (i)
> (lldb) print x86_dr_low.get_status
> (unsigned long (*)()) $0 = 0x0000000000000000
> (lldb) bt
> * thread #1, stop reason = step over
> * frame #0: 0x00000a116f00255d gdb`x86_dr_stopped_data_address(state=0x00000a13e5bdd790, addr_p=0x00007f7ffffe3ff8) at x86-dregs.c:610:12
> frame #1: 0x00000a116f00274e gdb`x86_dr_stopped_by_watchpoint(state=0x00000a13e5bdd790) at x86-dregs.c:661:10
> frame #2: 0x00000a116f331f99 gdb`x86_stopped_by_watchpoint() at x86-nat.c:209:10
> frame #3: 0x00000a116eac3f4f gdb`x86_nat_target<obsd_nat_target>::stopped_by_watchpoint(this=0x00000a116f6de860) at x86-nat.h:102:11
> frame #4: 0x00000a116f1cbefc gdb`target_stopped_by_watchpoint() at target.c:473:50
> frame #5: 0x00000a116eb26fa5 gdb`watchpoints_triggered(ws=0x00007f7ffffe4bc8) at breakpoint.c:4791:32
> frame #6: 0x00000a116ef17bdd gdb`handle_signal_stop(ecs=0x00007f7ffffe4ba0) at infrun.c:6084:29
> frame #7: 0x00000a116ef0a357 gdb`handle_inferior_event(ecs=0x00007f7ffffe4ba0) at infrun.c:5694:7
> frame #8: 0x00000a116ef08150 gdb`fetch_inferior_event() at infrun.c:4090:5
> frame #9: 0x00000a116eedd8d1 gdb`inferior_event_handler(event_type=INF_REG_EVENT) at inf-loop.c:41:7
> frame #10: 0x00000a116ef0e779 gdb`infrun_async_inferior_event_handler(data=0x0000000000000000) at infrun.c:9402:3
> frame #11: 0x00000a116eae7cdf gdb`check_async_event_handlers() at async-event.c:335:4
> frame #12: 0x00000a116f4fc947 gdb`gdb_do_one_event() at event-loop.cc:216:10
> frame #13: 0x00000a116ef7ce51 gdb`start_event_loop() at main.c:421:13
> frame #14: 0x00000a116ef7bf3a gdb`captured_command_loop() at main.c:481:3
> frame #15: 0x00000a116ef79662 gdb`captured_main(data=0x00007f7ffffe4e70) at main.c:1353:4
> frame #16: 0x00000a116ef79592 gdb`gdb_main(args=0x00007f7ffffe4e70) at main.c:1368:7
> frame #17: 0x00000a116ea29707 gdb`main(argc=5, argv=0x00007f7ffffe4f18) at gdb.c:32:10
> frame #18: 0x00000a116ea294a1 gdb`___start + 321
>
> Indeed, _initialize_x86_bsd_nat sets it only when HAVE_PT_GETDBREGS is
> defined, which it isn't on OpenBSD. I made the quick fix pasted below
> (which I think would be a good fix to have in master), which allowed it
> to go further, and hit:
>
> (gdb) r
> Starting program: /home/simark/build/binutils-gdb/gdb/a.out
> Child process unexpectedly missing: No child processes.
> /home/simark/src/binutils-gdb/gdb/inferior.c:303: internal-error: struct inferior *find_inferior_pid(process_stratum_target *, int): Assertion `pid != 0' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n) Process 69075 stopped
> * thread #1, stop reason = signal SIGSTOP
> frame #0: 0x000004c42ebf2e5a libc.so.96.0`_thread_sys_poll at -:3
> (lldb) bt
> * thread #1, stop reason = signal SIGSTOP
> * frame #0: 0x000004c42ebf2e5a libc.so.96.0`_thread_sys_poll at -:3
> frame #1: 0x000004c42ec6867e libc.so.96.0`_libc_poll_cancel(fds=<unavailable>, nfds=<unavailable>, timeout=<unavailable>) at w_poll.c:27:8
> frame #2: 0x000004c165907bb5 gdb`gdb_wait_for_event(block=1) at event-loop.cc:613:19
> frame #3: 0x000004c1659079f8 gdb`gdb_do_one_event() at event-loop.cc:237:7
> frame #4: 0x000004c165616565 gdb`gdb_readline_wrapper(prompt="/home/simark/src/binutils-gdb/gdb/inferior.c:303: internal-error: struct inferior *find_inferior_pid(process_stratum_target *, int): Assertion `pid != 0' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugging session? (y or n) ") at top.c:1122:10
> frame #5: 0x000004c1656efb35 gdb`defaulted_query(ctlstr="%s\nQuit this debugging session? ", defchar='\0', args=0x00007f7ffffe06e0) at utils.c:889:18
> frame #6: 0x000004c1656f007b gdb`query(ctlstr="%s\nQuit this debugging session? ") at utils.c:981:9
> frame #7: 0x000004c1656ee7b5 gdb`internal_vproblem(problem=0x000004c165aed310, file="/home/simark/src/binutils-gdb/gdb/inferior.c", line=303, fmt="%s: Assertion `%s' failed.", ap=0x00007f7ffffe0a00) at utils.c:373:11
> frame #8: 0x000004c1656ee402 gdb`internal_verror(file="/home/simark/src/binutils-gdb/gdb/inferior.c", line=303, fmt="%s: Assertion `%s' failed.", ap=0x00007f7ffffe0a00) at utils.c:439:3
> frame #9: 0x000004c165907740 gdb`internal_error(file="/home/simark/src/binutils-gdb/gdb/inferior.c", line=303, fmt="%s: Assertion `%s' failed.") at errors.cc:55:3
> frame #10: 0x000004c165301be3 gdb`find_inferior_pid(targ=0x000004c165ae98a0, pid=0) at inferior.c:303:3
> frame #11: 0x000004c165301cd8 gdb`find_inferior_ptid(targ=0x000004c165ae98a0, ptid=(m_pid = 0, m_lwp = 0, m_tid = 0)) at inferior.c:317:10
> frame #12: 0x000004c165605e43 gdb`find_thread_ptid(targ=0x000004c165ae98a0, ptid=(m_pid = 0, m_lwp = 0, m_tid = 0)) at thread.c:487:19
> frame #13: 0x000004c165314556 gdb`handle_inferior_event(ecs=0x00007f7ffffe12c0) at infrun.c:5380:21
> frame #14: 0x000004c1653131a0 gdb`fetch_inferior_event() at infrun.c:4090:5
> frame #15: 0x000004c1652e8921 gdb`inferior_event_handler(event_type=INF_REG_EVENT) at inf-loop.c:41:7
>
> obsd_nat_target::wait hits the error case (and prints the "Child process
> unexpectedly missing" message) and returns the value of inferior_ptid,
> which is null_ptid.
>
> So, there seems to be a bit too much bit-rot for me to actually test
> anything.
Thanks to the patches merged by John to improve the state on OpenBSD, I
was able to confirm that my patch does not break following fork (not
that it works again) on OpenBSD. I tested all four combinations of
follow-fork-mode parent/child, detach-on-fork on/off.
I'll push the patch shortly.
Simon
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-08-04 0:44 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-10 14:33 [PATCH v3 0/3] Follow fork improvements Simon Marchi
2021-06-10 14:33 ` [PATCH v3 1/3] gdb: call post_create_inferior at end of follow_fork_inferior Simon Marchi
2021-07-02 17:54 ` Pedro Alves
2021-07-15 3:20 ` Simon Marchi
2021-06-10 14:33 ` [PATCH v3 2/3] gdb: pass child_ptid and fork kind to target_ops::follow_fork Simon Marchi
2021-07-02 17:55 ` Pedro Alves
2021-07-15 3:22 ` Simon Marchi
2021-06-10 14:33 ` [PATCH v3 3/3] gdb: follow-fork: push target and add thread in target_follow_fork Simon Marchi
2021-07-02 17:55 ` Pedro Alves
2021-07-15 3:44 ` Simon Marchi
2021-07-15 19:31 ` Simon Marchi
2021-08-04 0:44 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).