[v2,00/24] Multi-target support

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

Message

Pedro Alves Oct. 17, 2019, 10:50 p.m.
Here's v2 of the multi-target patchset, which addresses all the review
comments so far, I believe.  Patch 15 is new, so all following patches
are shifted by one.

This time, I've adjusted the host-specific nat files to function API
changes.  I tried to find spots that would need changes using grep.  I
built the series on AIX, x86/SPARC Solaris, 64-bit Windows, x86
GNU/Linux -m64/-m32, and Aarch64 GNU/Linux.  I'm currently running
this through the buildbot, will have results tomorrow.  I don't expect
any serious major issue, if any, as so far runs that completed seem
OK.

Follows the intro blurb:

This series adds multi-target support to GDB.  What this means is that
with this series, 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 17 are preparatory patches.

Patch 18 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 19 adds tests.  Split out because patch 18 is already too big as
is.

Patch 22 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 24.  Eli already reviewed and approved it.

I've pushed this to users/palves/multi-target-v2 on sourceware.

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

Tankut Baris Aktemur (1):
  Avoid another inferior_ptid reference in gdb/remote.c

 gdb/doc/gdb.texinfo                                | 137 ++--
 gdb/doc/guile.texi                                 |   4 +-
 gdb/doc/python.texi                                |   6 +-
 gdb/NEWS                                           |  29 +
 gdb/Makefile.in                                    |   1 +
 gdb/aarch64-linux-nat.c                            |   2 +-
 gdb/ada-tasks.c                                    |   4 +-
 gdb/aix-thread.c                                   |  24 +-
 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/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/lynx-low.c                           |   2 +-
 gdb/gdbserver/nto-low.c                            |   2 +-
 gdb/gdbserver/remote-utils.c                       |   2 +-
 gdb/gdbserver/target.c                             |   8 +-
 gdb/gdbserver/target.h                             |  11 +-
 gdb/gdbserver/win32-low.c                          |   4 +-
 gdb/gdbsupport/common-gdbthread.h                  |   5 +-
 gdb/gdbthread.h                                    | 133 ++--
 gdb/i386-fbsd-tdep.c                               |   4 +-
 gdb/i386-linux-nat.c                               |   2 +-
 gdb/inf-child.c                                    |   2 +-
 gdb/inf-ptrace.c                                   |   6 +-
 gdb/infcall.c                                      |   3 +-
 gdb/infcmd.c                                       | 129 ++--
 gdb/inferior-iter.h                                |  76 ++-
 gdb/inferior.c                                     | 156 +++--
 gdb/inferior.h                                     |  71 +-
 gdb/infrun.c                                       | 720 ++++++++++++++++-----
 gdb/infrun.h                                       |  23 +-
 gdb/inline-frame.c                                 |  51 +-
 gdb/inline-frame.h                                 |  12 +-
 gdb/linux-fork.c                                   |   5 +-
 gdb/linux-nat.c                                    |  76 ++-
 gdb/linux-nat.h                                    |   1 +
 gdb/linux-tdep.c                                   |   3 +-
 gdb/linux-thread-db.c                              | 112 ++--
 gdb/mi/mi-interp.c                                 |  10 +-
 gdb/mi/mi-main.c                                   |   6 +-
 gdb/nat/fork-inferior.c                            |   8 +-
 gdb/nat/fork-inferior.h                            |   5 +-
 gdb/nto-procfs.c                                   |   2 +-
 gdb/ppc-fbsd-tdep.c                                |   4 +-
 gdb/proc-service.c                                 |  17 +-
 gdb/process-stratum-target.c                       |  12 +-
 gdb/process-stratum-target.h                       |  31 +-
 gdb/procfs.c                                       |  49 +-
 gdb/python/py-threadevent.c                        |   4 +-
 gdb/ravenscar-thread.c                             |  15 +-
 gdb/record-btrace.c                                |  41 +-
 gdb/record-full.c                                  |  22 +-
 gdb/regcache.c                                     | 162 +++--
 gdb/regcache.h                                     |  30 +-
 gdb/remote.c                                       | 307 +++++----
 gdb/riscv-fbsd-tdep.c                              |   4 +-
 gdb/serial.c                                       |   4 +
 gdb/serial.h                                       |   1 +
 gdb/sol-thread.c                                   |  28 +-
 gdb/sol2-tdep.c                                    |   2 +-
 gdb/solib-svr4.c                                   |   3 +-
 gdb/target-connection.c                            | 159 +++++
 gdb/target-connection.h                            |  40 ++
 gdb/target-delegates.c                             |  27 +
 gdb/target.c                                       | 192 +++---
 gdb/target.h                                       |  41 +-
 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/tids-gid-reset.c           |  22 +
 gdb/testsuite/gdb.multi/tids-gid-reset.exp         |  96 +++
 gdb/testsuite/gdb.multi/watchpoint-multi.exp       |   2 +-
 gdb/testsuite/gdb.python/py-inferior.exp           |   4 +-
 .../gdb.server/connect-without-multi-process.exp   |   7 +-
 .../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/tracectf.c                                     |   2 +-
 gdb/tracefile-tfile.c                              |   5 +-
 gdb/tracefile.h                                    |   2 +-
 gdb/windows-nat.c                                  |  20 +-
 111 files changed, 3360 insertions(+), 1073 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.multi/tids-gid-reset.c
 create mode 100644 gdb/testsuite/gdb.multi/tids-gid-reset.exp
 create mode 100644 gdb/testsuite/gdb.threads/async.c
 create mode 100644 gdb/testsuite/gdb.threads/async.exp

-- 
2.14.5

Comments

John Baldwin Oct. 18, 2019, 8:22 p.m. | #1
On 10/17/19 3:50 PM, Pedro Alves wrote:
> Here's v2 of the multi-target patchset, which addresses all the review

> comments so far, I believe.  Patch 15 is new, so all following patches

> are shifted by one.

> 

> This time, I've adjusted the host-specific nat files to function API

> changes.  I tried to find spots that would need changes using grep.  I

> built the series on AIX, x86/SPARC Solaris, 64-bit Windows, x86

> GNU/Linux -m64/-m32, and Aarch64 GNU/Linux.  I'm currently running

> this through the buildbot, will have results tomorrow.  I don't expect

> any serious major issue, if any, as so far runs that completed seem

> OK.


I (finally) have a patch to fix the build on FreeBSD/amd64 (and probably
FreeBSD on other platforms) here:

https://github.com/bsdjhb/gdb/commit/e58a36eaef6244d2040ce6f377497ee898978db4

It's a combined patch but has some commentary on bsd-kvm.c which is
kind of special.  That target adds new commands that need to find a
target to operate on.  I opted to have it look at the current inferior
and if (using a dynamic cast) it is a bsd-kvm target the commands
modify the state of that inferior.  I haven't tested it though as I don't
use bsd-kvm.c.

I did try a simple test of creating two inferiors both of which were
running /bin/ls and gdb hung trying to run the second inferior via
'start'.  I suspect this is some kind of bug in the FreeBSD target not
being multi-target ready that I will have to debug.

