]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commit
gdb: some process_stratum_target should not be shared
authorAndrew Burgess <aburgess@redhat.com>
Thu, 29 Sep 2022 11:45:26 +0000 (12:45 +0100)
committerAndrew Burgess <aburgess@redhat.com>
Tue, 30 Sep 2025 13:18:31 +0000 (14:18 +0100)
commit1b28027e89b199af795fd92c34b624e980171cea
treed5960f78076e853828d0e91fd18ea859fcd6ff4f
parentd69c70ce8990fa66020e3d7d568a1dce9caff43b
gdb: some process_stratum_target should not be shared

When multi-target support was added to GDB, an assumption was made
that all process_stratum_target sub-classes could be shared by
multiple inferiors.

For things like the Linux and FreeBSD native targets, this is
absolutely true (or was made true).  But some targets were either not
updated, or, due to design restrictions, cannot be shared.

This patch adds a target_ops::is_shareable member function.  When this
returns false then this indicates that an instance of a particular
target should only appear on a single target stack.  It is fine for
difference instances of a single target type to appear on different
target stacks though.

This is my second attempt at this patch.  The first attempt can be
found here:

  https://inbox.sourceware.org/gdb-patches/577f2c47793acb501c2611c0e6c7ea379f774830.1668789658.git.aburgess@redhat.com

The initial approach was to have all targets be shareable by default,
and to then mark those targets which I knew were problematic.
Specifically, the only target I was interested in was core_target,
which cannot be shared (details below).  During review Tom pointed out:

  I think there are a lot of other targets that can't be
  shared... remote-sim, all the trace targets, even I think windows-nat,
  since it isn't multi-inferior-capable yet.

The suggestion was that the default should be that targets were NOT
shareable, and we should then mark those targets which we know can be
shared.  That is the big change in this iteration of the patch.

The core_target is still non-shareable.  This target stores state
relating to the open core file in the core_target and in the current
inferior's program_space.  After an 'add-inferior' command, if we
share the core_target, the new inferior will have its own
program_space, but will share the core_target with the original
inferior.  This leaves the new inferior in an unexpected state where
the core BFD (from the new program_space) is NULL.  Attempting to make
use of the second inferior, e.g. to load a new executable, will (on
x86 at least) cause GDB to crash as it is not expecting the core BFD
to be NULL.

I am working to move the core file BFD into core_target, at which
point it might be possible to share the core_target, though I'm still
not entirely sure this makes sense; loading a core file will in many
cases, automatically set the executable in the program_space, creating
a new inferior would share the core_target, but the new inferior's
program space would not have the executable loaded.  But I figure we
can worry about this another day because ....

As Tom pointed out in his review of V1, there are other targets that
should be non-shareable (see quote above).  In addition, I believe
that the remote target can only be shared in some specific situations,
the 'add-inferior' case is one where the 'remote' target should NOT be
shared.

The 'remote' (not 'extended-remote') target doesn't allow new
inferior's to be started, you need to connect to an already running
target.  As such, it doesn't really make sense to allow a 'remote'
target to be shared over an 'add-inferior' call, what would the user
do with the new inferior?  They cannot start a new process.  They're
not debugging the same process as the original inferior.  This just
leaves GDB in a weird state.

However, 'remote' targets are a little weird in that, if the remote
inferior forks, and GDB is set to follow both the parent and the
child, then it does make sense to allow sharing.  In this case the new
inferior is automatically connected to the already running child
process.

So when we consider 'add-inferior' there are two things we need to
consider:

  1. Can the target be shared at all?  The new target_ops::is_shareable
     function tells us this.

  2. Can the target be used to start a new inferior?  The existing
     target_ops::can_create_inferior function tells us this.

If the process_stratum_target returns true for both of these functions
then it is OK to share it across an 'add-inferior' call.  If either
returns false then the target should not be shared.

When pushing a target onto an inferior's target stack, we only need to
consider target_ops::is_shareable, only shareable targets should be
pushed onto multiple target stacks.

The new target_ops::is_shareable function returns true as its default,
all the immediate sub-classes are shareable.

However, this is overridden in process_stratum_target::is_shareable, to
return false.  All process_stratum_target sub-classes are non-shareable
by default.

Finally, linux_nat_target, fbsd_nat_target, and remote_target, are all
marked as shareable.  This leaves all other process_stratum_target
sub-classes non-shareable.

I did some very light testing on Windows, and I don't believe that this
target supports multiple inferiors, but I could easily be wrong here.
My windows testing setup is really iffy, and I'm not 100% sure if I did
this right.

But for the Windows target, and any of the others, if this commit breaks
existing multi-inferior support, then the fix is as simple as adding an
is_shareable member function that returns true.

If the user tries to 'add-inferior' from an inferior with a
non-shareable target, or the 'remote' target as it cannot start new
inferiors, then they will get a warning, and the new inferior will be
created without a connection.

If the user explicitly asks for the new inferior to be created without
a connection, then no warning will be given.

At this point the user is free to setup the new inferior connection as
they see fit.

I've updated the docs, and added a NEWS entry for the new warning.  In
the docs for clone-inferior I've added reference to -no-connection,
which was previously missing.

Some tests needed fixing with this change, these were
gdb.base/quit-live.exp, gdb.mi/mi-add-inferior.exp,
gdb.mi/new-ui-mi-sync.exp, and gdb.python/py-connection-removed.exp.  In
all cases, when using the native-gdbserver board, these tests tried to
create a new inferior, and expected the new inferior to start sharing
the connection with the original inferior.  None of these tests actually
tried to run anything in the new inferior, if they did then they would
have discovered that the new inferior wasn't really sharing a
connection.  All the tests have been updated to understand that for
'remote' connections (not 'extended-remote') the connection will not be
shared.  These fixes are all pretty simple.

Approved-By: Tom Tromey <tom@tromey.com>
19 files changed:
gdb/NEWS
gdb/doc/gdb.texinfo
gdb/fbsd-nat.h
gdb/inferior.c
gdb/inferior.h
gdb/linux-nat.h
gdb/process-stratum-target.h
gdb/remote.c
gdb/target.c
gdb/target.h
gdb/testsuite/gdb.base/quit-live.exp
gdb/testsuite/gdb.mi/mi-add-inferior.exp
gdb/testsuite/gdb.mi/new-ui-mi-sync.exp
gdb/testsuite/gdb.multi/multi-core-files-1.c [new file with mode: 0644]
gdb/testsuite/gdb.multi/multi-core-files-2.c [new file with mode: 0644]
gdb/testsuite/gdb.multi/multi-core-files.exp [new file with mode: 0644]
gdb/testsuite/gdb.multi/multi-remote-target.c [new file with mode: 0644]
gdb/testsuite/gdb.multi/multi-remote-target.exp [new file with mode: 0644]
gdb/testsuite/gdb.python/py-connection-removed.exp