Re: [PATCH 00/23] Multi-target support

Philippe Waroquiers
2019-09-07T11:19:58Z

Patch is a nice target :).
This can be useful e.g. with the valgrind gdbserver, that only
supports to debug a single process (i.e. the valgrind process that
runs its embedded gdbserver).

First, a minor suggestion about the terminology (as introduced in patch 17, and used
in command names): the rational is that 'target' being already overloaded,
the wording 'target connection' is used (leading to e.g. the command
'info connections').
I am wondering if the word "connection" is not also too overloaded,
and too much interpreted as meaning 'a real connection', which might lead to
some user confusion.
Maybe other wording could be used instead (such as 'target channel'
and 'info channels') ? Or maybe another synonym of channel or similar ?

Otherwise, I did a minimal test to see how GDB could connect to 2 different
valgrind gdbservers (a 64 bits and a 32 bits), but I could not make it work.
The 2 valgrind I have launched are (in the top of a valgrind build):

./path to/bin/valgrind --vgdb-error=0 ./memcheck/tests/trivialleak
./path to/bin/valgrind --vgdb-error=0 ./memcheck/tests/x86-linux/scalar_exit_group

(gdb) tar rem|lvgdb
Remote debugging using |lvgdb
...
(gdb) add-inferior -no-connection
[New inferior 2]
Added inferior 2
(gdb) tar rem |lvgdb --pid=16727
Remote debugging using |lvgdb --pid=16727
...
(gdb) info connection
Num Name Description
1 remote lvgdb Remote serial target in gdb-specific protocol
* 2 remote lvgdb --pid=16727 Remote serial target in gdb-specific protocol
(gdb) b main
Breakpoint 1 at 0x109168: main. (2 locations)
????? this has put a break at 2 locations in 2 different inferiors, reporting
only one address. Wondering if that is the expected
behaviour. In any case, that behaviour does not look to be a big deal.
(gdb) infer 1
[Switching to inferior 1 [Remote target]
(/home/philippe/valgrind/git/trunk_untouched/memcheck/tests/trivialleak)]
[Switching to thread 1.1 (Thread 10050)]
#0 0x0000000004001090 in _start () from /lib64/ld-linux-x86-64.so.2
(gdb) c
Continuing.
Connection 2 (remote lvgdb --pid=16727) does not support multi-target resumption.
(gdb)

So, the continue command is refused both in inferior 1 and inferior 2.

Then when stopping this gdb (which automatically continues the valgrind executables
till valgrind reports the next error), launching a new gdb, and only connecting to the 32
bits valgrind gdbserver, here is what I see.

(gdb) tar rem|lvgdb --pid=16727
Remote debugging using |lvgdb
...
0x04001092 in _dl_sysinfo_int80 () from /lib/ld-linux.so.2
(gdb) bt
#0 0x04001092 in _dl_sysinfo_int80 () from /lib/ld-linux.so.2
#1 0x0495fd27 in syscall () at ../sysdeps/unix/sysv/linux/i386/syscall.S:29
#2 0x0010922c in main () at scalar_exit_group.c:14
(gdb) c
Continuing.
../../multi-target-v1/gdb/inferior.c:285: internal-error: 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)

So, that looks to be a regression with the valgrind gdbserver.

I did another trial using an abbreviation for -no-connection, but then
that does not work:
(gdb) add-inferior -no-connection
[New inferior 2]
Added inferior 2
(gdb) add-inferior -no-conn
[New inferior 3]
Added inferior 3 on connection 1 (remote lvgdb --pid=17046)
(gdb)

Maybe add-inferior should better be converted to the option framework ?

Thanks

Philippe

