public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <andrew.burgess@embecosm.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <andrew.burgess@embecosm.com>
Subject: [PATCHv2 2/2] gdb: Clean up inferior list when reconnecting to new target
Date: Wed, 01 Aug 2018 21:19:00 -0000	[thread overview]
Message-ID: <c8da3f591e5e274592d77438251b43c90e1cd748.1533158307.git.andrew.burgess@embecosm.com> (raw)
In-Reply-To: <cover.1533158307.git.andrew.burgess@embecosm.com>
In-Reply-To: <cover.1533158307.git.andrew.burgess@embecosm.com>

When connected to a target and running an inferior, if the inferior
forks then a new, inferior is created to track the child
process. Usually the second inferior (the child process) will be
automatically cleaned up (removed) by GDB once the child has completed.
However, if we connect to a new target then the second inferior is left
around, and we can even end up using the second inferior in order to run
a program on the new target, for example in this session:

   (gdb) target extended-remote :2347
   (gdb) file /path/to/exe
   (gdb) set remote exec-file /path/to/exe
   (gdb) set detach-on-fork off
   (gdb) break breakpt
   (gdb) run
   # ... hits breakpoint
   (gdb) info inferiors
     Num  Description       Executable
   * 1    process 15406     /path/to/exe
     2    process 15407     /path/to/exe
   (gdb) kill
   Kill the program being debugged? (y or n) y
   (gdb) info inferiors
     Num  Description       Executable
   * 1    <null>            /path/to/exe
     2    process 15433     /path/to/exe
   (gdb) target extended-remote :2348
   (gdb) file /path/to/exe
   (gdb) set remote exec-file /path/to/exe
   (gdb) run
   # ... hits breakpoint
   (gdb) info inferiors
     Num  Description       Executable
     1    <null>            /path/to/exe
   * 2    process 15408     /path/to/exe
     3    process 15409     /path/to/exe
   (gdb)

Notice how after connecting to the second extended-remote target, and
then running the test program we now have inferiors 1, 2, and 3.
There's nothing really _wrong_ with this, but a better experience would,
I think, be to have only inferiors 1 and 2 in the new target's session.

The issue here is that in target.c:dispose_inferior GDB uses
switch_to_thread, which also switches the current inferior.  This leaves
the last inferior in the list selected after target.c:target_preopen has
completed.

We could change target.c:dispose_inferior to ensure the selected
inferior is not left changed, however, in the above example, I think
that, even if the user manually selected inferior 2 _before_ connecting
to the new target, the best behaviour would still be for GDB to switch
back to inferior 1.  This seems to suggest that the right thing to do is
have GDB _choose_ a new inferior as part of target_preopen, after which
is can call prune_inferiors to remove transient inferiors from the list.

An existing test is extended to cover this behaviour.

gdb/ChangeLog:

	* target.c (find_first_killed_inferior): New function.
	(target_preopen): Use find_first_killed_inferior to reset selected
	inferior, and prune killed inferior as appropriate.

gdb/testsuite/ChangeLog:

	* gdb.base/extended-remote-restart.exp: Extend.
---
 gdb/ChangeLog                                      |  6 ++++
 gdb/target.c                                       | 35 ++++++++++++++++++++++
 gdb/testsuite/ChangeLog                            |  4 +++
 gdb/testsuite/gdb.base/extended-remote-restart.exp |  6 ++++
 4 files changed, 51 insertions(+)

diff --git a/gdb/target.c b/gdb/target.c
index f352e0d8283..d6b0bf42271 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2043,6 +2043,31 @@ dispose_inferior (struct inferior *inf, void *args)
   return 0;
 }
 
+/* Callback for iterate_over_inferiors.  Record in ARG the first killed
+   inferior.  If we find an inferior that is both killed and not removable,
+   this is returned in preference.  The definition of "first" here is
+   pretty loose, and depends on the order in the inferior list.  */
+
+static int
+find_first_killed_inferior (struct inferior *inf, void *arg)
+{
+  struct inferior **infp = (struct inferior **) arg;
+
+  if (inf->pid == 0)
+    {
+      if (!inf->removable)
+	{
+	  *infp = inf;
+	  return 1;
+	}
+      else if (*infp == nullptr)
+	*infp = inf;
+    }
+
+  return 0;
+}
+
+
 /* This is to be called by the open routine before it does
    anything.  */
 
@@ -2059,6 +2084,16 @@ target_preopen (int from_tty)
 	iterate_over_inferiors (dispose_inferior, NULL);
       else
 	error (_("Program not killed."));
+
+      /* The call to DISPOSE_INFERIOR will leave the last inferior we
+	 killed selected.  Reset the selection to the earliest inferior
+	 that is killed and not removable.  The prune any other killed
+	 inferiors.  */
+      struct inferior *inf = nullptr;
+      iterate_over_inferiors (find_first_killed_inferior, &inf);
+      if (inf != nullptr)
+	set_current_inferior (inf);
+      prune_inferiors ();
     }
 
   /* Calling target_kill may remove the target from the stack.  But if
diff --git a/gdb/testsuite/gdb.base/extended-remote-restart.exp b/gdb/testsuite/gdb.base/extended-remote-restart.exp
index 39fcb9e2e58..fb4bdd1755c 100644
--- a/gdb/testsuite/gdb.base/extended-remote-restart.exp
+++ b/gdb/testsuite/gdb.base/extended-remote-restart.exp
@@ -119,6 +119,12 @@ proc test_reload { do_kill_p follow_child_p } {
     } else {
 	fail "reconnect after fork"
     }
+
+    gdb_test "info inferiors" \
+	[multi_line \
+	     "  Num  Description       Executable.*" \
+	     "\\* 1 +${dead_inf_ptn} \[^\r\n\]+" ] \
+	"Check inferiors after reconnect"
 }
 
 # Run all combinations of the test.
-- 
2.14.4

  parent reply	other threads:[~2018-08-01 21:19 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-25 13:09 [PATCH 0/2] Issue reconnecting to remote target afte fork Andrew Burgess
2018-06-25 13:09 ` [PATCH 1/2] gdb: Fix assert for extended-remote target Andrew Burgess
2018-06-25 13:09 ` [PATCH 2/2] gdb: Clean up inferior list when reconnecting to new target Andrew Burgess
2018-07-17 17:47 ` [PATCH 0/2] Issue reconnecting to remote target afte fork Andrew Burgess
2018-08-01 21:19   ` PING: [PATCHv2 " Andrew Burgess
2018-08-01 21:19   ` Andrew Burgess [this message]
2018-08-08 12:31     ` [PATCHv2 2/2] gdb: Clean up inferior list when reconnecting to new target Andrew Burgess
2018-08-29 10:46       ` PING: " Andrew Burgess
2018-09-13 10:52         ` Andrew Burgess
2018-09-14 13:36     ` Pedro Alves
2018-08-01 21:19   ` [PATCHv2 1/2] gdb: Fix assert for extended-remote target Andrew Burgess
2018-08-02 19:21     ` Tom Tromey
2018-08-08 12:23       ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c8da3f591e5e274592d77438251b43c90e1cd748.1533158307.git.andrew.burgess@embecosm.com \
    --to=andrew.burgess@embecosm.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).