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; 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 /* 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. will make it impossible for frontend to know what's going on.
In future (MI3), we'll be outputting "^done" here. */ 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", gdb_printf (mi->raw_stdout, "%s^running\n",
current_token ? current_token : ""); 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)) for (thread_info *tp : all_non_exited_threads (targ, ptid))
mi_output_running (tp); 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 /* This is what gdb used to do historically -- printing prompt
even if it cannot actually accept any input. This will be even if it cannot actually accept any input. This will be
surely removed for MI3, and may be removed even earlier. */ surely removed for MI3, and may be removed even earlier. */

View file

@ -64,6 +64,12 @@ public:
/* MI's CLI builder (wraps OUT). */ /* MI's CLI builder (wraps OUT). */
struct ui_out *cli_uiout; 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. */ /* Output the shared object attributes to UIOUT. */

View file

@ -84,12 +84,6 @@ char *current_token;
command including all option, and make it possible. */ command including all option, and make it possible. */
static struct mi_parse *current_context; 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_cmd_execute (struct mi_parse *parse);
static void mi_execute_async_cli_command (const char *cli_command, 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, scoped_restore save_token = make_scoped_restore (&current_token,
context->token); context->token);
running_result_record_printed = 0; mi->running_result_record_printed = 0;
mi_proceeded = 0; mi->mi_proceeded = 0;
switch (context->op) switch (context->op)
{ {
case MI_COMMAND: 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 to directly use the mi_interp's uiout, since the command
could have reset the interpreter, in which case the current could have reset the interpreter, in which case the current
uiout will most likely crash in the mi_out_* routines. */ 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); gdb_puts (context->token, mi->raw_stdout);
/* There's no particularly good reason why target-connect results /* 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_MI3)
|| current_interp_named_p (INTERP_MI4)) || 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 (context->token, mi->raw_stdout);
gdb_puts ("^done", 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 char *current_token;
extern int running_result_record_printed;
extern int mi_proceeded;
struct mi_suppress_notification struct mi_suppress_notification
{ {
/* Breakpoint notification suppressed? */ /* 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 # 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 # 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 gdb_start
# Create the new PTY for the MI UI. # Create the new PTY for the MI UI.
spawn -pty lassign [create_mi_ui] mi_spawn_id mi_tty_name
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 $" {
}
}
# Switch to the MI channel. # Switch to the MI channel.
set gdb_main_spawn_id $gdb_spawn_id set gdb_main_spawn_id $gdb_spawn_id
@ -822,7 +831,7 @@ proc mi_gdb_test { args } {
fail "$errmsg" fail "$errmsg"
return -1 return -1
} }
-re ".*$mi_gdb_prompt\[ \]*$" { -re "(.*$mi_gdb_prompt\[ \]*)$" {
if {![string match "" $message]} { if {![string match "" $message]} {
fail "$message (unexpected output)" fail "$message (unexpected output)"
} }
@ -1082,6 +1091,7 @@ proc mi_runto_helper {func run_or_continue args} {
# file.", etc. to the CLI stream. # file.", etc. to the CLI stream.
set extra_output "&\"\[^\r\n\]+\"\r\n" set extra_output "&\"\[^\r\n\]+\"\r\n"
} }
mi_gdb_test "200-break-insert [join $extra_opts " "] -t $func" "${extra_output}200\\^done,$bp" \ mi_gdb_test "200-break-insert [join $extra_opts " "] -t $func" "${extra_output}200\\^done,$bp" \
"breakpoint at $func" "breakpoint at $func"