On Sat, 2019-09-07 at 00:27 +0100, Pedro Alves wrote:
> This commit adds multi-target support to GDB. What this means is that
> with this commit, GDB can now be connected to different targets at the
> same time. E.g., you can debug a live native process and a core dump
> at the same time, connect to multiple gdbservers, etc.
>
> Patches 1 to 16 are preparatory patches.
>
> Patch 17 is the actual multi-target patch. This is the largest patch
> in the series. It does a number of things at the same time, but
> they're all intertwined, so I gave up on splitting it further.
>
> Patch 18 adds tests. Split out because patch 17 is already too big as
> is.
>
> Patch 21 does some user-visible changes, including a new command to
> list open target connections. This is just the bare minimum I could
> think of that is necessary for multi-target work. I'm sure we'll find
> other tweaks to other commands necessary.
>
> Documentation is in patch 23.
>
> This surely breaks the build on non-Linux ports as is. I have not
> tried to adjust the host-specific nat files to function API changes,
> but I don't envision any serious trouble. The fixes they'll need will
> be quite mechanical, mostly usually to pass
> 'current_inferior()->process_target ()' to functions that gained a new
> 'process_stratum_target *' parameter, similar to the changes to tdep
> files in the multi-target patch, need for which was caught with
> --enable-targets=all.
>
> I've pushed this to users/palves/multi-target-v1 on sourceware for
> review and testing convenience.
>
> Pedro Alves (23):
> Preserve selected thread in all-stop w/ background execution
> Don't rely on inferior_ptid in record_full_wait
> Make "show remote exec-file" inferior-aware
> exceptions.c:print_flush: Remove obsolete check
> Make target_ops::has_execution take an 'inferior *' instead of a
> ptid_t
> Don't check target is running in remote_target::mourn_inferior
> Delete unnecessary code from kill_command
> Introduce switch_to_inferior_no_thread
> switch inferior/thread before calling target methods
> Some get_last_target_status tweaks
> tfile_target::close: trace_fd can't be -1
> Use all_non_exited_inferiors in infrun.c
> Delete exit_inferior_silent(int pid)
> Tweak handling of remote errors in response to resumption packet
> Fix reconnecting to a gdbserver already debugging multiple processes,
> I
> Fix reconnecting to a gdbserver already debugging multiple processes,
> II
> Multi-target support
> Add multi-target tests
> gdbarch-selftests.c: No longer error out if debugging something
> Revert 'Remove unused struct serial::name field'
> Add "info connections" command, "info inferiors" connection
> number/string
> Require always-non-stop for multi-target resumptions
> Multi-target: NEWS and user manual
>
> gdb/doc/gdb.texinfo | 140 +++--
> gdb/doc/guile.texi | 4 +-
> gdb/doc/python.texi | 6 +-
> gdb/NEWS | 29 +
> gdb/Makefile.in | 1 +
> gdb/ada-tasks.c | 4 +-
> gdb/amd64-fbsd-tdep.c | 4 +-
> gdb/amd64-linux-nat.c | 2 +-
> gdb/break-catch-sig.c | 3 +-
> gdb/break-catch-syscall.c | 3 +-
> gdb/breakpoint.c | 25 +-
> gdb/bsd-uthread.c | 20 +-
> gdb/btrace.c | 2 +-
> gdb/corelow.c | 10 +-
> gdb/ctf.c | 2 +-
> gdb/event-top.c | 14 +-
> gdb/exceptions.c | 6 +-
> gdb/exec.c | 51 +-
> gdb/exec.h | 7 +
> gdb/fbsd-tdep.c | 3 +-
> gdb/fork-child.c | 7 +-
> gdb/gdbarch-selftests.c | 5 -
> gdb/gdbserver/fork-child.c | 3 +-
> gdb/gdbserver/inferiors.c | 2 +-
> gdb/gdbserver/linux-low.c | 2 +-
> gdb/gdbserver/remote-utils.c | 2 +-
> gdb/gdbserver/target.c | 8 +-
> gdb/gdbserver/target.h | 9 +-
> gdb/gdbsupport/common-gdbthread.h | 5 +-
> gdb/gdbthread.h | 133 ++--
> gdb/i386-fbsd-tdep.c | 4 +-
> gdb/inf-child.c | 2 +-
> gdb/inf-ptrace.c | 6 +-
> gdb/infcall.c | 3 +-
> gdb/infcmd.c | 129 ++--
> gdb/inferior-iter.h | 77 ++-
> gdb/inferior.c | 155 +++--
> gdb/inferior.h | 71 ++-
> gdb/infrun.c | 683 ++++++++++++++++-----
> gdb/infrun.h | 16 +-
> gdb/inline-frame.c | 51 +-
> gdb/inline-frame.h | 12 +-
> gdb/linux-fork.c | 5 +-
> gdb/linux-nat.c | 74 ++-
> gdb/linux-nat.h | 1 +
> gdb/linux-tdep.c | 3 +-
> gdb/linux-thread-db.c | 112 ++--
> gdb/mi/mi-interp.c | 10 +-
> gdb/nat/fork-inferior.c | 8 +-
> gdb/nat/fork-inferior.h | 5 +-
> gdb/ppc-fbsd-tdep.c | 4 +-
> gdb/proc-service.c | 17 +-
> gdb/process-stratum-target.c | 12 +-
> gdb/process-stratum-target.h | 31 +-
> gdb/python/py-threadevent.c | 4 +-
> gdb/ravenscar-thread.c | 16 +-
> gdb/record-btrace.c | 43 +-
> gdb/record-full.c | 22 +-
> gdb/regcache.c | 162 +++--
> gdb/regcache.h | 30 +-
> gdb/remote.c | 290 +++++----
> gdb/riscv-fbsd-tdep.c | 4 +-
> gdb/serial.c | 4 +
> gdb/serial.h | 1 +
> gdb/sol2-tdep.c | 2 +-
> gdb/solib-spu.c | 3 +-
> gdb/solib-svr4.c | 3 +-
> gdb/spu-multiarch.c | 6 +-
> gdb/spu-tdep.c | 8 +-
> gdb/target-connection.c | 153 +++++
> gdb/target-connection.h | 40 ++
> gdb/target-delegates.c | 27 +
> gdb/target.c | 166 +++--
> gdb/target.h | 23 +-
> gdb/testsuite/gdb.base/fork-running-state.exp | 17 +-
> .../gdb.base/kill-detach-inferiors-cmd.exp | 4 +-
> gdb/testsuite/gdb.base/quit-live.exp | 2 +-
> gdb/testsuite/gdb.base/remote-exec-file.exp | 46 ++
> gdb/testsuite/gdb.guile/scm-progspace.exp | 2 +-
> gdb/testsuite/gdb.linespec/linespec.exp | 2 +-
> gdb/testsuite/gdb.mi/new-ui-mi-sync.exp | 2 +-
> .../gdb.mi/user-selected-context-sync.exp | 2 +-
> gdb/testsuite/gdb.multi/multi-target.c | 100 +++
> gdb/testsuite/gdb.multi/multi-target.exp | 387 ++++++++++++
> gdb/testsuite/gdb.multi/remove-inferiors.exp | 2 +-
> gdb/testsuite/gdb.multi/watchpoint-multi.exp | 2 +-
> gdb/testsuite/gdb.python/py-inferior.exp | 4 +-
> .../gdb.server/extended-remote-restart.exp | 22 +-
> gdb/testsuite/gdb.threads/async.c | 70 +++
> gdb/testsuite/gdb.threads/async.exp | 98 +++
> gdb/testsuite/gdb.threads/fork-plus-threads.exp | 2 +-
> .../forking-threads-plus-breakpoint.exp | 2 +-
> gdb/testsuite/gdb.trace/report.exp | 2 +-
> gdb/testsuite/lib/gdbserver-support.exp | 4 +
> gdb/thread-iter.c | 14 +-
> gdb/thread-iter.h | 25 +-
> gdb/thread.c | 230 ++++---
> gdb/top.c | 17 +-
> gdb/tracefile-tfile.c | 5 +-
> gdb/tracefile.h | 2 +-
> 100 files changed, 3098 insertions(+), 977 deletions(-)
> create mode 100644 gdb/target-connection.c
> create mode 100644 gdb/target-connection.h
> create mode 100644 gdb/testsuite/gdb.base/remote-exec-file.exp
> create mode 100644 gdb/testsuite/gdb.multi/multi-target.c
> create mode 100644 gdb/testsuite/gdb.multi/multi-target.exp
> create mode 100644 gdb/testsuite/gdb.threads/async.c
> create mode 100644 gdb/testsuite/gdb.threads/async.exp
>