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:
parent
417bfaa9b5
commit
f818c32ba4
7 changed files with 111 additions and 25 deletions
|
@ -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. */
|
||||||
|
|
|
@ -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. */
|
||||||
|
|
|
@ -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 (¤t_token,
|
scoped_restore save_token = make_scoped_restore (¤t_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);
|
||||||
|
|
|
@ -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? */
|
||||||
|
|
7
gdb/testsuite/gdb.mi/run-with-two-mi-uis.c
Normal file
7
gdb/testsuite/gdb.mi/run-with-two-mi-uis.c
Normal file
|
@ -0,0 +1,7 @@
|
||||||
|
#include <unistd.h>
|
||||||
|
|
||||||
|
int
|
||||||
|
main (void)
|
||||||
|
{
|
||||||
|
sleep (1234);
|
||||||
|
}
|
67
gdb/testsuite/gdb.mi/run-with-two-mi-uis.exp
Normal file
67
gdb/testsuite/gdb.mi/run-with-two-mi-uis.exp
Normal 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
|
||||||
|
}
|
||||||
|
|
|
@ -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"
|
||||||
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue