[00/23] Multi-target support

Message ID 20190906232807.6191-1-palves@redhat.com
Headers show
Series
  • Multi-target support
Related show

Message

Pedro Alves Sept. 6, 2019, 11:27 p.m.
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

-- 
2.14.5

Comments

Philippe Waroquiers Sept. 7, 2019, 11:19 a.m. | #1
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

>
Pedro Alves Sept. 8, 2019, 8:06 p.m. | #2
On 9/7/19 12:19 PM, Philippe Waroquiers wrote:
> 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).


Ahah, yeah, this was one of potential use cases for the multi-target work
that I talked about in last year's cauldron (in the multicore bof).

https://gcc.gnu.org/wiki/cauldron2018?action=AttachFile&do=get&target=palves-TheMulticoreGDBBoF%282018%29.pdf

> 

> 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 ?


Hmm, I'm not seeing how those would be an improvement over connection,
to be honest.

I'm already used to saying / hearing "open a connection to some target",
so I thought it was natural.  E.g., we have this command already:

 (gdb) help set auto-connect-native-target 
 Set whether GDB may automatically connect to the native target.
 When on, and GDB is not connected to a target yet, GDB
 attempts "run" and other commands with the native target.

Granted, I was the one who invented/added that command and
wrote that text.

But see the much older "target" command:

(gdb) help target 
Connect to a target machine or process.
^^^^^^^
The first argument is the type or protocol of the target machine.
Remaining arguments are interpreted by the target protocol.  For more
information on the arguments for a particular protocol, type
`help target ' followed by the protocol name.


Also, we have the "disconnect" command, with "disconnect" obviously
being the opposite of "connect", meaning close a connection:

 (gdb) target native 
 Done.  Use the "run" command to start a process.
 (gdb) maintenance print target-stack 
 The current target stack is:
   - native (Native process)
   - None (None)
 (gdb) disconnect 
 (gdb) maintenance print target-stack 
 The current target stack is:
   - None (None)

I'm thinking that it would be natural to use that
command to stop debugging a core dump too, for example,
I think this should be made to work:

 (gdb) core core.10872 
 ...
 Program terminated with signal SIGSEGV, Segmentation fault.
 #0  0x00007f176838d3e7 in ?? ()
 [Current thread is 1 (LWP 10872)]
 (gdb) maint print target-stack 
 The current target stack is:
   - core (Local core dump file)
   - None (None)
 (gdb) disconnect 
 You can't do that when your target is `core'
 (gdb) 

> 

> 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.


Yeah, this behavior shouldn't have changed with this patchset.  You should
see the exact same if you were debugging the two inferiors under the
same target.  Does it differ for you?  You have two locations, because
we create one location per inferior.  I don't recall offhand why we only
show one address -- maybe the address is the same for all locations?

