[PATCH] gdb/mi: fix breakpoint script field output

[PATCH] gdb/mi: fix breakpoint script field output

[Simon Marchi]

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<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 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 <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 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 <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 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

Re: [PATCH] gdb/mi: fix breakpoint script field output

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> The "script" field, output whenever information about a breakpoint with
Simon> commands is output, uses wrong MI syntax.

I think this is PR mi/24285.

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

Simon>  - Add a fix_breakpoint_script_output uiout flag.  MI uiouts will use
Simon>    this flag if the version is >= 4.

I was under the impression that MI version 3 was somehow experimental,
so we didn't need version 4 yet.  But maybe the idea is to roll out new
MI versions with some regularity and just update the version whenever we
think we've made a breaking change?

Tom

Re: [PATCH] gdb/mi: fix breakpoint script field output

[Simon Marchi]

On 2022-05-26 14:10, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> The "script" field, output whenever information about a breakpoint with
> Simon> commands is output, uses wrong MI syntax.
> 
> I think this is PR mi/24285.

Thanks, will add the footer.

> 
> Simon> Fix it, keeping the backwards compatibility by introducing a new MI
> Simon> version (MI4), in exactly the same way as was done when fixing
> Simon> multi-locations breakpoint output in [2].
> 
> Simon>  - Add a fix_breakpoint_script_output uiout flag.  MI uiouts will use
> Simon>    this flag if the version is >= 4.
> 
> I was under the impression that MI version 3 was somehow experimental,
> so we didn't need version 4 yet.  But maybe the idea is to roll out new
> MI versions with some regularity and just update the version whenever we
> think we've made a breaking change?

MI3 is the default today, so not experimental.  I think it's fine to
bump the version and ship it as soon as we make a fix that is also a
breaking change.  There's no reason to hold back fixes.  Users (well,
frontends) are supposed to use the interpreter name with the version in
it (mi2, mi3) rather than just "mi", so they don't get bad surprises
when a new MI version is released.  And then they can migrate at their
convenience.

Simon

Re: [PATCH] gdb/mi: fix breakpoint script field output

[Simon Marchi]

On 2022-05-26 14:21, Simon Marchi via Gdb-patches wrote:
> 
> 
> On 2022-05-26 14:10, Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>>
>> Simon> The "script" field, output whenever information about a breakpoint with
>> Simon> commands is output, uses wrong MI syntax.
>>
>> I think this is PR mi/24285.
> 
> Thanks, will add the footer.
> 
>>
>> Simon> Fix it, keeping the backwards compatibility by introducing a new MI
>> Simon> version (MI4), in exactly the same way as was done when fixing
>> Simon> multi-locations breakpoint output in [2].
>>
>> Simon>  - Add a fix_breakpoint_script_output uiout flag.  MI uiouts will use
>> Simon>    this flag if the version is >= 4.
>>
>> I was under the impression that MI version 3 was somehow experimental,
>> so we didn't need version 4 yet.  But maybe the idea is to roll out new
>> MI versions with some regularity and just update the version whenever we
>> think we've made a breaking change?
> 
> MI3 is the default today, so not experimental.  I think it's fine to
> bump the version and ship it as soon as we make a fix that is also a
> breaking change.  There's no reason to hold back fixes.  Users (well,
> frontends) are supposed to use the interpreter name with the version in
> it (mi2, mi3) rather than just "mi", so they don't get bad surprises
> when a new MI version is released.  And then they can migrate at their
> convenience.
> 
> Simon

I pushed this.

Simon

Re: [PATCH] gdb/mi: fix breakpoint script field output

[Tom de Vries]

On 8/10/22 21:38, Simon Marchi via Gdb-patches wrote:
> 
> 
> On 2022-05-26 14:21, Simon Marchi via Gdb-patches wrote:
>>
>>
>> On 2022-05-26 14:10, Tom Tromey wrote:
>>>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>>>
>>> Simon> The "script" field, output whenever information about a breakpoint with
>>> Simon> commands is output, uses wrong MI syntax.
>>>
>>> I think this is PR mi/24285.
>>
>> Thanks, will add the footer.
>>
>>>
>>> Simon> Fix it, keeping the backwards compatibility by introducing a new MI
>>> Simon> version (MI4), in exactly the same way as was done when fixing
>>> Simon> multi-locations breakpoint output in [2].
>>>
>>> Simon>  - Add a fix_breakpoint_script_output uiout flag.  MI uiouts will use
>>> Simon>    this flag if the version is >= 4.
>>>
>>> I was under the impression that MI version 3 was somehow experimental,
>>> so we didn't need version 4 yet.  But maybe the idea is to roll out new
>>> MI versions with some regularity and just update the version whenever we
>>> think we've made a breaking change?
>>
>> MI3 is the default today, so not experimental.  I think it's fine to
>> bump the version and ship it as soon as we make a fix that is also a
>> breaking change.  There's no reason to hold back fixes.  Users (well,
>> frontends) are supposed to use the interpreter name with the version in
>> it (mi2, mi3) rather than just "mi", so they don't get bad surprises
>> when a new MI version is released.  And then they can migrate at their
>> convenience.
>>
>> Simon
> 
> I pushed this.
> 

Hi,

I'm seeing:
...
FAIL: gdb.mi/mi-break.exp: mi-mode=main: test_breakpoint_commands: 
breakpoint commands: check that commands are set (unexpected output)
FAIL: gdb.mi/mi-break.exp: mi-mode=separate: test_breakpoint_commands: 
breakpoint commands: check that commands are set (unexpected output)
...
which looks like due to getting:
...
script=["print 10","continue"]
...
while expecting:
...
script=\{"print 10","continue"\}
...

Thanks,
- Tom