-- 
John Baldwin
Philippe Waroquiers Oct. 20, 2019, 11:41 a.m. | #2
I played a little bit with the patch and valgrind
(I had to apply the patch with git am --3way, otherwise it failed to apply).
No problem encountered when playing with the result.
I also re-read the documentation (as I forgot about how this was all
working), and it was all pretty clear.

3 notes:
  * It might be interesting to one day add
     something like 'inferior apply all/list of inferiors SOMMECOMMAND'
  * when having 2 inferiors connected to 2 different valgrind gdbservers,
    I could continue both inferiors by using 'continue&',
    but I had to (artificially) issue 'interrupt' in the second inferior
    to have GDB accepting 'continue&'.  So, this might be the indication
    of a status 'running' which should be maintained per inferior,
    while it might now be maintained globally.
  * I was (again?) confused by add-inferior silently ignoring abbreviations
    (or more generally anything starting with - unless matching exactly
     an option).
    Waiting for option framework to be extended, the add-inferior command
    could use the below that accepts abbreviations and does not ignore
    wrong options.

Thanks

Philippe


diff --git a/gdb/inferior.c b/gdb/inferior.c
index 061cf5cebb..5d72780eb3 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -786,24 +786,23 @@ add_inferior_command (const char *args, int from_tty)
 
       for (char **argv = built_argv.get (); *argv != NULL; argv++)
 	{
-	  if (**argv == '-')
+	  int len = strlen (*argv);
+
+	  if (strncmp (*argv, "-copies", len) == 0)
 	    {
-	      if (strcmp (*argv, "-copies") == 0)
-		{
-		  ++argv;
-		  if (!*argv)
-		    error (_("No argument to -copies"));
-		  copies = parse_and_eval_long (*argv);
-		}
-	      else if (strcmp (*argv, "-no-connection") == 0)
-		no_connection = true;
-	      else if (strcmp (*argv, "-exec") == 0)
-		{
-		  ++argv;
-		  if (!*argv)
-		    error (_("No argument to -exec"));
-		  exec.reset (tilde_expand (*argv));
-		}
+	      ++argv;
+	      if (!*argv)
+		error (_("No argument to -copies"));
+	      copies = parse_and_eval_long (*argv);
+	    }
+	  else if (strncmp (*argv, "-no-connection", len) == 0)
+	    no_connection = true;
+	  else if (strncmp (*argv, "-exec", len) == 0)
+	    {
+	      ++argv;
+	      if (!*argv)
+		error (_("No argument to -exec"));
+	      exec.reset (tilde_expand (*argv));
 	    }
 	  else
 	    error (_("Invalid argument"));



On Thu, 2019-10-17 at 23:50 +0100, Pedro Alves wrote:
> Here's v2 of the multi-target patchset, which addresses all the review

> comments so far, I believe.  Patch 15 is new, so all following patches

> are shifted by one.

> 

> This time, I've adjusted the host-specific nat files to function API

> changes.  I tried to find spots that would need changes using grep.  I

> built the series on AIX, x86/SPARC Solaris, 64-bit Windows, x86

> GNU/Linux -m64/-m32, and Aarch64 GNU/Linux.  I'm currently running

> this through the buildbot, will have results tomorrow.  I don't expect

> any serious major issue, if any, as so far runs that completed seem

> OK.

> 

> Follows the intro blurb:

> 

> This series adds multi-target support to GDB.  What this means is that

> with this series, 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 17 are preparatory patches.

> 

> Patch 18 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 19 adds tests.  Split out because patch 18 is already too big as

> is.

> 

> Patch 22 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 24.  Eli already reviewed and approved it.

> 

> I've pushed this to users/palves/multi-target-v2 on sourceware.

> 

> 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

> 

> Tankut Baris Aktemur (1):

>   Avoid another inferior_ptid reference in gdb/remote.c

> 

>  gdb/doc/gdb.texinfo                                | 137 ++--

>  gdb/doc/guile.texi                                 |   4 +-

>  gdb/doc/python.texi                                |   6 +-

>  gdb/NEWS                                           |  29 +

>  gdb/Makefile.in                                    |   1 +

>  gdb/aarch64-linux-nat.c                            |   2 +-

>  gdb/ada-tasks.c                                    |   4 +-

>  gdb/aix-thread.c                                   |  24 +-

>  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/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/lynx-low.c                           |   2 +-

>  gdb/gdbserver/nto-low.c                            |   2 +-

>  gdb/gdbserver/remote-utils.c                       |   2 +-

>  gdb/gdbserver/target.c                             |   8 +-

>  gdb/gdbserver/target.h                             |  11 +-

>  gdb/gdbserver/win32-low.c                          |   4 +-

>  gdb/gdbsupport/common-gdbthread.h                  |   5 +-

>  gdb/gdbthread.h                                    | 133 ++--

>  gdb/i386-fbsd-tdep.c                               |   4 +-

>  gdb/i386-linux-nat.c                               |   2 +-

>  gdb/inf-child.c                                    |   2 +-

>  gdb/inf-ptrace.c                                   |   6 +-

>  gdb/infcall.c                                      |   3 +-

>  gdb/infcmd.c                                       | 129 ++--

>  gdb/inferior-iter.h                                |  76 ++-

>  gdb/inferior.c                                     | 156 +++--

>  gdb/inferior.h                                     |  71 +-

>  gdb/infrun.c                                       | 720 ++++++++++++++++-----

>  gdb/infrun.h                                       |  23 +-

>  gdb/inline-frame.c                                 |  51 +-

>  gdb/inline-frame.h                                 |  12 +-

>  gdb/linux-fork.c                                   |   5 +-

>  gdb/linux-nat.c                                    |  76 ++-

>  gdb/linux-nat.h                                    |   1 +

>  gdb/linux-tdep.c                                   |   3 +-

>  gdb/linux-thread-db.c                              | 112 ++--

>  gdb/mi/mi-interp.c                                 |  10 +-

>  gdb/mi/mi-main.c                                   |   6 +-

>  gdb/nat/fork-inferior.c                            |   8 +-

>  gdb/nat/fork-inferior.h                            |   5 +-

>  gdb/nto-procfs.c                                   |   2 +-

>  gdb/ppc-fbsd-tdep.c                                |   4 +-

>  gdb/proc-service.c                                 |  17 +-

>  gdb/process-stratum-target.c                       |  12 +-

>  gdb/process-stratum-target.h                       |  31 +-

>  gdb/procfs.c                                       |  49 +-

>  gdb/python/py-threadevent.c                        |   4 +-

>  gdb/ravenscar-thread.c                             |  15 +-

>  gdb/record-btrace.c                                |  41 +-

>  gdb/record-full.c                                  |  22 +-

>  gdb/regcache.c                                     | 162 +++--

>  gdb/regcache.h                                     |  30 +-

>  gdb/remote.c                                       | 307 +++++----

>  gdb/riscv-fbsd-tdep.c                              |   4 +-

>  gdb/serial.c                                       |   4 +

>  gdb/serial.h                                       |   1 +

>  gdb/sol-thread.c                                   |  28 +-

>  gdb/sol2-tdep.c                                    |   2 +-

>  gdb/solib-svr4.c                                   |   3 +-

>  gdb/target-connection.c                            | 159 +++++

>  gdb/target-connection.h                            |  40 ++

>  gdb/target-delegates.c                             |  27 +

>  gdb/target.c                                       | 192 +++---

>  gdb/target.h                                       |  41 +-

>  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/tids-gid-reset.c           |  22 +

>  gdb/testsuite/gdb.multi/tids-gid-reset.exp         |  96 +++

>  gdb/testsuite/gdb.multi/watchpoint-multi.exp       |   2 +-

>  gdb/testsuite/gdb.python/py-inferior.exp           |   4 +-

>  .../gdb.server/connect-without-multi-process.exp   |   7 +-

>  .../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/tracectf.c                                     |   2 +-

>  gdb/tracefile-tfile.c                              |   5 +-

>  gdb/tracefile.h                                    |   2 +-

>  gdb/windows-nat.c                                  |  20 +-

>  111 files changed, 3360 insertions(+), 1073 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.multi/tids-gid-reset.c

>  create mode 100644 gdb/testsuite/gdb.multi/tids-gid-reset.exp

>  create mode 100644 gdb/testsuite/gdb.threads/async.c

>  create mode 100644 gdb/testsuite/gdb.threads/async.exp

>
Pedro Alves Oct. 29, 2019, 7:13 p.m. | #3
On 10/18/19 9:22 PM, John Baldwin wrote:
> On 10/17/19 3:50 PM, Pedro Alves wrote:

>> Here's v2 of the multi-target patchset, which addresses all the review

>> comments so far, I believe.  Patch 15 is new, so all following patches

>> are shifted by one.

>>

>> This time, I've adjusted the host-specific nat files to function API

>> changes.  I tried to find spots that would need changes using grep.  I

>> built the series on AIX, x86/SPARC Solaris, 64-bit Windows, x86

>> GNU/Linux -m64/-m32, and Aarch64 GNU/Linux.  I'm currently running

>> this through the buildbot, will have results tomorrow.  I don't expect

>> any serious major issue, if any, as so far runs that completed seem

>> OK.

> 

> I (finally) have a patch to fix the build on FreeBSD/amd64 (and probably

> FreeBSD on other platforms) here:

> 

> https://github.com/bsdjhb/gdb/commit/e58a36eaef6244d2040ce6f377497ee898978db4

> 


Thanks!

> It's a combined patch but has some commentary on bsd-kvm.c which is

> kind of special.  That target adds new commands that need to find a

> target to operate on.  I opted to have it look at the current inferior

> and if (using a dynamic cast) it is a bsd-kvm target the commands

> modify the state of that inferior.  I haven't tested it though as I don't

> use bsd-kvm.c.


I'd rather not merge the bsd-kvm.c parts into my patches as is, for the
reason that it doesn't appear necessary for a minimal keep-working-as-before
change.  I think it would be fine as a follow up patch, though.

A couple comments:

 - I think we should get away from doing this

      inferior_ptid = null_ptid;
      exit_inferior_silent (current_inferior ());

   in 'bsd_kvm_target::close ()'.

 - Class private fields should be named 'm_foo', so m_kd, m_corefile, etc.

For bsd-kvm.c, I _think_ that the only change necessary to keep things
building would be this:

--- a/gdb/bsd-kvm.c
+++ b/gdb/bsd-kvm.c
@@ -136,7 +136,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)
   core_kd = temp_kd;
   push_target (&bsd_kvm_ops);
 
