From 1e864019e430f2cac5ab6b822c4e1db39bd62699 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 11 Apr 2022 22:11:03 -0400 Subject: [PATCH] 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 | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index 15051403e3c..4e7ca803c79 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -5240,12 +5240,7 @@ handle_no_resumed (struct execution_control_state *ecs) inferior *curr_inf = current_inferior (); scoped_restore_current_thread restore_thread; - - for (auto *target : all_non_exited_process_targets ()) - { - switch_to_target_no_thread (target); - update_thread_list (); - } + update_thread_list (); /* If: -- 2.30.2