From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id CB5A33857822 for ; Wed, 4 May 2022 12:13:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CB5A33857822 X-ASG-Debug-ID: 1651666423-0c856e06abd86a20001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id wbrY8cw6N7c219pz (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 04 May 2022 08:13:43 -0400 (EDT) X-Barracuda-Envelope-From: simon.marchi@polymtl.ca X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from simark.localdomain (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) by smtp.ebox.ca (Postfix) with ESMTP id AD3DD441B21; Wed, 4 May 2022 08:13:43 -0400 (EDT) From: Simon Marchi X-Barracuda-RBL-IP: 192.222.157.6 X-Barracuda-Effective-Source-IP: 192-222-157-6.qc.cable.ebox.net[192.222.157.6] X-Barracuda-Apparent-Source-IP: 192.222.157.6 To: gdb-patches@sourceware.org Subject: [PATCH] gdb/mi: fix breakpoint script field output Date: Wed, 4 May 2022 08:13:42 -0400 X-ASG-Orig-Subj: [PATCH] gdb/mi: fix breakpoint script field output Message-Id: <20220504121342.1208374-1-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.36.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1651666423 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 19848 X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.50 X-Barracuda-Spam-Status: No, SCORE=0.50 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=8.0 tests=BSF_RULE7568M X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.97791 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.50 BSF_RULE7568M Custom Rule 7568M X-Spam-Status: No, score=-3612.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_QUARANTINE, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 May 2022 12:14:01 -0000 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 --- gdb/NEWS | 16 +++ 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, 227 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 a72fee81550b..bc835db82dab 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -41,6 +41,22 @@ maintenance info line-table This is the same format that GDB uses when printing address, symbol, and offset information from the disassembler. +* MI changes + + ** 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. + *** Changes in GDB 12 * DBX mode is deprecated, and will be removed in GDB 13 diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 7241b7b69851..b34ee8e9d5de 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -6154,6 +6154,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 @@ -6425,7 +6429,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 1322bc1b9b9b..b9fb5bbf4eca 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -1851,6 +1851,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 38ad2ac32b08..d5ee2f063ca5 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -30162,6 +30162,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 @@ -30172,9 +30187,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 @@ -31343,14 +31363,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 330c1ba66153..79bc228f934b 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 05714693023b..852f8c092641 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 8d6e0334a90d..56eadf9da528 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -1346,6 +1346,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 18707bf62e74..bf324a16bfac 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 8e4fb637a031..0ac83685e6e7 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 567ef83de9bb..eedc5f70549c 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 8f2f2d82ec0a..36d7e42345f8 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 f24ce6ba86cb..26ced0b3cf7e 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 000000000000..6b3984dc7d22 --- /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 000000000000..ddf0be15f98a --- /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 9e6ff9a29bf0..1d74da905e41 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); base-commit: 3569f4ab7e4bb8bf34db35b623da0c5774b4ee90 -- 2.36.0