Skip to content

Commit

Permalink
gdb/mi: fix breakpoint script field output
Browse files Browse the repository at this point in the history
The "script" field, output whenever information about a breakpoint with
commands is output, uses wrong MI syntax.

    $ ./gdb -nx -q --data-directory=data-directory -x script -i mi
    =thread-group-added,id="i1"
    =breakpoint-created,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x000000000000111d",func="main",file="test.c",fullname="/home/simark/build/binutils-gdb-one-target/gdb/test.c",line="3",thread-groups=["i1"],times="0",original-location="main"}
    =breakpoint-modified,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x000000000000111d",func="main",file="test.c",fullname="/home/simark/build/binutils-gdb-one-target/gdb/test.c",line="3",thread-groups=["i1"],times="0",script={"aaa","bbb","ccc"},original-location="main"}
    (gdb)
    -break-info
    ^done,BreakpointTable={nr_rows="1",nr_cols="6",hdr=[{width="7",alignment="-1",col_name="number",colhdr="Num"},{width="14",alignment="-1",col_name="type",colhdr="Type"},{width="4",alignment="-1",col_name="disp",colhdr="Disp"},{width="3",alignment="-1",col_name="enabled",colhdr="Enb"},{width="18",alignment="-1",col_name="addr",colhdr="Address"},{width="40",alignment="2",col_name="what",colhdr="What"}],body=[bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x000000000000111d",func="main",file="test.c",fullname="/home/simark/build/binutils-gdb-one-target/gdb/test.c",line="3",thread-groups=["i1"],times="0",script={"aaa","bbb","ccc"},original-location="main"}]}
    (gdb)

In both the =breakpoint-modified and -break-info output, we have:

     script={"aaa","bbb","ccc"}

According to the output syntax [1], curly braces means tuple, and a
tuple contains key=value pairs.  This looks like it should be a list,
but uses curly braces by mistake.  This would make more sense:

    script=["aaa","bbb","ccc"]

Fix it, keeping the backwards compatibility by introducing a new MI
version (MI4), in exactly the same way as was done when fixing
multi-locations breakpoint output in [2].

 - Add a fix_breakpoint_script_output uiout flag.  MI uiouts will use
   this flag if the version is >= 4.
 - Add a fix_breakpoint_script_output_globally variable and the
   -fix-breakpoint-script-output MI command to set it, if frontends want
   to use the fixed output for this without using the newer MI version.
 - When emitting the script field, use list instead of tuple, if we want
   the fixed output (depending on the two criteria above)
 -

[1] https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Output-Syntax.html#GDB_002fMI-Output-Syntax
[2] https://gitlab.com/gnutools/binutils-gdb/-/commit/b4be1b0648608a2578bbed39841c8ee411773edd

Change-Id: I7113c6892832c8d6805badb06ce42496677e2242
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24285
  • Loading branch information
simark committed Aug 10, 2022
1 parent daf2618 commit 9db0d85
Show file tree
Hide file tree
Showing 15 changed files with 225 additions and 10 deletions.
14 changes: 14 additions & 0 deletions gdb/NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,20 @@ winheight
option, which causes the new inferior to start without a
connection.

** The default version of the MI interpreter is now 4 (-i=mi4).

** The "script" field in breakpoint output (which is syntactically
incorrect in MI 3 and below) has changed in MI 4 to become a list.
This affects the following commands and events:

- -break-insert
- -break-info
- =breakpoint-created
- =breakpoint-modified

The -fix-breakpoint-script-output command can be used to enable
this behavior with previous MI versions.

* New targets

GNU/Linux/LoongArch loongarch*-*-linux*
Expand Down
18 changes: 17 additions & 1 deletion gdb/breakpoint.c
Original file line number Diff line number Diff line change
Expand Up @@ -6162,6 +6162,10 @@ output_thread_groups (struct ui_out *uiout,
}
}

/* See breakpoint.h. */

bool fix_breakpoint_script_output_globally = false;

/* Print B to gdb_stdout. If RAW_LOC, print raw breakpoint locations
instead of going via breakpoint_ops::print_one. This makes "maint
info breakpoints" show the software breakpoint locations of
Expand Down Expand Up @@ -6458,7 +6462,19 @@ print_one_breakpoint_location (struct breakpoint *b,
if (!part_of_multiple && l)
{
annotate_field (9);
ui_out_emit_tuple tuple_emitter (uiout, "script");

bool use_fixed_output =
(uiout->test_flags (fix_breakpoint_script_output)
|| fix_breakpoint_script_output_globally);

gdb::optional<ui_out_emit_tuple> tuple_emitter;
gdb::optional<ui_out_emit_list> list_emitter;

if (use_fixed_output)
list_emitter.emplace (uiout, "script");
else
tuple_emitter.emplace (uiout, "script");

print_command_lines (uiout, l, 4);
}

Expand Down
5 changes: 5 additions & 0 deletions gdb/breakpoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -1871,6 +1871,11 @@ extern cmd_list_element *commands_cmd_element;

extern bool fix_multi_location_breakpoint_output_globally;

/* Whether to use the fixed output when printing information about
commands attached to a breakpoint. */

extern bool fix_breakpoint_script_output_globally;

/* Deal with "catch catch", "catch throw", and "catch rethrow" commands and
the MI equivalents. Sets up to catch events of type EX_EVENT. When
TEMPFLAG is true only the next matching event is caught after which the
Expand Down
26 changes: 23 additions & 3 deletions gdb/doc/gdb.texinfo
Original file line number Diff line number Diff line change
Expand Up @@ -30472,6 +30472,21 @@ The multiple locations are now placed in a @code{locations} field, whose value
is a list.
@end itemize

@item
@center 4
@tab
@center 13.1
@tab

@itemize
@item
The syntax of the "script" field in breakpoint output has changed in the
responses to the @code{-break-insert} and @code{-break-info} commands, as
well as the @code{=breakpoint-created} and @code{=breakpoint-modified}
events. The previous output was syntactically invalid. The new output is
a list.
@end itemize

@end multitable

If your front end cannot yet migrate to a more recent version of the
Expand All @@ -30482,9 +30497,14 @@ available in those recent MI versions, using the following commands:

@item -fix-multi-location-breakpoint-output
Use the output for multi-location breakpoints which was introduced by
MI 3, even when using MI versions 2 or 1. This command has no
MI 3, even when using MI versions below 3. This command has no
effect when using MI version 3 or later.

@item -fix-breakpoint-script-output
Use the output for the breakpoint "script" field which was introduced by
MI 4, even when using MI versions below 4. This command has no effect when
using MI version 4 or later.

@end table

The best way to avoid unexpected changes in MI that might break your front
Expand Down Expand Up @@ -31647,14 +31667,14 @@ The corresponding @value{GDBN} command is @samp{dprintf}.
4^done,bkpt=@{number="1",type="dprintf",disp="keep",enabled="y",
addr="0x000000000040061b",func="foo",file="mi-dprintf.c",
fullname="mi-dprintf.c",line="25",thread-groups=["i1"],
times="0",script=@{"printf \"At foo entry\\n\"","continue"@},
times="0",script=["printf \"At foo entry\\n\"","continue"],
original-location="foo"@}
(gdb)
5-dprintf-insert 26 "arg=%d, g=%d\n" arg g
5^done,bkpt=@{number="2",type="dprintf",disp="keep",enabled="y",
addr="0x000000000040062a",func="foo",file="mi-dprintf.c",
fullname="mi-dprintf.c",line="26",thread-groups=["i1"],
times="0",script=@{"printf \"arg=%d, g=%d\\n\", arg, g","continue"@},
times="0",script=["printf \"arg=%d, g=%d\\n\", arg, g","continue"],
original-location="mi-dprintf.c:26"@}
(gdb)
@end smallexample
Expand Down
1 change: 1 addition & 0 deletions gdb/interps.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ extern void interpreter_completer (struct cmd_list_element *ignore,
#define INTERP_MI1 "mi1"
#define INTERP_MI2 "mi2"
#define INTERP_MI3 "mi3"
#define INTERP_MI4 "mi4"
#define INTERP_MI "mi"
#define INTERP_TUI "tui"
#define INTERP_INSIGHT "insight"
Expand Down
2 changes: 2 additions & 0 deletions gdb/mi/mi-cmds.c
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,8 @@ add_builtin_mi_commands ()
add_mi_cmd_mi ("file-list-shared-libraries",
mi_cmd_file_list_shared_libraries),
add_mi_cmd_cli ("file-symbol-file", "symbol-file", 1);
add_mi_cmd_mi ("fix-breakpoint-script-output",
mi_cmd_fix_breakpoint_script_output),
add_mi_cmd_mi ("fix-multi-location-breakpoint-output",
mi_cmd_fix_multi_location_breakpoint_output),
add_mi_cmd_mi ("gdb-exit", mi_cmd_gdb_exit);
Expand Down
1 change: 1 addition & 0 deletions gdb/mi/mi-interp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1343,6 +1343,7 @@ _initialize_mi_interp ()
interp_factory_register (INTERP_MI1, mi_interp_factory);
interp_factory_register (INTERP_MI2, mi_interp_factory);
interp_factory_register (INTERP_MI3, mi_interp_factory);
interp_factory_register (INTERP_MI4, mi_interp_factory);
interp_factory_register (INTERP_MI, mi_interp_factory);

gdb::observers::signal_received.attach (mi_on_signal_received, "mi-interp");
Expand Down
11 changes: 10 additions & 1 deletion gdb/mi/mi-main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1865,7 +1865,8 @@ captured_mi_execute_command (struct ui_out *uiout, struct mi_parse *context)
if (current_interp_named_p (INTERP_MI)
|| current_interp_named_p (INTERP_MI1)
|| current_interp_named_p (INTERP_MI2)
|| current_interp_named_p (INTERP_MI3))
|| current_interp_named_p (INTERP_MI3)
|| current_interp_named_p (INTERP_MI4))
{
if (!running_result_record_printed)
{
Expand Down Expand Up @@ -2709,6 +2710,14 @@ mi_cmd_fix_multi_location_breakpoint_output (const char *command, char **argv,
fix_multi_location_breakpoint_output_globally = true;
}

/* See mi/mi-main.h. */

void
mi_cmd_fix_breakpoint_script_output (const char *command, char **argv, int argc)
{
fix_breakpoint_script_output_globally = true;
}

/* Implement the "-complete" command. */

void
Expand Down
5 changes: 5 additions & 0 deletions gdb/mi/mi-main.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,9 @@ extern void mi_execute_cli_command (const char *cmd, bool args_p,
extern void mi_cmd_fix_multi_location_breakpoint_output (const char *command,
char **argv, int argc);

/* Implementation of -fix-breakpoint-script-output. */

extern void mi_cmd_fix_breakpoint_script_output (const char *command,
char **argv, int argc);

#endif /* MI_MI_MAIN_H */
8 changes: 5 additions & 3 deletions gdb/mi/mi-out.c
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,7 @@ mi_ui_out::version ()
/* Constructor for an `mi_out_data' object. */

mi_ui_out::mi_ui_out (int mi_version)
: ui_out (mi_version >= 3
? fix_multi_location_breakpoint_output : (ui_out_flag) 0),
: ui_out (make_flags (mi_version)),
m_suppress_field_separator (false),
m_suppress_output (false),
m_mi_version (mi_version)
Expand All @@ -307,7 +306,10 @@ mi_ui_out::~mi_ui_out ()
mi_ui_out *
mi_out_new (const char *mi_version)
{
if (streq (mi_version, INTERP_MI3) || streq (mi_version, INTERP_MI))
if (streq (mi_version, INTERP_MI4) || streq (mi_version, INTERP_MI))
return new mi_ui_out (4);

if (streq (mi_version, INTERP_MI3))
return new mi_ui_out (3);

if (streq (mi_version, INTERP_MI2))
Expand Down
19 changes: 19 additions & 0 deletions gdb/mi/mi-out.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,25 @@ class mi_ui_out : public ui_out
redirected. */
string_file *main_stream ();

/* Helper for the constructor, deduce ui_out_flags for the given
MI_VERSION. */
static ui_out_flags make_flags (int mi_version)
{
ui_out_flags flags = 0;

/* In MI version 2 and below, multi-location breakpoints had a wrong
syntax. It is fixed in version 3. */
if (mi_version >= 3)
flags |= fix_multi_location_breakpoint_output;

/* In MI version 3 and below, the "script" field in breakpoint output
had a wrong syntax. It is fixed in version 4. */
if (mi_version >= 4)
flags |= fix_breakpoint_script_output;

return flags;
}

bool m_suppress_field_separator;
bool m_suppress_output;
int m_mi_version;
Expand Down
2 changes: 1 addition & 1 deletion gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ proc test_insert_delete_modify { } {
$test
set test "dprintf marker, \"arg\" \""
mi_gdb_test $test \
{.*=breakpoint-created,bkpt=\{number="6",type="dprintf".*,script=\{\"printf \\\"arg\\\" \\\"\"\}.*\}\r\n\^done} \
{.*=breakpoint-created,bkpt=\{number="6",type="dprintf".*,script=\[\"printf \\\"arg\\\" \\\"\"\].*\}\r\n\^done} \
$test

# 2. when modifying condition
Expand Down
22 changes: 22 additions & 0 deletions gdb/testsuite/gdb.mi/mi-breakpoint-script.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/* This testcase is part of GDB, the GNU debugger.
Copyright 2022 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/>. */

int
main (void)
{
return 0;
}
98 changes: 98 additions & 0 deletions gdb/testsuite/gdb.mi/mi-breakpoint-script.exp
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
# Copyright 2022 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/>.

# Tests the breakpoint script field, including the wrong syntax that GDB
# emitted before version 13.1.

load_lib mi-support.exp
standard_testfile .c

if {[gdb_compile "$srcdir/$subdir/$srcfile" $binfile executable {debug}] != "" } {
return -1
}

# Generate the regexp pattern used to match the breakpoint description emitted
# in the various breakpoint command results/events.
#
# - EXPECT_FIXED_OUTPUT: If true, expect GDB to output the fixed output for the
# script field, else expect it to output the broken pre-mi4 format.

proc make_pattern { expect_fixed_output } {
if $expect_fixed_output {
return "bkpt=\{number=\"${::decimal}\",type=\"breakpoint\",.*,script=\\\[\"abc\",\"def\"\\\],.*"
} else {
return "bkpt=\{number=\"${::decimal}\",type=\"breakpoint\",.*,script=\\\{\"abc\",\"def\"\\\},.*"
}
}

# Run the test with the following parameters:
#
# - MI_VERSION: the version of the MI interpreter to use (e.g. "2")
# - USE_FIX_FLAG: Whether to issue the -fix-breakpoint-script-output
# command after starting GDB
# - EXPECT_FIXED_OUTPUT: If true, expect GDB to output the fixed output for the
# script field, else expect it to output the broken pre-mi4 format.

proc do_test { mi_version use_fix_flag expect_fixed_output } {
with_test_prefix "mi_version=${mi_version}" {
with_test_prefix "use_fix_flag=${use_fix_flag}" {
save_vars { ::MIFLAGS } {
set ::MIFLAGS "-i=mi${mi_version}"

mi_clean_restart $::binfile
}

if $use_fix_flag {
mi_gdb_test "-fix-breakpoint-script-output" "\\^done" \
"send -fix-multi-location-breakpoint-output"
}

# Create a breakpoint.
mi_gdb_test "-break-insert main" ".*" "add breakpoint on main"

set pattern [make_pattern $expect_fixed_output]

# Add commands. Use the CLI command, so we can verify the
# =breakpoint-modified output.
mi_gdb_test "commands\nabc\ndef\nend" ".*=breakpoint-modified,$pattern\r\n\\^done" "add breakpoint commands"

# Check the -break-info output.
mi_gdb_test "-break-info" \
"\\^done,BreakpointTable=.*${pattern}" \
"-break-info"

mi_gdb_exit
}
}
}

# Vanilla mi3
do_test 3 0 0

# mi3 with -fix-breakpoint-script-output
do_test 3 1 1

# Vanilla mi4
do_test 4 0 1

# mi4 with -fix-breakpoint-script-output
do_test 4 1 1

# Whatever MI version is currently the default one, vanilla
do_test "" 0 1

# Whatever MI version is currently the default one, with
# -fix-breakpoint-script-output
do_test "" 1 1
3 changes: 2 additions & 1 deletion gdb/ui-out.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ enum ui_out_flag
fix_multi_location_breakpoint_output = (1 << 1),
/* This indicates that %pF should be disallowed in a printf format
string. */
disallow_ui_out_field = (1 << 2)
disallow_ui_out_field = (1 << 2),
fix_breakpoint_script_output = (1 << 3),
};

DEF_ENUM_FLAGS_TYPE (ui_out_flag, ui_out_flags);
Expand Down

0 comments on commit 9db0d85

Please sign in to comment.