gdb: handle_no_resumed: only update thread list of event target
authorSimon Marchi <simon.marchi@efficios.com>
Tue, 12 Apr 2022 02:11:03 +0000 (22:11 -0400)
committerSimon Marchi <simon.marchi@efficios.com>
Fri, 22 Apr 2022 18:13:54 +0000 (14:13 -0400)
commit1e864019e430f2cac5ab6b822c4e1db39bd62699
tree3a34cfa7e027121fb70e9c59a8b2dfbab9c921d0
parentf551c8ef326a022ec44279e66ed9734692225067
gdb: handle_no_resumed: only update thread list of event target

When running:

    $ make check TESTS="gdb.multi/multi-re-run.exp" RUNTESTFLAGS="--target_board=native-gdbserver"

I get:

    target remote localhost:2347^M
    Remote debugging using localhost:2347^M
    Reading symbols from /lib64/ld-linux-x86-64.so.2...^M
    Reading symbols from /usr/lib/debug//lib/x86_64-linux-gnu/ld-2.31.so...^M
    0x00007ffff7fd0100 in _start () from /lib64/ld-linux-x86-64.so.2^M
    (gdb) continue^M
    Continuing.^M
    Cannot execute this command while the target is running.^M
    Use the "interrupt" command to stop the target^M
    and then try again.^M
    (gdb) FAIL: gdb.multi/multi-re-run.exp: re_run_inf=1: iter=1: runto: run to all_started

The test does:

 - Connect to a remote target with inferior 2, continue and stop on the
   all_started function
 - Connect to a separate remote target / GDBserver instance with inferior 1,
   continue and (expect to) stop on the all_started function

The failure seen above happens when trying to continue inferior 1.

What happens is:

 - GDB tells inferior 1's remote target to continue
 - We go into fetch_inferior_event, try to get an event at random from
   the targets
 - do_target_wait happens to pick inferior 2's target
 - That target return TARGET_WAITKIND_NO_RESUMED, which makes sense:
   inferior 2 is stopped, that target has no resumed thread
 - handle_no_resumed tries to update the thread list of all targets:

   for (auto *target : all_non_exited_process_targets ())
     {
       switch_to_target_no_thread (target);
       update_thread_list ();
     }

 - When trying to update the thread list of inferior 1's target, it hits
   the "Cannot execute this command while the target is running" error.
   This target is working in "remote all-stop" mode, and it is currently
   waiting for a stop reply, so it can't send packets to update the
   thread list at this time.

To handle the problem described in the comment in handle_no_resumed, I
don't think it is necessary to update the thread list of all targets,
but only the event target.  That comment describes a kind of race
condition where some target reports a breakpoint hit for a thread and
then its last remaining resumed thread exits, so sends a "no resumed"
event.  If we ended up resuming the thread that hit a breakpoint, we
want to ignore the "no resumed" and carry on.

But I don't really see why we need to update the thread list on the
other targets.  I can't really articulate this, it's more a gut feeling,
maybe I just fail to imagine the situation where this is needed.  But
here is the patch anyway, so we can discuss it.  The patch changes
handle_no_resumed to only update the thread list of the event target.
This fixes the test run shown above.

The way I originally tried to fix this was to make
remote_target::update_thread_list return early if the target is
currently awaiting a stop reply, since there's no way it can update the
thread list at that time.  But that felt more like papering over the
problem.  I then thought that we probably shouldn't be asking the target
to update the thread list unnecessarily.

Change-Id: Ide3df22b4f556478e155ad1c67ad4b4fe7c26a58
gdb/infrun.c