[PATCH] Fix MI output for multi-location breakpoints
Simon Marchi
simon.marchi@ericsson.com
Fri Jan 11 00:15:00 GMT 2019
[CCing Pedro because we had some discussions earlier about that offline]
Various MI commands or events related to breakpoints output invalid MI
records when printing information about a multi-location breakpoint.
For example:
-break-insert allo
^done,bkpt={...,addr="<MULTIPLE>",...},{number="1.1",...},{number="1.2",...}
The problem is that according to the syntax [1], the top-level elements
are of type "result" and should be of the form "variable=value".
This patch changes the output to wrap the locations in a list:
^done,bkpt={...,addr="<MULTIPLE>",...},locations=[{number="1.1",...},{number="1.2",...}]
The events =breakpoint-created, =breakpoint-modified, as well as the
-break-info command also suffer from this (and maybe others I didn't
find).
Since this is a breaking change for MI, we have to deal somehow with
backwards compatibility. The approach taken by this patch is to bump
the MI version, use the new syntax in MI3 while retaining the old syntax
in MI2. Frontends are expected to use a precise MI version (-i=mi2), so
if they do that they should be unaffected.
My previous attempts of this patch (not published) introduced new
commands (-break-insert2, -break-info2) or a flag to the original
commands to make them output the fixed syntax. That doesn't really work
because async events need to be fixed too.
Another solution would be to have a setting or command
(-use-fixed-breakpoint-output) to enable the fixed output, keeping the
broken one by default. The only thing I don't like about that is that
we will keep the broken behavior by default, which means that somebody
writing a frontend who is not aware of that gotcha will go through the
trouble of stumbling on broken MI output, and then hopefully discover
that there is a command to un-break it. I also don't see an easy way to
deprecate and remove the old output over time using this strategy. With
a new MI version, somebody starting from scratch will use the latest MI
version available, and therefore the fixed version. Also, we can
deprecate and then remove old MI versions after, let's say, 5 years of a
newer version being available.
[1] https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Output-Syntax.html#GDB_002fMI-Output-Syntax
gdb/ChangeLog:
* NEWS: Mention that the new default MI version is 3. Mention
changes to the output of commands and events that deal with
multi-location breakpoints.
* breakpoint.c: Include "mi/mi-out.h".
(print_one_breakpoint): Change output syntax if using MI version
>= 3.
* mi/mi-interp.c (mi_interp::init): Change default MI version to
3.
gdb/testsuite/ChangeLog:
* mi-breakpooint-location-ena-dis.exp: Rename to ...
* mi-breakpooint-multiple-locations.exp: ... this.
(make_breakpoints_pattern): New proc.
(do_test): Add mi_version parameter, test -break-insert,
-break-info and =breakpoint-created.
gdb/doc/ChangeLog:
* gdb.texinfo (Choosing Modes): Mention mi3.
(Command Interpreters): Likewise.
---
gdb/NEWS | 11 ++
gdb/breakpoint.c | 12 ++-
gdb/doc/gdb.texinfo | 20 ++--
gdb/mi/mi-interp.c | 5 +-
.../gdb.mi/mi-breakpoint-location-ena-dis.exp | 56 ----------
...cc => mi-breakpoint-multiple-locations.cc} | 0
.../mi-breakpoint-multiple-locations.exp | 100 ++++++++++++++++++
7 files changed, 134 insertions(+), 70 deletions(-)
delete mode 100644 gdb/testsuite/gdb.mi/mi-breakpoint-location-ena-dis.exp
rename gdb/testsuite/gdb.mi/{mi-breakpoint-location-ena-dis.cc => mi-breakpoint-multiple-locations.cc} (100%)
create mode 100644 gdb/testsuite/gdb.mi/mi-breakpoint-multiple-locations.exp
diff --git a/gdb/NEWS b/gdb/NEWS
index eaef6aa3849..f9c68871c93 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -165,6 +165,8 @@ set style address intensity VALUE
* MI changes
+ ** The default version of the MI interpreter is now 3 (-i=mi3).
+
** The '-data-disassemble' MI command now accepts an '-a' option to
disassemble the whole function surrounding the given program
counter value or function name. Support for this feature can be
@@ -174,6 +176,15 @@ set style address intensity VALUE
** Command responses and notifications that include a frame now include
the frame's architecture in a new "arch" attribute.
+ ** The output of information about multi-location breakpoints (which is
+ syntactically incorrect in MI 2) has changed in MI 3. This affects
+ the following commands and events:
+
+ - -break-insert
+ - -break-info
+ - =breakpoint-created
+ - =breakpoint-modified
+
* New native configurations
GNU/Linux/RISC-V riscv*-*-linux*
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 2ab8a76326c..059f2d97419 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -69,6 +69,7 @@
#include "thread-fsm.h"
#include "tid-parse.h"
#include "cli/cli-style.h"
+#include "mi/mi-out.h"
/* readline include files */
#include "readline/readline.h"
@@ -6384,10 +6385,15 @@ print_one_breakpoint (struct breakpoint *b,
&& !is_hardware_watchpoint (b)
&& (b->loc->next || !b->loc->enabled))
{
- struct bp_location *loc;
- int n = 1;
+ gdb::optional<ui_out_emit_list> locations_list;
- for (loc = b->loc; loc; loc = loc->next, ++n)
+ /* For MI version <= 2, keep the behavior where GDB outputs an invalid MI
+ record. For later versions, place breakpoint locations in a list. */
+ if (uiout->is_mi_like_p () && mi_version (uiout) >= 3)
+ locations_list.emplace (uiout, "locations");
+
+ int n = 1;
+ for (bp_location *loc = b->loc; loc != NULL; loc = loc->next, ++n)
{
ui_out_emit_tuple tuple_emitter (uiout, NULL);
print_one_breakpoint_location (b, loc, n, last_loc, allflag);
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 4a00834d0bf..a422a395c47 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1268,12 +1268,11 @@ program or device. This option is meant to be set by programs which
communicate with @value{GDBN} using it as a back end.
@xref{Interpreters, , Command Interpreters}.
-@samp{--interpreter=mi} (or @samp{--interpreter=mi2}) causes
+@samp{--interpreter=mi} (or @samp{--interpreter=mi3}) causes
@value{GDBN} to use the @dfn{@sc{gdb/mi} interface} (@pxref{GDB/MI, ,
-The @sc{gdb/mi} Interface}) included since @value{GDBN} version 6.0. The
-previous @sc{gdb/mi} interface, included in @value{GDBN} version 5.3 and
-selected with @samp{--interpreter=mi1}, is deprecated. Earlier
-@sc{gdb/mi} interfaces are no longer supported.
+The @sc{gdb/mi} Interface}). The @sc{gdb/mi} interfaces 1 and 2 are
+available, but deprecated. Earlier @sc{gdb/mi} interfaces are no
+longer available.
@item -write
@cindex @code{--write}
@@ -26501,18 +26500,23 @@ used interpreter with @value{GDBN}. With no interpreter specified at runtime,
@item mi
@cindex mi interpreter
-The newest @sc{gdb/mi} interface (currently @code{mi2}). Used primarily
+The newest @sc{gdb/mi} interface (currently @code{mi3}). Used primarily
by programs wishing to use @value{GDBN} as a backend for a debugger GUI
or an IDE. For more information, see @ref{GDB/MI, ,The @sc{gdb/mi}
Interface}.
+@item mi3
+@cindex mi3 interpreter
+The @sc{gdb/mi} interface introduced in @value{GDBN} 9. It is the latest
+@sc{gdb/mi} version.
+
@item mi2
@cindex mi2 interpreter
-The current @sc{gdb/mi} interface.
+The @sc{gdb/mi} interface introduced in @value{GDBN} 6.0.
@item mi1
@cindex mi1 interpreter
-The @sc{gdb/mi} interface included in @value{GDBN} 5.1, 5.2, and 5.3.
+The @sc{gdb/mi} interface introduced in @value{GDBN} 5.1.
@end table
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index d4baa485210..d51d3d72553 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -129,10 +129,9 @@ mi_interp::init (bool top_level)
mi->targ = new mi_console_file (mi->raw_stdout, "@", '"');
mi->event_channel = new mi_console_file (mi->raw_stdout, "=", 0);
- /* INTERP_MI selects the most recent released version. "mi2" was
- released as part of GDB 6.0. */
+ /* INTERP_MI selects the most recent released version. */
if (strcmp (name (), INTERP_MI) == 0)
- mi_version = 2;
+ mi_version = 3;
else if (strcmp (name (), INTERP_MI1) == 0)
mi_version = 1;
else if (strcmp (name (), INTERP_MI2) == 0)
diff --git a/gdb/testsuite/gdb.mi/mi-breakpoint-location-ena-dis.exp b/gdb/testsuite/gdb.mi/mi-breakpoint-location-ena-dis.exp
deleted file mode 100644
index eb781490fe7..00000000000
--- a/gdb/testsuite/gdb.mi/mi-breakpoint-location-ena-dis.exp
+++ /dev/null
@@ -1,56 +0,0 @@
-# Copyright 2018-2019 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 whether =breakpoint=modified notification is sent when a single
-# breakpoint location is enabled or disabled via CLI.
-
-load_lib mi-support.exp
-set MIFLAGS "-i=mi"
-
-gdb_exit
-if {[mi_gdb_start]} {
- continue
-}
-
-#
-# Start here
-#
-standard_testfile .cc
-
-if {[gdb_compile "$srcdir/$subdir/$srcfile" $binfile executable {debug c++}] != "" } {
- return -1
-}
-
-mi_run_to_main
-
-mi_gdb_test "break add" \
- {(&.*)*.*~"Breakpoint 2 at.*\\n".*=breakpoint-created,bkpt=\{number="2",type="breakpoint".*\},\{number="2.1",enabled="y".*\}.*\n\^done} \
- "break add"
-
-# Modify enableness through MI commands shouldn't trigger MI
-# notification.
-mi_gdb_test "-break-disable 2.2" "\\^done" "-break-disable 2.2"
-mi_gdb_test "-break-enable 2.2" "\\^done" "-break-enable 2.2"
-
-# Modify enableness through CLI commands should trigger MI
-# notification.
-mi_gdb_test "dis 2.2" \
- {.*=breakpoint-modified,bkpt=\{number="2",type="breakpoint".*\},\{number="2.1",enabled="y".*\},\{number="2.2",enabled="n".*\}.*\n\^done} \
- "dis 2.2"
-mi_gdb_test "en 2.2" \
- {.*=breakpoint-modified,bkpt=\{number="2",type="breakpoint".*\},\{number="2.1",enabled="y".*\},\{number="2.2",enabled="y".*\}.*\n\^done} \
- "en 2.2"
-
-mi_gdb_exit
diff --git a/gdb/testsuite/gdb.mi/mi-breakpoint-location-ena-dis.cc b/gdb/testsuite/gdb.mi/mi-breakpoint-multiple-locations.cc
similarity index 100%
rename from gdb/testsuite/gdb.mi/mi-breakpoint-location-ena-dis.cc
rename to gdb/testsuite/gdb.mi/mi-breakpoint-multiple-locations.cc
diff --git a/gdb/testsuite/gdb.mi/mi-breakpoint-multiple-locations.exp b/gdb/testsuite/gdb.mi/mi-breakpoint-multiple-locations.exp
new file mode 100644
index 00000000000..4d61859d2f0
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-breakpoint-multiple-locations.exp
@@ -0,0 +1,100 @@
+# Copyright 2018-2019 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 various things related to breakpoints with multiple locations.
+
+load_lib mi-support.exp
+standard_testfile .cc
+
+if {[gdb_compile "$srcdir/$subdir/$srcfile" $binfile executable {debug c++}] != "" } {
+ return -1
+}
+
+# Generate the regexp pattern used to match the breakpoint description emitted
+# in the various breakpoint command results/events, as expected for the given
+# MI_VERSION.
+#
+# - BP_NUM is the expected breakpoint number
+# - LOC1_EN and LOC2_EN are the expected value of the enabled field, for the
+# two locations.
+
+
+proc make_breakpoints_pattern { mi_version bp_num loc1_en loc2_en } {
+ if { $mi_version == "" || $mi_version >= 3 } {
+ return "bkpt=\{number=\"${bp_num}\",type=\"breakpoint\",.*\},locations=\\\[\{number=\"${bp_num}\\.1\",enabled=\"${loc1_en}\",.*\},\{number=\"${bp_num}\\.2\",enabled=\"${loc2_en}\",.*\}\\\]"
+ } else {
+ return "bkpt=\{number=\"${bp_num}\",type=\"breakpoint\",.*\},\{number=\"${bp_num}\\.1\",enabled=\"${loc1_en}\",.*\},\{number=\"${bp_num}\\.2\",enabled=\"${loc2_en}\",.*\}"
+ }
+}
+
+# Run the test against the given version of the MI interpreter.
+
+proc do_test { mi_version } {
+ with_test_prefix "mi_version=${mi_version}" {
+ global MIFLAGS decimal
+ set MIFLAGS "-i=mi${mi_version}"
+
+ gdb_exit
+ if {[mi_gdb_start]} {
+ return
+ }
+
+ mi_run_to_main
+
+ # Check the breakpoint-created event.
+ set pattern [make_breakpoints_pattern $mi_version 2 y y]
+ mi_gdb_test "break add" \
+ [multi_line "&\"break add\\\\n\"" \
+ "~\"Breakpoint ${decimal} at.*\\(2 locations\\)\\\\n\"" \
+ "=breakpoint-created,${pattern}" \
+ "\\^done" ] \
+ "break add"
+
+ # Check the -break-info output.
+ mi_gdb_test "-break-info" \
+ "\\^done,BreakpointTable=\{.*,body=\\\[${pattern}\\\]\}" \
+ "-break-info"
+
+ # Check the -break-insert response.
+ set pattern [make_breakpoints_pattern $mi_version 3 y y]
+ mi_gdb_test "-break-insert add" "\\^done,${pattern}" "insert breakpoint with MI command"
+
+ # Modify enableness through MI commands shouldn't trigger MI
+ # notification.
+ mi_gdb_test "-break-disable 2.2" "\\^done" "-break-disable 2.2"
+ mi_gdb_test "-break-enable 2.2" "\\^done" "-break-enable 2.2"
+
+ # Modify enableness through CLI commands should trigger MI
+ # notification.
+ set pattern [make_breakpoints_pattern $mi_version 2 y n]
+ mi_gdb_test "dis 2.2" \
+ [multi_line "&\"dis 2.2\\\\n\"" \
+ "=breakpoint-modified,${pattern}" \
+ "\\^done" ] \
+ "dis 2.2"
+ set pattern [make_breakpoints_pattern $mi_version 2 y y]
+ mi_gdb_test "en 2.2" \
+ [multi_line "&\"en 2.2\\\\n\"" \
+ "=breakpoint-modified,${pattern}" \
+ "\\^done" ] \
+ "en 2.2"
+
+ mi_gdb_exit
+ }
+}
+
+do_test ""
+do_test 2
+do_test 3
--
2.20.1
More information about the Gdb-patches
mailing list