-  add_thread_silent (bsd_kvm_ptid);
+  add_thread_silent (&bsd_kvm_ops, bsd_kvm_ptid);
   inferior_ptid = bsd_kvm_ptid;

Right?  See updated patch below.

> I did try a simple test of creating two inferiors both of which were

> running /bin/ls and gdb hung trying to run the second inferior via

> 'start'.  I suspect this is some kind of bug in the FreeBSD target not

> being multi-target ready that I will have to debug.


Does that work without the multi-target series?  I mean, maybe it's more
a multi-process debugging issue than a multi-target issue?

From 3f879fe9334cbd69f50efd0131c2bed5498cde92 Mon Sep 17 00:00:00 2001
From: John Baldwin <jhb@FreeBSD.org>

Date: Fri, 18 Oct 2019 13:12:11 -0700
Subject: [PATCH] Update FreeBSD native build for multi-target-v2.

---
 gdb/bsd-kvm.c  |  2 +-
 gdb/fbsd-nat.c | 33 +++++++++++++++++----------------
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/gdb/bsd-kvm.c b/gdb/bsd-kvm.c
index 21f978728d..5ffc303f46 100644
--- a/gdb/bsd-kvm.c
+++ b/gdb/bsd-kvm.c
@@ -136,7 +136,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)
   core_kd = temp_kd;
   push_target (&bsd_kvm_ops);
 
-  add_thread_silent (bsd_kvm_ptid);
+  add_thread_silent (&bsd_kvm_ops, bsd_kvm_ptid);
   inferior_ptid = bsd_kvm_ptid;
 
   target_fetch_registers (get_current_regcache (), -1);
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 0274ff542e..bb34a9769d 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -991,11 +991,11 @@ fbsd_enable_proc_events (pid_t pid)
    called to discover new threads each time the thread list is updated.  */
 
 static void
-fbsd_add_threads (pid_t pid)
+fbsd_add_threads (fbsd_nat_target *target, pid_t pid)
 {
   int i, nlwps;
 
-  gdb_assert (!in_thread_list (ptid_t (pid)));
+  gdb_assert (!in_thread_list (target, ptid_t (pid)));
   nlwps = ptrace (PT_GETNUMLWPS, pid, NULL, 0);
   if (nlwps == -1)
     perror_with_name (("ptrace"));
@@ -1010,7 +1010,7 @@ fbsd_add_threads (pid_t pid)
     {
       ptid_t ptid = ptid_t (pid, lwps[i], 0);
 
-      if (!in_thread_list (ptid))
+      if (!in_thread_list (target, ptid))
 	{
 #ifdef PT_LWP_EVENTS
 	  struct ptrace_lwpinfo pl;
@@ -1026,7 +1026,7 @@ fbsd_add_threads (pid_t pid)
 	    fprintf_unfiltered (gdb_stdlog,
 				"FLWP: adding thread for LWP %u\n",
 				lwps[i]);
-	  add_thread (ptid);
+	  add_thread (target, ptid);
 	}
     }
 }
@@ -1043,7 +1043,7 @@ fbsd_nat_target::update_thread_list ()
 #else
   prune_threads ();
 
-  fbsd_add_threads (inferior_ptid.pid ());
+  fbsd_add_threads (this, inferior_ptid.pid ());
 #endif
 }
 
