From 9db0d8536dbc21744bc6763d5ef341e6af827111 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 10 Aug 2022 15:38:19 -0400 Subject: [PATCH] gdb/mi: fix breakpoint script field output 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 --- gdb/NEWS | 14 +++ gdb/breakpoint.c | 18 +++- gdb/breakpoint.h | 5 + gdb/doc/gdb.texinfo | 26 ++++- gdb/interps.h | 1 + gdb/mi/mi-cmds.c | 2 + gdb/mi/mi-interp.c | 1 + gdb/mi/mi-main.c | 11 ++- gdb/mi/mi-main.h | 5 + gdb/mi/mi-out.c | 8 +- gdb/mi/mi-out.h | 19 ++++ .../gdb.mi/mi-breakpoint-changed.exp | 2 +- gdb/testsuite/gdb.mi/mi-breakpoint-script.c | 22 +++++ gdb/testsuite/gdb.mi/mi-breakpoint-script.exp | 98 +++++++++++++++++++ gdb/ui-out.h | 3 +- 15 files changed, 225 insertions(+), 10 deletions(-) create mode 100644 gdb/testsuite/gdb.mi/mi-breakpoint-script.c create mode 100644 gdb/testsuite/gdb.mi/mi-breakpoint-script.exp diff --git a/gdb/NEWS b/gdb/NEWS index 8c837df76e5..f2040e26c0c 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -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* diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index dae96d205be..37f70a77721 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -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 @@ -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 tuple_emitter; + gdb::optional list_emitter; + + if (use_fixed_output) + list_emitter.emplace (uiout, "script"); + else + tuple_emitter.emplace (uiout, "script"); + print_command_lines (uiout, l, 4); } diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index c0b370a3e62..4c62f9d46fc 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -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 diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 382df00ee7d..82cb1c8d668 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -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 @@ -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 @@ -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 diff --git a/gdb/interps.h b/gdb/interps.h index e393b08c962..cd5fb3521d8 100644 --- a/gdb/interps.h +++ b/gdb/interps.h @@ -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" diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c index 05714693023..852f8c09264 100644 --- a/gdb/mi/mi-cmds.c +++ b/gdb/mi/mi-cmds.c @@ -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); diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index ae15177890c..368c13f5925 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -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"); diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 68868e49e99..b758f398e2a 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -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) { @@ -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 diff --git a/gdb/mi/mi-main.h b/gdb/mi/mi-main.h index 8e4fb637a03..0ac83685e6e 100644 --- a/gdb/mi/mi-main.h +++ b/gdb/mi/mi-main.h @@ -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 */ diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c index 567ef83de9b..eedc5f70549 100644 --- a/gdb/mi/mi-out.c +++ b/gdb/mi/mi-out.c @@ -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) @@ -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)) diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h index 8f2f2d82ec0..36d7e42345f 100644 --- a/gdb/mi/mi-out.h +++ b/gdb/mi/mi-out.h @@ -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; diff --git a/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp b/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp index 6472dcd0113..b71ea35d3c8 100644 --- a/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp +++ b/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp @@ -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 diff --git a/gdb/testsuite/gdb.mi/mi-breakpoint-script.c b/gdb/testsuite/gdb.mi/mi-breakpoint-script.c new file mode 100644 index 00000000000..6b3984dc7d2 --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-breakpoint-script.c @@ -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 . */ + +int +main (void) +{ + return 0; +} diff --git a/gdb/testsuite/gdb.mi/mi-breakpoint-script.exp b/gdb/testsuite/gdb.mi/mi-breakpoint-script.exp new file mode 100644 index 00000000000..ddf0be15f98 --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-breakpoint-script.exp @@ -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 . + +# 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 diff --git a/gdb/ui-out.h b/gdb/ui-out.h index 9e6ff9a29bf..1d74da905e4 100644 --- a/gdb/ui-out.h +++ b/gdb/ui-out.h @@ -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);