public inbox for gdb-cvs@sourceware.org
help / color / mirror / Atom feed
* [binutils-gdb] gdb/mi: fix breakpoint script field output
@ 2022-08-10 19:38 Simon Marchi
0 siblings, 0 replies; only message in thread
From: Simon Marchi @ 2022-08-10 19:38 UTC (permalink / raw)
To: gdb-cvs
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=9db0d8536dbc21744bc6763d5ef341e6af827111
commit 9db0d8536dbc21744bc6763d5ef341e6af827111
Author: Simon Marchi <simon.marchi@polymtl.ca>
Date: Wed Aug 10 15:38:19 2022 -0400
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
Diff:
---
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/testsuite/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(-)
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<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);
}
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 @@ private:
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 <http://www.gnu.org/licenses/>. */
+
+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 <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
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);
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2022-08-10 19:38 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-10 19:38 [binutils-gdb] gdb/mi: fix breakpoint script field output Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).