gdb/remote: call target_pre_inferior in remote_target::remote_add_inferior

Since commit 3cb6bc13e328 ("gdb/progspace: add solib_ops pointer in
program_space"), and with the previous patch applied ("gdb/remote: use
scoped_restore_current_program_space in remote_unpush_target"), we get
this failure:

    $ make check TESTS="gdb.server/extended-remote-restart.exp" RUNTESTFLAGS="--target_board=native-extended-gdbserver"

In gdb.log:

    (gdb) PASS: gdb.server/extended-remote-restart.exp: kill: 0, follow-child 1: disconnect
    target extended-remote localhost:2348
    Remote debugging using localhost:2348
    /home/smarchi/src/binutils-gdb/gdb/progspace.h:240: internal-error: set_solib_ops: Assertion `m_solib_ops == nullptr' failed.

When connecting to a remote that has one or more inferior already
running, the remote target (the GDB-side code) tries to re-use existing
GDB inferiors that are unused.  The problem is that the program space of
the inferior that gets re-used unexpectedly has its solib_ops set.

I think that the problem is that when connecting to a remote target that
has multiple inferiors, target_pre_inferior only gets called for the
currently selected (client-side) inferior.  It happens here:

    #0  target_pre_inferior () at /home/smarchi/src/wt/amd/gdb/target.c:2454
    #1  0x0000559c832a350a in target_preopen (from_tty=1) at /home/smarchi/src/wt/amd/gdb/target.c:2510
    #2  0x0000559c82e1b8f1 in remote_target::open_1 (name=0x50200006eb58 ":2345", from_tty=1, extended_p=1) at /home/smarchi/src/wt/amd/gdb/remote.c:6171
    #3  0x0000559c82e18a5d in extended_remote_target::open (name=0x50200006eb58 ":2345", from_tty=1) at /home/smarchi/src/wt/amd/gdb/remote.c:5446
    #4  0x0000559c8329a43e in open_target (args=0x50200006eb58 ":2345", from_tty=1, command=0x512000072c40) at /home/smarchi/src/wt/amd/gdb/target.c:839

I think that target_pre_inferior should be called for the other
inferiors that gain execution as a result of connecting to the remote
target, to make sure inferior or program space-specific data from
previous executions gets cleared.  target_pre_inferior is what clears
any previous solib_ops.

It is possible to observe the problem by adding this print in
target_pre_inferior:

  printf (">>> target_pre_inferior called for inferior %d\n",
          current_inferior ()->num);

Then, starting a gdbserver:

    $ gdbserver --multi localhost:2345

Then, this gdb command that starts two remote inferiors, disconnects
(leaving gdbserver and the inferiors running), then reconnects:

    $ ./gdb -nx --data-directory=data-directory -q \
	  -ex 'set sysroot /' \
	  -ex 'target extended-remote :2345' \
	  -ex 'file testsuite/outputs/gdb.server/extended-remote-restart/extended-remote-restart' \
	  -ex 'set remote exec-file testsuite/outputs/gdb.server/extended-remote-restart/extended-remote-restart' \
	  -ex 'b main' \
	  -ex r \
	  -ex 'add-inferior' \
	  -ex 'inferior 2' \
	  -ex 'file testsuite/outputs/gdb.server/extended-remote-restart/extended-remote-restart' \
	  -ex 'run' \
	  -ex 'inferior 1' \
	  -ex 'disconnect' \
	  -ex 'echo About to reconnect\n' \
	  -ex 'target extended-remote :2345'
    >>> target_pre_inferior called for inferior 1
    Remote debugging using :2345
    Reading symbols from /home/smarchi/build/wt/amd/gdb/testsuite/outputs/gdb.server/extended-remote-restart/extended-remote-restart...
    Breakpoint 1 at 0x11fc: file /home/smarchi/src/wt/amd/gdb/testsuite/gdb.server/extended-remote-restart.c, line 50.
    >>> target_pre_inferior called for inferior 1
    Starting program: /home/smarchi/build/wt/amd/gdb/testsuite/outputs/gdb.server/extended-remote-restart/extended-remote-restart

    Breakpoint 1, main () at /home/smarchi/src/wt/amd/gdb/testsuite/gdb.server/extended-remote-restart.c:50
    50        pid = fork ();
    [New inferior 2]
    Added inferior 2 on connection 1 (extended-remote :2345)
    [Switching to inferior 2 [<null>] (<noexec>)]
    Reading symbols from /home/smarchi/build/wt/amd/gdb/testsuite/outputs/gdb.server/extended-remote-restart/extended-remote-restart...
    >>> target_pre_inferior called for inferior 2
    Starting program: /home/smarchi/build/wt/amd/gdb/testsuite/outputs/gdb.server/extended-remote-restart/extended-remote-restart

    Thread 2.1 "extended-remote" hit Breakpoint 1.2, main () at /home/smarchi/src/wt/amd/gdb/testsuite/gdb.server/extended-remote-restart.c:50
    50        pid = fork ();
    [Switching to inferior 1 [process 2591936] (/home/smarchi/build/wt/amd/gdb/testsuite/outputs/gdb.server/extended-remote-restart/extended-remote-restart)]
    [Switching to thread 1.1 (Thread 2591936.2591936)]
    #0  main () at /home/smarchi/src/wt/amd/gdb/testsuite/gdb.server/extended-remote-restart.c:50
    50        pid = fork ();
    Ending remote debugging.
    About to reconnect
    >>> target_pre_inferior called for inferior 1
    Remote debugging using :2345
    main () at /home/smarchi/src/wt/amd/gdb/testsuite/gdb.server/extended-remote-restart.c:50
    50        pid = fork ();

We can see that target_pre_inferior is only called for inferior 1 when
reconnecting (after the "About to reconnect" message).

After adding the call to target_pre_inferior in remote_add_inferior, we
get (just the last bit):

    About to reconnect
    >>> target_pre_inferior called for inferior 1
    Remote debugging using :2345
    >>> target_pre_inferior called for inferior 1
    >>> target_pre_inferior called for inferior 2
    Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...
    (No debugging symbols found in /lib/x86_64-linux-gnu/libc.so.6)
    Reading symbols from /lib64/ld-linux-x86-64.so.2...
    (No debugging symbols found in /lib64/ld-linux-x86-64.so.2)
    main () at /home/smarchi/src/wt/amd/gdb/testsuite/gdb.server/extended-remote-restart.c:50
    50        pid = fork ();

The duplicate calls to target_pre_inferior for inferior 1 are due to the
existing call in target_preopen.  It might be possible to get rid of it:
with the call I added in remote_target::remote_add_inferior, I presume
it's now unnecessary for the remote target to have the call in
target_preopen as well.  But since target_preopen is used by other
targets, I prefer to leave it there to be safe, for the moment.  Calling
target_pre_inferior multiple times should not be a problem, as it should
be idempotent.

However, once I added that, test gdb.server/stop-reply-no-thread.exp
started failing, with this in the logs:

    target remote localhost:2347
    Remote debugging using localhost:2347
    Remote 'g' packet reply is too long (expected 560 bytes, got 820 bytes): 000000... <truncated>

It became apparent that the new call to target_pre_inferior would wipe a
previously fetched target description.  I fixed that by adding calls to
target_find_description in two callers of remote_add_inferior.  I'm not
100% sure of what I'm doing here, but it seems somewhat correct that
when we map a remote inferior to an existing client-side inferior, we
wipe out any previous target description (which would have been left by
a previous execution) and fetch a new one.

The other call to remote_add_inferior is in
extended_remote_target::attach, where there is already a call to
target_find_description shortly after.

Change-Id: I85426bfff286a67d5fb74bbf978df80060ee6deb

diff --git a/gdb/remote.c b/gdb/remote.c
index 7b954b7a887e2bfed944536da86ed736eae2a678..fb1e88ae67842e8562e5c6a6542a2815b20ad445 100644 (file)
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -2904,6 +2904,10 @@ remote_target::remote_add_inferior (bool fake_pid_p, int pid, int attached,
          inf = add_inferior_with_spaces ();
        }
       switch_to_inferior_no_thread (inf);
+
+      /* Clear any data left by previous executions.  */
+      target_pre_inferior ();
+
       inf->push_target (this);
       inferior_appeared (inf, pid);
     }
@@ -3029,6 +3033,12 @@ remote_target::remote_notice_new_inferior (ptid_t currthread, bool executing)
 
          inf = remote_add_inferior (fake_pid_p,
                                     currthread.pid (), -1, 1);
+
+         /* Fetch the target description for this inferior.  Make sure to
+            leave the currently selected inferior unchanged.  */
+         scoped_restore_current_thread restore_thread;
+         switch_to_inferior_no_thread (inf);
+         target_find_description ();
        }
 
       /* This is really a new thread.  Add it.  */
@@ -4869,7 +4879,11 @@ remote_target::add_current_inferior_and_thread (const char *wait_status)
       fake_pid_p = true;
     }
 
-  remote_add_inferior (fake_pid_p, curr_ptid.pid (), -1, 1);
+  const auto inf = remote_add_inferior (fake_pid_p, curr_ptid.pid (), -1, 1);
+  switch_to_inferior_no_thread (inf);
+
+  /* Fetch the target description for this inferior.  */
+  target_find_description ();
 
   /* Add the main thread and switch to it.  Don't try reading
      registers yet, since we haven't fetched the target description