gdb/mi: fix ^running record with multiple MI interpreters

I stumbled on the mi_proceeded and running_result_record_printed
globals, which are shared by all MI interpreter instances (it's unlikely
that people use multiple MI interpreter instances, but it's possible).
After poking at it, I found this bug:

1. Start GDB in MI mode
2. Add a second MI interpreter with the new-ui command
3. Use -exec-run on the second interpreter

This is the output I get on the first interpreter:

    =thread-group-added,id="i1"
    ~"Reading symbols from a.out...\n"
    ~"New UI allocated\n"
    (gdb)
    =thread-group-started,id="i1",pid="94718"
    =thread-created,id="1",group-id="i1"
    ^running
    *running,thread-id="all"

And this is the output I get on the second intepreter:

    =thread-group-added,id="i1"
    (gdb)
    -exec-run
    =thread-group-started,id="i1",pid="94718"
    =thread-created,id="1",group-id="i1"
    *running,thread-id="all"

The problem here is that the `^running` reply to the -exec-run command
is printed on the wrong UI.  It is printed on the first one, it should
be printed on the second (the one on which we sent the -exec-run).

What happens under the hood is that captured_mi_execute_command, while
executing a command for the second intepreter, clears the
running_result_record_printed and mi_proceeded globals.
mi_about_to_proceed then sets mi_proceeded.  Then, mi_on_resume_1 gets
called for the first intepreter first.  Since the

    !running_result_record_printed && mi_proceeded

condition is true, it prints a ^running, and sets
running_result_record_printed.  When mi_on_resume_1 gets called for the
second interpreter, running_result_record_printed is already set, so
^running is not printed there.

It took me a while to understand the relationship between these two
variables.  I think that in the end, this is what we want to track:

 1. When executing an MI command, take note if that command causes a
    "proceed".  This is done in mi_about_to_proceed.
 2. In mi_on_resume_1, if the command indeed caused a "proceed", we want
    to output a ^running record.  And we want to remember that we did,
    because...
 3. Back in captured_mi_execute_command, if we did not output a
    ^running, we want to output a ^done.

Moving those two variables to the mi_interp struture appears to fix it.
Only for the interpreter doing the -exec-run command does the
running_result_record_printed flag get cleared, and therefore only or
that one does the ^running record get printed.

Add a new test for this, that does pretty much what the reproducer above
shows.  Without the fix, the test fails because
mi_send_resuming_command_raw never sees the ^running record.

Change-Id: I63ea30e6cb61a8e1dd5ef03377e6003381a9209b
Tested-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
This commit is contained in:
Simon Marchi 2023-04-21 12:08:42 -04:00
parent 417bfaa9b5
commit f818c32ba4
7 changed files with 111 additions and 25 deletions

View file

@ -677,7 +677,12 @@ mi_about_to_proceed (void)
return;
}
mi_proceeded = 1;
mi_interp *mi = as_mi_interp (top_level_interpreter ());
if (mi == nullptr)
return;
mi->mi_proceeded = 1;
}
/* When the element is non-zero, no MI notifications will be emitted in
@ -961,7 +966,7 @@ mi_on_resume_1 (struct mi_interp *mi,
will make it impossible for frontend to know what's going on.
In future (MI3), we'll be outputting "^done" here. */
if (!running_result_record_printed && mi_proceeded)
if (!mi->running_result_record_printed && mi->mi_proceeded)
{
gdb_printf (mi->raw_stdout, "%s^running\n",
current_token ? current_token : "");
@ -977,9 +982,9 @@ mi_on_resume_1 (struct mi_interp *mi,
for (thread_info *tp : all_non_exited_threads (targ, ptid))
mi_output_running (tp);
if (!running_result_record_printed && mi_proceeded)
if (!mi->running_result_record_printed && mi->mi_proceeded)
{
running_result_record_printed = 1;
mi->running_result_record_printed = 1;
/* This is what gdb used to do historically -- printing prompt
even if it cannot actually accept any input. This will be
surely removed for MI3, and may be removed even earlier. */

View file

@ -64,6 +64,12 @@ public:
/* MI's CLI builder (wraps OUT). */
struct ui_out *cli_uiout;
int running_result_record_printed = 1;
/* Flag indicating that the target has proceeded since the last
command was issued. */
int mi_proceeded;
};
/* Output the shared object attributes to UIOUT. */

View file

@ -84,12 +84,6 @@ char *current_token;
command including all option, and make it possible. */
static struct mi_parse *current_context;
int running_result_record_printed = 1;
/* Flag indicating that the target has proceeded since the last
command was issued. */
int mi_proceeded;
static void mi_cmd_execute (struct mi_parse *parse);
static void mi_execute_async_cli_command (const char *cli_command,
@ -1818,8 +1812,8 @@ captured_mi_execute_command (struct ui_out *uiout, struct mi_parse *context)
scoped_restore save_token = make_scoped_restore (&current_token,
context->token);
running_result_record_printed = 0;
mi_proceeded = 0;
mi->running_result_record_printed = 0;
mi->mi_proceeded = 0;
switch (context->op)
{
case MI_COMMAND:
@ -1837,7 +1831,7 @@ captured_mi_execute_command (struct ui_out *uiout, struct mi_parse *context)
to directly use the mi_interp's uiout, since the command
could have reset the interpreter, in which case the current
uiout will most likely crash in the mi_out_* routines. */
if (!running_result_record_printed)
if (!mi->running_result_record_printed)
{
gdb_puts (context->token, mi->raw_stdout);
/* There's no particularly good reason why target-connect results
@ -1876,7 +1870,7 @@ captured_mi_execute_command (struct ui_out *uiout, struct mi_parse *context)
|| current_interp_named_p (INTERP_MI3)
|| current_interp_named_p (INTERP_MI4))
{
if (!running_result_record_printed)
if (!mi->running_result_record_printed)
{
gdb_puts (context->token, mi->raw_stdout);
gdb_puts ("^done", mi->raw_stdout);

View file

@ -36,9 +36,6 @@ extern int mi_async_p (void);
extern char *current_token;
extern int running_result_record_printed;
extern int mi_proceeded;
struct mi_suppress_notification
{
/* Breakpoint notification suppressed? */

View file

@ -0,0 +1,7 @@
#include <unistd.h>
int
main (void)
{
sleep (1234);
}

View file

@ -0,0 +1,67 @@
# Copyright 2023 Free Software Foundation, Inc.
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
# Test doing an -exec-run while there are two MI UIs.
load_lib mi-support.exp
standard_testfile
if {[build_executable $testfile.exp $testfile ${srcfile} "debug"] == -1} {
untested "failed to compile"
return
}
# Run one configuration of the test.
#
# UI_TO_RUN is the UI that should issue the run command.
proc do_test { ui_to_run } {
if {[mi_clean_restart $::binfile "separate-mi-tty"] != 0} {
fail "could not start gdb"
return
}
with_spawn_id $::gdb_main_spawn_id {
lassign [create_mi_ui] second_mi_spawn_id second_mi_tty_name
}
with_spawn_id $second_mi_spawn_id {
gdb_expect {
-re "=thread-group-added,id=\"i1\"\r\n$::mi_gdb_prompt$" {
pass "consume"
}
}
}
if { $ui_to_run == "first" } {
set spawn_id_to_run $::mi_spawn_id
} elseif { $ui_to_run == "second" } {
set spawn_id_to_run $second_mi_spawn_id
} else {
error "invalid ui_to_run value"
}
with_spawn_id $spawn_id_to_run {
# mi_runto_main implicitly verifies that the UI doing the -exec-run gets
# the expected ^running record.
mi_runto_main
}
}
foreach_with_prefix ui_to_run {first second} {
do_test $ui_to_run
}

View file

@ -131,6 +131,21 @@ proc mi_create_inferior_pty {} {
}
}
# Create a new pty, and reate a new MI UI (using the new-ui command) on it.
#
# Return a list with the spawn id for that pty and the pty file name.
proc create_mi_ui {} {
spawn -pty
set tty_name $spawn_out(slave,name)
gdb_test_multiple "new-ui mi $tty_name" "new-ui" {
-re "New UI allocated\r\n$::gdb_prompt $" {
}
}
return [list $spawn_id $tty_name]
}
#
# Like default_mi_gdb_start below, but the MI is created as a separate
# ui in a new tty. The global MI_SPAWN_ID is updated to point at the
@ -154,13 +169,7 @@ proc mi_gdb_start_separate_mi_tty { { flags {} } } {
gdb_start
# Create the new PTY for the MI UI.
spawn -pty
set mi_spawn_id $spawn_id
set mi_tty_name $spawn_out(slave,name)
gdb_test_multiple "new-ui mi $mi_tty_name" "new-ui" {
-re "New UI allocated\r\n$gdb_prompt $" {
}
}
lassign [create_mi_ui] mi_spawn_id mi_tty_name
# Switch to the MI channel.
set gdb_main_spawn_id $gdb_spawn_id
@ -822,7 +831,7 @@ proc mi_gdb_test { args } {
fail "$errmsg"
return -1
}
-re ".*$mi_gdb_prompt\[ \]*$" {
-re "(.*$mi_gdb_prompt\[ \]*)$" {
if {![string match "" $message]} {
fail "$message (unexpected output)"
}
@ -1082,6 +1091,7 @@ proc mi_runto_helper {func run_or_continue args} {
# file.", etc. to the CLI stream.
set extra_output "&\"\[^\r\n\]+\"\r\n"
}
mi_gdb_test "200-break-insert [join $extra_opts " "] -t $func" "${extra_output}200\\^done,$bp" \
"breakpoint at $func"