@@ -1174,7 +1174,7 @@ fbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signo)
   if (ptid.lwp_p ())
     {
       /* If ptid is a specific LWP, suspend all other LWPs in the process.  */
-      inferior *inf = find_inferior_ptid (ptid);
+      inferior *inf = find_inferior_ptid (this, ptid);
 
       for (thread_info *tp : inf->non_exited_threads ())
         {
@@ -1193,7 +1193,7 @@ fbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signo)
     {
       /* If ptid is a wildcard, resume all matching threads (they won't run
 	 until the process is continued however).  */
-      for (thread_info *tp : all_non_exited_threads (ptid))
+      for (thread_info *tp : all_non_exited_threads (this, ptid))
 	if (ptrace (PT_RESUME, tp->ptid.lwp (), NULL, 0) == -1)
 	  perror_with_name (("ptrace"));
       ptid = inferior_ptid;
@@ -1239,7 +1239,8 @@ fbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signo)
    core, return true.  */
 
 static bool
-fbsd_handle_debug_trap (ptid_t ptid, const struct ptrace_lwpinfo &pl)
+fbsd_handle_debug_trap (fbsd_nat_target *target, ptid_t ptid,
+			const struct ptrace_lwpinfo &pl)
 {
 
   /* Ignore traps without valid siginfo or for signals other than
@@ -1266,7 +1267,7 @@ fbsd_handle_debug_trap (ptid_t ptid, const struct ptrace_lwpinfo &pl)
   if (pl.pl_siginfo.si_code == TRAP_BRKPT)
     {
       /* Fixup PC for the software breakpoint.  */
-      struct regcache *regcache = get_thread_regcache (ptid);
+      struct regcache *regcache = get_thread_regcache (target, ptid);
       struct gdbarch *gdbarch = regcache->arch ();
       int decr_pc = gdbarch_decr_pc_after_break (gdbarch);
 
@@ -1340,7 +1341,7 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 		 threads might be skipped during post_attach that
 		 have not yet reported their PL_FLAG_EXITED event.
 		 Ignore EXITED events for an unknown LWP.  */
-	      thread_info *thr = find_thread_ptid (wptid);
+	      thread_info *thr = find_thread_ptid (this, wptid);
 	      if (thr != nullptr)
 		{
 		  if (debug_fbsd_lwp)
@@ -1364,13 +1365,13 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 	     PL_FLAG_BORN in case the first stop reported after
 	     attaching to an existing process is a PL_FLAG_BORN
 	     event.  */
-	  if (in_thread_list (ptid_t (pid)))
+	  if (in_thread_list (this, ptid_t (pid)))
 	    {
 	      if (debug_fbsd_lwp)
 		fprintf_unfiltered (gdb_stdlog,
 				    "FLWP: using LWP %u for first thread\n",
 				    pl.pl_lwpid);
-	      thread_change_ptid (ptid_t (pid), wptid);
+	      thread_change_ptid (this, ptid_t (pid), wptid);
 	    }
 
 #ifdef PT_LWP_EVENTS
@@ -1380,13 +1381,13 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 		 threads might be added by fbsd_add_threads that have
 		 not yet reported their PL_FLAG_BORN event.  Ignore
 		 BORN events for an already-known LWP.  */
-	      if (!in_thread_list (wptid))
+	      if (!in_thread_list (this, wptid))
 		{
 		  if (debug_fbsd_lwp)
 		    fprintf_unfiltered (gdb_stdlog,
 					"FLWP: adding thread for LWP %u\n",
 					pl.pl_lwpid);
-		  add_thread (wptid);
+		  add_thread (this, wptid);
 		}
 	      ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
 	      return wptid;
@@ -1474,7 +1475,7 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 #endif
 
 #ifdef USE_SIGTRAP_SIGINFO
-	  if (fbsd_handle_debug_trap (wptid, pl))
+	  if (fbsd_handle_debug_trap (this, wptid, pl))
 	    return wptid;
 #endif
 
@@ -1633,7 +1634,7 @@ void
 fbsd_nat_target::post_attach (int pid)
 {
   fbsd_enable_proc_events (pid);
-  fbsd_add_threads (pid);
+  fbsd_add_threads (this, pid);
 }
 
 #ifdef PL_FLAG_EXEC
-- 
2.14.5
Pedro Alves Oct. 29, 2019, 7:56 p.m. | #4
On 10/20/19 12:41 PM, Philippe Waroquiers wrote:
> I played a little bit with the patch and valgrind

> (I had to apply the patch with git am --3way, otherwise it failed to apply).


Guess you missed that this was in the users/palves/multi-target-v2 branch.  :-/

> No problem encountered when playing with the result.

> I also re-read the documentation (as I forgot about how this was all

> working), and it was all pretty clear.


That's great to hear.

> 

> 3 notes:

>   * It might be interesting to one day add

>      something like 'inferior apply all/list of inferiors SOMMECOMMAND'


Agreed.

>   * when having 2 inferiors connected to 2 different valgrind gdbservers,

>     I could continue both inferiors by using 'continue&',

>     but I had to (artificially) issue 'interrupt' in the second inferior

>     to have GDB accepting 'continue&'.  So, this might be the indication

>     of a status 'running' which should be maintained per inferior,

>     while it might now be maintained globally.


I've tried to debug this a little, but I'd like to punt for now.  The error
I'm seeing first comes from a breakpoint re-set, where gdb tries to
read memory from the valgrind that is running.  Breakpoint re-sets currently
are too dumb and re-set all locations, instead of only adding/removing
the locations necessary.  And then that is running into the fact that
the remote protocol in the old all-stop mode (which is what valgrind supports)
is synchronous, gdb can't talk to the remote target until the target
reports a stop.

These sorts of issues is why I required that the remote backends
support non-stop mode for this initial pass.  I'd rather not have to fix
all these issues before landing the initial multi-target support, even
if we know there are issues we need to fix to make it cope better with
all-stop-only targets.

>   * I was (again?) confused by add-inferior silently ignoring abbreviations

>     (or more generally anything starting with - unless matching exactly

>      an option).

>     Waiting for option framework to be extended, the add-inferior command

>     could use the below that accepts abbreviations and does not ignore

>     wrong options.


Yes, that is fine with me.

Thanks,
Pedro Alves
Tom Tromey Nov. 1, 2019, 2:56 p.m. | #5
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:


Pedro> Here's v2 of the multi-target patchset, which addresses all the review
Pedro> comments so far, I believe.  Patch 15 is new, so all following patches
Pedro> are shifted by one.

I read through all of these.  Well, I skimmed patch 18, as it is
probably too big to review (maybe could have been split up a bit more,
but too late now!).

I sent all my comments.  They're basically nits, though there was one
question I'd like to understand.

I think this should all go in.  Congratulations on doing this, it's a
big improvement!

thanks,
Tom
John Baldwin Jan. 9, 2020, 7:31 p.m. | #6
On 10/29/19 12:13 PM, Pedro Alves wrote:
> On 10/18/19 9:22 PM, John Baldwin wrote:

>> On 10/17/19 3:50 PM, Pedro Alves wrote:

>>> Here's v2 of the multi-target patchset, which addresses all the review

>>> comments so far, I believe.  Patch 15 is new, so all following patches

>>> are shifted by one.

>>>

>>> This time, I've adjusted the host-specific nat files to function API

>>> changes.  I tried to find spots that would need changes using grep.  I

>>> built the series on AIX, x86/SPARC Solaris, 64-bit Windows, x86

>>> GNU/Linux -m64/-m32, and Aarch64 GNU/Linux.  I'm currently running

>>> this through the buildbot, will have results tomorrow.  I don't expect

>>> any serious major issue, if any, as so far runs that completed seem

>>> OK.

>>

>> I (finally) have a patch to fix the build on FreeBSD/amd64 (and probably

>> FreeBSD on other platforms) here:

>>

>> https://github.com/bsdjhb/gdb/commit/e58a36eaef6244d2040ce6f377497ee898978db4

>>

> 

> Thanks!

> 

>> It's a combined patch but has some commentary on bsd-kvm.c which is

>> kind of special.  That target adds new commands that need to find a

>> target to operate on.  I opted to have it look at the current inferior

>> and if (using a dynamic cast) it is a bsd-kvm target the commands

>> modify the state of that inferior.  I haven't tested it though as I don't

>> use bsd-kvm.c.

> 

> I'd rather not merge the bsd-kvm.c parts into my patches as is, for the

> reason that it doesn't appear necessary for a minimal keep-working-as-before

> change.  I think it would be fine as a follow up patch, though.


Ok, that's fine.

> For bsd-kvm.c, I _think_ that the only change necessary to keep things

> building would be this:

> 

> --- a/gdb/bsd-kvm.c

> +++ b/gdb/bsd-kvm.c

> @@ -136,7 +136,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)

>    core_kd = temp_kd;

>    push_target (&bsd_kvm_ops);

>  

> -  add_thread_silent (bsd_kvm_ptid);

> +  add_thread_silent (&bsd_kvm_ops, bsd_kvm_ptid);

>    inferior_ptid = bsd_kvm_ptid;

> 

> Right?  See updated patch below.


Ok, this works for me.

-- 
John Baldwin
Pedro Alves Jan. 9, 2020, 7:50 p.m. | #7
On 1/9/20 7:31 PM, John Baldwin wrote:
> On 10/29/19 12:13 PM, Pedro Alves wrote:


>> For bsd-kvm.c, I _think_ that the only change necessary to keep things

>> building would be this:

>>

>> --- a/gdb/bsd-kvm.c

>> +++ b/gdb/bsd-kvm.c

>> @@ -136,7 +136,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)

>>    core_kd = temp_kd;

>>    push_target (&bsd_kvm_ops);

>>  

>> -  add_thread_silent (bsd_kvm_ptid);

>> +  add_thread_silent (&bsd_kvm_ops, bsd_kvm_ptid);

>>    inferior_ptid = bsd_kvm_ptid;

>>

>> Right?  See updated patch below.

> 

> Ok, this works for me.


Excellent!  I think I'll be merging this soon, just need to
address Aktemur's latest comments.

Thanks,
Pedro Alves
Tankut Baris Aktemur Jan. 10, 2020, 1:49 p.m. | #8
On Thursday, January 9, 2020 8:50 PM, Pedro Alves wrote:
> 

> Excellent!  I think I'll be merging this soon, just need to

> address Aktemur's latest comments.

> 

> Thanks,

> Pedro Alves


There is one more minor thing, I think.  The "info inferiors" command
iterates over the list of inferiors and prints their info.  One piece of
information is the inferior id, which is printed through the target.
This means that for each inferior, the target of the current inferior is
used, potentially yielding incorrect output.  To fix, inferior can be switched
temporarily before printing its id.  Please see the patches below for details.
The second one attempts to modify the multi-target.exp test to check correct
output.

Thanks
-Baris

From efa9736da4d5545232fd3c3f8c65e297963d556f Mon Sep 17 00:00:00 2001
From: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>

Date: Fri, 10 Jan 2020 14:27:55 +0100
Subject: [PATCH 1/2] Switch the inferior before outputting its id in "info
 inferiors"

GDB uses the 'current_top_target' when displaying the description of
an inferior.  This leads to same target being used for each inferior
and, in turn, yields incorrect output when the inferior has a target
that is supposed to give a specialized output.  For instance, the
remote target outputs "Remote target" instead of "process XYZ" as the
description if the multi-process feature is not supported or turned
off.

E.g.: Suppose we have a native and a remote target, and the native is
the current inferior.  The remote target does not support multi-process.
For "info inferiors", we would expect to see:

~~~
(gdb) i inferiors
  Num  Description       Connection       Executable
* 1    process 29060     1 (native)       /a/path
  2    Remote target     2 (remote ...)
~~~

but instead we get

~~~
(gdb) i inferiors
  Num  Description       Connection       Executable
* 1    process 29060     1 (native)       /a/path
  2    process 42000     2 (remote ...)
~~~

Similarly, if the current inferior is the remote one, we would expect
to see

~~~
(gdb) i inferiors
  Num  Description       Connection       Executable
  1    process 29060     1 (native)       /a/path
* 2    Remote target     2 (remote ...)
~~~

but we get

~~~
(gdb) i inferiors
  Num  Description       Connection       Executable
* 1    Remote target     1 (native)       /a/path
  2    Remote target     2 (remote ...)
~~~

With this patch, we switch to the inferior when outputting its
description, so that the current_top_target will be aligned to the
inferior we are displaying.

---
 gdb/inferior.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/gdb/inferior.c b/gdb/inferior.c
index 3ce43860f7..ef91e74e89 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -494,6 +494,11 @@ print_inferior (struct ui_out *uiout, const char *requested_inferiors)
   uiout->table_header (17, ui_left, "exec", "Executable");
 
   uiout->table_body ();
+
+  /* Restore the current thread after the loop because we switch the
+     inferior in the loop.  */
+  scoped_restore_current_pspace_and_thread restore_pspace_thread;
+  inferior *current_inf = current_inferior ();
   for (inferior *inf : all_inferiors ())
     {
       if (!number_is_in_list (requested_inferiors, inf->num))
@@ -501,13 +506,17 @@ print_inferior (struct ui_out *uiout, const char *requested_inferiors)
 
       ui_out_emit_tuple tuple_emitter (uiout, NULL);
 
-      if (inf == current_inferior ())
+      if (inf == current_inf)
 	uiout->field_string ("current", "*");
       else
 	uiout->field_skip ("current");
 
       uiout->field_signed ("number", inf->num);
 
+      /* Because the pid_to_str uses current_top_target,
+	 switch the inferior.  */
+      switch_to_inferior_no_thread (inf);
+
       uiout->field_string ("target-id", inferior_pid_to_str (inf->pid));
 
       std::string conn = uiout_field_connection (inf->process_target ());
-- 
2.17.1

From 0779c90c63359abb2574aa02647969296d944791 Mon Sep 17 00:00:00 2001
From: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>

Date: Fri, 10 Jan 2020 14:27:54 +0100
Subject: [PATCH 2/2] testsuite: enhance "info inferiors" test for multi-target

Expand the "info inferiors" test for the multi-target feature.  The
test was checking for the output of the info commands after setup,
only when the current inferior is the last added inferior.

This patch does the following:

1. The "info inferiors" and "info connections" test is extracted out
   from the "setup" procedure to a separate procedure.

2. The test is enriched to check the output after switching to each
   inferior, not just the last one.

3. The test is performed twice; one for when the multi-process feature
   is turned on, one for off.

---
 gdb/testsuite/gdb.multi/multi-target.exp | 111 +++++++++++++++++------
 1 file changed, 84 insertions(+), 27 deletions(-)

diff --git a/gdb/testsuite/gdb.multi/multi-target.exp b/gdb/testsuite/gdb.multi/multi-target.exp
index be466c110b..63a7a287d1 100644
--- a/gdb/testsuite/gdb.multi/multi-target.exp
+++ b/gdb/testsuite/gdb.multi/multi-target.exp
@@ -137,33 +137,6 @@ proc setup {non-stop} {
 	return 0
     }
 
-    set ws "\[ \t\]+"
-    global decimal
-
-    # Test "info connections" and "info inferior"'s "Connection"
-    # column, while at it.
-
-    gdb_test "info connections" \
-	[multi_line \
-	     "Num${ws}What${ws}Description${ws}" \
-	     "  1${ws}native${ws}Native process${ws}" \
-	     "  2${ws}extended-remote localhost:$decimal${ws}Extended remote serial target in gdb-specific protocol${ws}" \
-	     "  3${ws}core${ws}Local core dump file${ws}" \
-	     "  4${ws}extended-remote localhost:$decimal${ws}Extended remote serial target in gdb-specific protocol${ws}" \
-	   "\\* 5${ws}core${ws}Local core dump file${ws}" \
-	    ]
-
-    gdb_test "info inferiors" \
-	[multi_line \
-	     "Num${ws}Description${ws}Connection${ws}Executable${ws}" \
-	     "  1${ws}process ${decimal}${ws}1 \\(native\\)${ws}${binfile}${ws}" \
-	     "  2${ws}process ${decimal}${ws}2 \\(extended-remote localhost:$decimal\\)${ws}${binfile}${ws}" \
-	     "  3${ws}process ${decimal}${ws}3 \\(core\\)${ws}${binfile}${ws}" \
-	     "  4${ws}process ${decimal}${ws}1 \\(native\\)${ws}${binfile}${ws}" \
-	     "  5${ws}process ${decimal}${ws}4 \\(extended-remote localhost:$decimal\\)${ws}${binfile}${ws}" \
-	   "\\* 6${ws}process ${decimal}${ws}5 \\(core\\)${ws}${binfile}${ws}" \
-	    ]
-
     # For debugging.
     gdb_test "info threads" ".*"
 
@@ -365,6 +338,83 @@ proc test_ping_pong_next {} {
     }
 }
 
+# Test "info inferiors" and "info connections" where the multi-process
+# feature of remote targets is turned off or on.
+proc test_info_inferiors {multi_process} {
+    setup "off"
+
+    gdb_test_no_output \
+	"set remote multiprocess-feature-packet $multi_process"
+
+    # Get the description for inferior INF for when the current
+    # inferior id is CURRENT.
+    proc inf_desc {inf current} {
+	set ws "\[ \t\]+"
+	global decimal
+	upvar multi_process multi_process
+
+	if {($multi_process == "off") && ($inf == 2 || $inf == 5)} {
+	    set desc "Remote target"
+	} else {
+	    set desc "process ${decimal}"
+	}
+
+	set desc "${inf}${ws}${desc}${ws}"
+	if {$inf == $current} {
+	    return "\\* $desc"
+	} else {
+	    return "  $desc"
+	}
+    }
+
+    # Get the "Num" column for CONNECTION for when the current
+    # inferior id is CURRENT_INF
+    proc connection_num {connection current_inf} {
+	switch $current_inf {
+	    "4" { set current_connection "1"}
+	    "5" { set current_connection "4"}
+	    "6" { set current_connection "5"}
+	    default { set current_connection $current_inf}
+	}
+	if {$connection == $current_connection} {
+	    return "\\* $connection"
+	} else {
+	    return "  $connection"
+	}
+    }
+
+    set ws "\[ \t\]+"
+    global decimal binfile
+
+    # Test "info connections" and "info inferior" by switching to each
+    # inferior one by one.
+    for {set inf 1} {$inf <= 6} {incr inf} {
+	gdb_test "inferior $inf" "Switching to inferior $inf.*" \
+	    "inferior switch ($inf)"
+
+	gdb_test "info connections" \
+	    [multi_line \
+		 "Num${ws}What${ws}Description${ws}" \
+		 "[connection_num 1 $inf]${ws}native${ws}Native process${ws}" \
+		 "[connection_num 2 $inf]${ws}extended-remote localhost:$decimal${ws}Extended remote serial target in gdb-specific protocol${ws}" \
+		 "[connection_num 3 $inf]${ws}core${ws}Local core dump file${ws}" \
+		 "[connection_num 4 $inf]${ws}extended-remote localhost:$decimal${ws}Extended remote serial target in gdb-specific protocol${ws}" \
+		 "[connection_num 5 $inf]${ws}core${ws}Local core dump file${ws}" \
+		] "info connections ($inf)"
+
+	gdb_test "info inferiors" \
+	    [multi_line \
+		 "Num${ws}Description${ws}Connection${ws}Executable${ws}" \
+		 "[inf_desc 1 $inf]1 \\(native\\)${ws}${binfile}${ws}" \
+		 "[inf_desc 2 $inf]2 \\(extended-remote localhost:$decimal\\)${ws}${binfile}${ws}" \
+		 "[inf_desc 3 $inf]3 \\(core\\)${ws}${binfile}${ws}" \
+		 "[inf_desc 4 $inf]1 \\(native\\)${ws}${binfile}${ws}" \
+		 "[inf_desc 5 $inf]4 \\(extended-remote localhost:$decimal\\)${ws}${binfile}${ws}" \
+		 "[inf_desc 6 $inf]5 \\(core\\)${ws}${binfile}${ws}" \
+		] "info inferiors ($inf)"
+    }
+}
+
 # Make a core file with two threads upfront.  Several tests load the
 # same core file.
 prepare_core
@@ -385,3 +435,10 @@ with_test_prefix "interrupt" {
 with_test_prefix "ping-pong" {
     test_ping_pong_next
 }
+
+# Test "info inferiors" and "info connections" commands
+with_test_prefix "info-inferiors" {
+    foreach_with_prefix multi_process {"on" "off"} {
+	test_info_inferiors $multi_process
+    }
+}
-- 
2.17.1



Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
Pedro Alves Jan. 10, 2020, 8:13 p.m. | #9
I've now merged the multi-target work to master, including
the couple follow up patches developed and discussed on this
thread.

Massive thank you to everyone involved, for reviews,
discussions, groundwork, patches, and whatnot!

Thanks,
Pedro Alves
Kevin Buettner via Gdb-patches Aug. 4, 2020, 3:30 a.m. | #10
On Fri, 10 Jan 2020 20:13:22 +0000
Pedro Alves <palves@redhat.com> wrote:

> I've now merged the multi-target work to master, including

> the couple follow up patches developed and discussed on this

> thread.


I've run into a regression stemming from this commit:

5b6d1e4fa4 (HEAD, refs/bisect/bad) Multi-target support

More info in Bug 26336:

https://sourceware.org/bugzilla/show_bug.cgi?id=26336

(I thought at first that I introduced this problem with my recent
core file work, but realized that was not the case after I disabled
it.  I then did a bisection starting from late last year.)

Kevin
Pedro Alves Aug. 6, 2020, 3:16 p.m. | #11
On 8/4/20 4:30 AM, Kevin Buettner via Gdb-patches wrote:
> On Fri, 10 Jan 2020 20:13:22 +0000

> Pedro Alves <palves@redhat.com> wrote:

> 

>> I've now merged the multi-target work to master, including

>> the couple follow up patches developed and discussed on this

>> thread.

> 

> I've run into a regression stemming from this commit:

> 

> 5b6d1e4fa4 (HEAD, refs/bisect/bad) Multi-target support

> 

> More info in Bug 26336:

> 

> https://sourceware.org/bugzilla/show_bug.cgi?id=26336

> 

> (I thought at first that I introduced this problem with my recent

> core file work, but realized that was not the case after I disabled

> it.  I then did a bisection starting from late last year.)


Thanks for the bisect and the initial analysis.  This patch fixes
it for me.  No regressions for me on either -m32 nor -m64, and
the corefile.exp regression is fixed.  Let me know what you think.

From a82de99c678920ba3f7a296419a8aaa376c92d98 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>

Date: Thu, 6 Aug 2020 13:05:02 +0100
Subject: [PATCH] gdb.base/corefile.exp regression for unix/-m32 on x86_64 (PR
 26336)

gdb.base/corefile.exp is showing an unexpected failure and an
unresolved testcase when testing against unix/-m32:

 (gdb) PASS: gdb.base/corefile.exp: attach: sanity check we see the core file
 attach 15741
 gdb/dwarf2-frame.c:1009: internal-error: dwarf2_frame_cache* dwarf2_frame_cache(frame_info*, void**): Assertion `fde != NULL' failed.
 A problem internal to GDB has been detected,
 further debugging may prove unreliable.
 Quit this debugging session? (y or n) FAIL: gdb.base/corefile.exp: attach: with core (GDB internal error)
 Resyncing due to internal error.

This regressed with:

 From 5b6d1e4fa4fc6827c7b3f0e99ff120dfa14d65d2 Mon Sep 17 00:00:00 2001
 From: Pedro Alves <palves@redhat.com>

 Date: Fri, 10 Jan 2020 20:06:08 +0000
 Subject: [PATCH] Multi-target support

The assertion is here:

 #0  internal_error (file=0xbffffccb0 <error: Cannot access memory at address 0xbffffccb0>, line=0, fmt=0x555556327320 "en_US.UTF-8") at sr
 c/gdbsupport/errors.cc:51
 #1  0x00005555557d4e45 in dwarf2_frame_cache (this_frame=0x55555672f950, this_cache=0x55555672f968) at src/gdb/dwarf2/frame.c:1013
 #2  0x00005555557d5886 in dwarf2_frame_this_id (this_frame=0x55555672f950, this_cache=0x55555672f968, this_id=0x55555672f9b0) at src/gdb/d
 warf2/frame.c:1226
 #3  0x00005555558b184e in compute_frame_id (fi=0x55555672f950) at src/gdb/frame.c:558
 #4  0x00005555558b19b2 in get_frame_id (fi=0x55555672f950) at src/gdb/frame.c:588
 #5  0x0000555555bda338 in scoped_restore_current_thread::scoped_restore_current_thread (this=0x7fffffffd0d8) at src/gdb/thread.c:1458
 #6  0x00005555556ce41f in scoped_restore_current_pspace_and_thread::scoped_restore_current_pspace_and_thread (During symbol reading: .debug_line address at offset 0x1db2d3
 is 0 [in module /home/pedro/gdb/cascais-builds/binutils-gdb/gdb/gdb]
 this=0x7fffffffd0d0) at src/gdb/progspace-and-thread.h:29
 #7  0x0000555555898ea6 in remove_target_sections (owner=0x555556935550) at src/gdb/exec.c:798
 #8  0x0000555555b700b6 in symfile_free_objfile (objfile=0x555556935550) at src/gdb/symfile.c:3742
 #9  0x000055555565050e in std::_Function_handler<void (objfile*), void (*)(objfile*)>::_M_invoke(std::_Any_data const&, objfile*&&) (__functor=..., __args#0=@0x7fffffffd190
 : 0x555556935550) at /usr/include/c++/9/bits/std_function.h:300
 #10 0x0000555555a3053d in std::function<void (objfile*)>::operator()(objfile*) const (this=0x555556752a20, __args#0=0x555556935550) at /usr/include/c++/9/bits/std_function.
 h:688
 #11 0x0000555555a2ff01 in gdb::observers::observable<objfile*>::notify (this=0x5555562eaa80 <gdb::observers::free_objfile>, args#0=0x555556935550) at /net/cascais.nfs/gdb/b
 inutils-gdb/src/gdb/../gdbsupport/observable.h:106
 #12 0x0000555555a2c56a in objfile::~objfile (this=0x555556935550, __in_chrg=<optimized out>) at src/gdb/objfiles.c:521
 #13 0x0000555555a31d46 in std::_Sp_counted_ptr<objfile*, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x555556c1f6f0) at /usr/include/c++/9/bits/shared_ptr_base.h:377
 #14 0x00005555556d3444 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x555556c1f6f0) at /usr/include/c++/9/bits/shared_ptr_base.h:155
 #15 0x00005555556cec77 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x555556b99ee8, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:730
 #16 0x0000555555a2f8da in std::__shared_ptr<objfile, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x555556b99ee0, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:1169
 #17 0x0000555555a2f8fa in std::shared_ptr<objfile>::~shared_ptr (this=0x555556b99ee0, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr.h:103
 #18 0x0000555555a63fba in __gnu_cxx::new_allocator<std::_List_node<std::shared_ptr<objfile> > >::destroy<std::shared_ptr<objfile> > (this=0x55555679f0c0, __p=0x555556b99ee0) at /usr/include/c++/9/ext/new_allocator.h:153
 #19 0x0000555555a638fb in std::allocator_traits<std::allocator<std::_List_node<std::shared_ptr<objfile> > > >::destroy<std::shared_ptr<objfile> > (__a=..., __p=0x555556b99ee0) at /usr/include/c++/9/bits/alloc_traits.h:497
 #20 0x0000555555a6351c in std::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::_M_erase (this=0x55555679f0c0, __position=std::shared_ptr<objfile> (expired, weak count 1) = {get() = 0x555556935550}) at /usr/include/c++/9/bits/stl_list.h:1921
 #21 0x0000555555a62dab in std::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::erase (this=0x55555679f0c0, __position=std::shared_ptr<objfile> (expired, weak count 1) = {get() = 0x555556935550}) at /usr/include/c++/9/bits/list.tcc:158
 #22 0x0000555555a614dd in program_space::remove_objfile (this=0x55555679f080, objfile=0x555556935550) at src/gdb/progspace.c:207
 #23 0x0000555555a2c4dc in objfile::unlink (this=0x555556935550) at src/gdb/objfiles.c:497
 #24 0x0000555555a2da65 in objfile_purge_solibs () at src/gdb/objfiles.c:904
 #25 0x0000555555b3af74 in no_shared_libraries (ignored=0x0, from_tty=1) at src/gdb/solib.c:1236
 #26 0x0000555555bbafc7 in target_pre_inferior (from_tty=1) at src/gdb/target.c:1900
 #27 0x0000555555940afb in attach_command (args=0x5555563277c7 "15741", from_tty=1) at src/gdb/infcmd.c:2582
 ...


The problem is that the multi-target commit added a
scoped_restore_current_thread to remove_target_sections (frame #7
above).  scoped_restore_current_thread's ctor fetches the selected
frame's frame id.  If the frame had not had its frame id computed yet,
it is computed then (frame #4 above).  Because it has been determined
earlier that the frame's unwinder is the DWARF unwinder, we end up
here:

 static struct dwarf2_frame_cache *
 dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
 {
 ...
   /* Find the correct FDE.  */
   fde = dwarf2_frame_find_fde (&pc1, &cache->per_objfile);
   gdb_assert (fde != NULL);

And, that assertion fails.  The assertion is reasonable, because the
DWARF unwinder only claims the frame if it managed to find the FDE
earlier (in dwarf2_frame_sniffer).

(unix/-m32 is thus really a red herring here -- it's just that on
x86_64 -m64, the frame is not claimed by the DWARF unwinder.)

The reason the assertion is failing, is because the objfile that
contains the FDE has been removed from the objfiles list already when
we get here (frame #22 above).  This suggests that the fix should be
to invalidate DWARF frames when their objfile is removed.  Or to keep
it simple and safe, invalidate the frame cache when an objfile is
removed.  That is what this commit does.

OOC, I checked why is it that when you unload a file with plain "(gdb)
file", we don't hit the assertion.  It must be because we're already
flushing the frame cache somewhere else in that case.  And indeed, we
flush the frame cache here:

 (gdb) bt
 #0  reinit_frame_cache () at src/gdb/frame.c:1857
 #1  0x0000555555ad1ad6 in registers_changed_ptid (target=0x0, ptid=...) at src/gdb/regcache.c:470
 #2  0x0000555555ad1b58 in registers_changed () at src/gdb/regcache.c:485
 #3  0x00005555558d095e in set_target_gdbarch (new_gdbarch=0x555556d5f5b0) at src/gdb/gdbarch.c:5528
 #4  0x0000555555677175 in set_gdbarch_from_file (abfd=0x0) at src/gdb/arch-utils.c:601
 #5  0x0000555555897c6b in exec_file_attach (filename=0x0, from_tty=1) at src/gdb/exec.c:409
 #6  0x000055555589852d in exec_file_command (args=0x0, from_tty=1) at src/gdb/exec.c:571
 #7  0x00005555558985a1 in file_command (arg=0x0, from_tty=1) at src/gdb/exec.c:583
 #8  0x000055555572b55f in do_const_cfunc (c=0x55555672e200, args=0x0, from_tty=1) at src/gdb/cli/cli-decode.c:95
 #9  0x000055555572f3d3 in cmd_func (cmd=0x55555672e200, args=0x0, from_tty=1) at src/gdb/cli/cli-decode.c:2181
 #10 0x0000555555be1ecc in execute_command (p=0x555556327804 "", from_tty=1) at src/gdb/top.c:668
 #11 0x0000555555895427 in command_handler (command=0x555556327800 "file") at src/gdb/event-top.c:588
 #12 0x00005555558958af in command_line_handler (rl=...) at src/gdb/event-top.c:773
 #13 0x0000555555894b3e in gdb_rl_callback_handler (rl=0x55555a09e240 "file") at src/gdb/event-top.c:219
 #14 0x0000555555ccfeec in rl_callback_read_char () at src/readline/readline/callback.c:281
 #15 0x000055555589495a in gdb_rl_callback_read_char_wrapper_noexcept () at src/gdb/event-top.c:177
 #16 0x0000555555894a08 in gdb_rl_callback_read_char_wrapper (client_data=0x555556327520) at src/gdb/event-top.c:194
 #17 0x00005555558952a5 in stdin_event_handler (error=0, client_data=0x555556327520) at src/gdb/event-top.c:516
 #18 0x0000555555e027d6 in handle_file_event (file_ptr=0x555558d20840, ready_mask=1) at src/gdbsupport/event-loop.cc:548
 #19 0x0000555555e02d88 in gdb_wait_for_event (block=1) at src/gdbsupport/event-loop.cc:673
 #20 0x0000555555e01c42 in gdb_do_one_event () at src/gdbsupport/event-loop.cc:215
 #21 0x00005555559c47c2 in start_event_loop () at src/gdb/main.c:356
 #22 0x00005555559c490d in captured_command_loop () at src/gdb/main.c:416
 #23 0x00005555559c6217 in captured_main (data=0x7fffffffdc00) at src/gdb/main.c:1253
 #24 0x00005555559c6289 in gdb_main (args=0x7fffffffdc00) at src/gdb/main.c:1268
 #25 0x0000555555621756 in main (argc=3, argv=0x7fffffffdd18) at src/gdb/gdb.c:32

gdb/ChangeLog:

	* progspace.c (program_space::remove_objfile): Invalidate the
	frame cache.
---
 gdb/progspace.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gdb/progspace.c b/gdb/progspace.c
index a0b14a6d2eb..462083ce1f6 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -198,6 +198,12 @@ program_space::add_objfile (std::shared_ptr<objfile> &&objfile,
 void
 program_space::remove_objfile (struct objfile *objfile)
 {
+  /* Removing an objfile from the objfile list invalidates any frame
+     that was built using frame info found in the objfile.  Reinit the
+     frame cache to get rid of any frame that might otherwise
+     reference stale info.  */
+  reinit_frame_cache ();
+
   auto iter = std::find_if (objfiles_list.begin (), objfiles_list.end (),
 			    [=] (const std::shared_ptr<::objfile> &objf)
 			    {

base-commit: 1a9f72a7a8f445b8d665eb36b053a18e758e63e6
-- 
2.14.5
Tom Tromey Aug. 6, 2020, 5:49 p.m. | #12
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:


Pedro> 	* progspace.c (program_space::remove_objfile): Invalidate the
Pedro> 	frame cache.

Looks good to me.

Tom
Kevin Buettner via Gdb-patches Aug. 7, 2020, 10:43 p.m. | #13
On Thu, 6 Aug 2020 16:16:10 +0100
Pedro Alves <pedro@palves.net> wrote:

> On 8/4/20 4:30 AM, Kevin Buettner via Gdb-patches wrote:

> > On Fri, 10 Jan 2020 20:13:22 +0000

> > Pedro Alves <palves@redhat.com> wrote:

> >   

> >> I've now merged the multi-target work to master, including

> >> the couple follow up patches developed and discussed on this

> >> thread.  

> > 

> > I've run into a regression stemming from this commit:

> > 

> > 5b6d1e4fa4 (HEAD, refs/bisect/bad) Multi-target support

> > 

> > More info in Bug 26336:

> > 

> > https://sourceware.org/bugzilla/show_bug.cgi?id=26336

> > 

> > (I thought at first that I introduced this problem with my recent

> > core file work, but realized that was not the case after I disabled

> > it.  I then did a bisection starting from late last year.)  

> 

> Thanks for the bisect and the initial analysis.  This patch fixes

> it for me.  No regressions for me on either -m32 nor -m64, and

> the corefile.exp regression is fixed.  Let me know what you think.


Your analysis make sense to me and the patch looks good.

I've tested your patch locally and no longer see the corefile.exp
regression when testing w/ x86_64/-m32 (or with just x86_64).

Kevin