> (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.


Does valgrind's gdbserver support non-stop mode?  I thought it didn't,
but if it does, then you need to do "maint set target-non-stop on"
before connecting.  This is one of the limitations that I described
in patch #17.

Was this with "set schedule-multiple" set to "on", or "off", BTW?

> 

> 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.


Thanks.  I'll have to try reproducing this.

> 

> 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 ?


Right, here's what I said in patch #17 about this:

 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 I tried converting "add-inferior" to the gdb::option framework, as a
 preparatory patch, but that stumbled on the fact that gdb::option does
 not support file options yet, for "add-inferior -exec".  I have a WIP
 patchset that adds that, but it's not a trivial patch, mainly due to
 need to integrate readline's filename completion, so I deferred that
 to some other time.
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Here's that WIP patchset in question:

 https://github.com/palves/gdb/commits/palves/filename-options

Thanks,
Pedro Alves
Philippe Waroquiers Sept. 8, 2019, 8:50 p.m. | #3
On Sun, 2019-09-08 at 21:06 +0100, Pedro Alves wrote:
> Maybe other wording could be used instead (such as 'target channel' 

> > and 'info channels') ?  Or maybe another synonym of channel or similar ?

> 

> Hmm, I'm not seeing how those would be an improvement over connection,

> to be honest.

Ok, the rational to use connection is convincing.

> (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.

> 

> Yeah, this behavior shouldn't have changed with this patchset.  You should

> see the exact same if you were debugging the two inferiors under the

> same target.  Does it differ for you?  You have two locations, because

> we create one location per inferior.  I don't recall offhand why we only

> show one address -- maybe the address is the same for all locations?

When using 2 native inferiors, one 64 bit and one 32 bits, origin/master gives:
(gdb) b main
Breakpoint 1 at 0x1168: main. (2 locations)
(gdb) info bre
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   <MULTIPLE>         
1.1                         y   0x0000000000001168 in main at trivialleak.c:12 inf 1
1.2                         y   0x000011d6         in main at scalar_exit_group.c:6 inf 2
(gdb) 

But I did some other tests with Ada generics and c++ templates,
and this all shows only one address, while multiple breakpoints have
been set at different addresses.
So, the behaviour with multi-target is normal.

> 

> > (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.

> 

> Does valgrind's gdbserver support non-stop mode?  I thought it didn't,

> but if it does, then you need to do "maint set target-non-stop on"

> before connecting.  This is one of the limitations that I described

> in patch #17.

valgrind gdbserver does not support non-stop mode.
> 

> Was this with "set schedule-multiple" set to "on", or "off", BTW?

I just tried with both values, and neither of them allow to continue.
So, with multiple valgrind gdbserver targets, I have not seen how to continue
execution.

> 

> > 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.

> 

> Thanks.  I'll have to try reproducing this.

> 

> > 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 ?

> 

> Right, here's what I said in patch #17 about this:

> 

>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

>  I tried converting "add-inferior" to the gdb::option framework, as a

>  preparatory patch, but that stumbled on the fact that gdb::option does

>  not support file options yet, for "add-inferior -exec".  I have a WIP

>  patchset that adds that, but it's not a trivial patch, mainly due to

>  need to integrate readline's filename completion, so I deferred that

>  to some other time.

>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Ouch, missed that.
What created the surprise is that add-inferior does not use
something like:
   strncmp (opt.get (), "-someoption", strlen (opt.get ())

Thanks

Philippe
Tom Tromey Sept. 9, 2019, 7:09 p.m. | #4
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:


Pedro> This commit adds multi-target support to GDB.  What this means is that
Pedro> with this commit, GDB can now be connected to different targets at the
Pedro> same time.  E.g., you can debug a live native process and a core dump
Pedro> at the same time, connect to multiple gdbservers, etc.

Awesome news!

Pedro> Patches 1 to 16 are preparatory patches.

I read these and sent comments where needed.  The rest looked ok to me.

I haven't read the remaining patches yet.

Pedro> This surely breaks the build on non-Linux ports as is.  I have
Pedro> not tried to adjust the host-specific nat files to function API
Pedro> changes, but I don't envision any serious trouble.

The buildbot should at least exercise a few of these.

Tom
Tom Tromey Sept. 9, 2019, 8:22 p.m. | #5
Pedro> Patch 17 is the actual multi-target patch.

I've looked at all the patches except this one, now.
I don't have time to look at it today.

Tom
Pedro Alves Oct. 16, 2019, 7:08 p.m. | #6
On 9/8/19 9:50 PM, Philippe Waroquiers wrote:

>>> (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.


I've finally debugged this, it was a simple bug in target.c.  We need this:

 @@ -2181,7 +2181,7 @@ user_visible_resume_ptid (int step)
  process_stratum_target *
  user_visible_resume_target (ptid_t resume_ptid)
  {
 -  return (resume_ptid == minus_one_ptid
 +  return (resume_ptid == minus_one_ptid && sched_multi
           ? NULL
           : current_inferior ()->process_target ());

I'm folding that into the main multi-target patch, which introduces
that function.

Thanks,
Pedro Alves