From: Simon Marchi Date: Tue, 8 Jul 2025 20:54:45 +0000 (-0400) Subject: gdb/remote: call target_pre_inferior in remote_target::remote_add_inferior X-Git-Tag: gdb-17-branchpoint~83 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4520d8dd1cf8ef91b090f1af73b48874d6804445;p=thirdparty%2Fbinutils-gdb.git 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 [] ()] 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... 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 7b954b7a887..fb1e88ae678 100644 --- 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