[v2] Fix MI output for multi-location breakpoints

diff --git a/gdb/NEWS b/gdb/NEWS
index eaef6aa3849..531296f481e 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,18 @@  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
+
+     The -fix-multi-location-breakpoint-output command can be used to enable
+     this behavior with previous MI versions.
+
 * New native configurations
 
 GNU/Linux/RISC-V		riscv*-*-linux*
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3166fa01296..7385166a37f 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-main.h"
 
 /* readline include files */
 #include "readline/readline.h"
@@ -6361,12 +6362,15 @@  print_one_breakpoint (struct breakpoint *b,
 		      int allflag)
 {
   struct ui_out *uiout = current_uiout;
+  bool use_fixed_output = mi_multi_location_breakpoint_output_fixed (uiout);
 
-  {
-    ui_out_emit_tuple tuple_emitter (uiout, "bkpt");
+  gdb::optional<ui_out_emit_tuple> tuple_emitter (gdb::in_place, uiout, "bkpt");
+  print_one_breakpoint_location (b, NULL, 0, last_loc, allflag);
 
-    print_one_breakpoint_location (b, NULL, 0, last_loc, allflag);
-  }
+  /* The mi2 broken format: the main breakpoint tuple ends here, the locations
+     are outside.  */
+  if (!use_fixed_output)
+    tuple_emitter.reset ();
 
   /* If this breakpoint has custom print function,
      it's already printed.  Otherwise, print individual
@@ -6384,10 +6388,16 @@  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 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 () && use_fixed_output)
+	    locations_list.emplace (uiout, "locations");
 
-	  for (loc = b->loc; loc; loc = loc->next, ++n)
+	  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 173d18be6f7..c268a423b20 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1268,12 +1268,12 @@  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
-@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.
+@samp{--interpreter=mi} (or @samp{--interpreter=mi3}) causes
+@value{GDBN} to use the @dfn{@sc{gdb/mi} interface} version 3 (@pxref{GDB/MI, ,
+The @sc{gdb/mi} Interface}) included since @value{GDBN} version 9.1.  @sc{gdb/mi}
+version 2 (@code{mi2}), included in @value{GDBN} 6.0 and version 1 (@code{mi1}),
+included in @value{GDBN} 5.3, are also available.  Earlier @sc{gdb/mi}
+interfaces are no longer supported.
 
 @item -write
 @cindex @code{--write}
@@ -26501,18 +26501,22 @@  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.1.
+
 @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
 
@@ -27834,8 +27838,36 @@  than a tuple.
 a tuple.
 @end itemize
 
+@item
+@center 3
+@tab
+@center 9.1
+@tab
+
+@itemize
+@item
+The output of information about multi-location breakpoints has changed in the
+responses to the @code{-break-insert} and @code{-break-info} commands, as well
+as in the @code{=breakpoint-created} and @code{=breakpoint-modified} events.
+The multiple locations are now placed in a @code{locations} field, whose value
+is a list.
+@end itemize
+
 @end multitable
 
+The following commands can be used to selectively enable behaviors from a
+newer MI version.  They can be useful if you want to take advantage of feature
+or bug fix only available in a more recent MI version than what you are using,
+but can't yet migrate to that version for some reason.
+
+@table @code
+
+@item -fix-multi-location-breakpoint-output
+Use the output for multi-location breakpoints which was introduced by MI 3.
+This has no effect when using MI version 3 or later.
+
+@end table
+
 The best way to avoid unexpected changes in MI that might break your front
 end is to make your project known to @value{GDBN} developers and
 follow development on @email{gdb@@sourceware.org} and
@@ -28164,9 +28196,7 @@  following fields:
 
 @table @code
 @item number
-The breakpoint number.  For a breakpoint that represents one location
-of a multi-location breakpoint, this will be a dotted pair, like
-@samp{1.2}.
+The breakpoint number.
 
 @item type
 The type of the breakpoint.  For ordinary breakpoints this will be
@@ -28266,6 +28296,52 @@  is not.
 @item what
 Some extra data, the exact contents of which are type-dependent.
 
+@item locations
+This field is present if the breakpoint has multiple locations.  It is also
+exceptionally present if the breakpoint is enabled and has a single, disabled
+location.
+
+The value is a list of locations.  The format of a location is decribed below.
+
+@end table
+
+A location in a multi-location breakpoint is represented as a tuple with the
+following fields:
+
+@table @code
+
+@item number
+The location number as a dotted pair, like @samp{1.2}.  The first digit is the
+number of the parent breakpoint.  The second digit is the number of the
+location within that breakpoint.
+
+@item enabled
+This indicates whether the location is enabled, in which case the
+value is @samp{y}, or disabled, in which case the value is @samp{n}.
+Note that this is not the same as the field @code{enable}.
+
+@item addr
+The address of this location as an hexidecimal number.
+
+@item func
+If known, the function in which the location appears.
+If not known, this field is not present.
+
+@item file
+The name of the source file which contains this location, if known.
+If not known, this field is not present.
+
+@item fullname
+The full file name of the source file which contains this location, if
+known.  If not known, this field is not present.
+
+@item line
+The line number at which this location appears, if known.
+If not known, this field is not present.
+
+@item thread-groups
+The thread groups this location is in.
+
 @end table
 
 For example, here is what the output of @code{-break-insert}
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index bb7c20c777b..fe30ac2e822 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -118,6 +118,8 @@  static struct mi_cmd mi_cmds[] =
   DEF_MI_CMD_MI ("file-list-shared-libraries",
 		 mi_cmd_file_list_shared_libraries),
   DEF_MI_CMD_CLI ("file-symbol-file", "symbol-file", 1),
+  DEF_MI_CMD_MI ("fix-multi-location-breakpoint-output",
+		 mi_cmd_fix_multi_location_breakpoint_output),
   DEF_MI_CMD_MI ("gdb-exit", mi_cmd_gdb_exit),
   DEF_MI_CMD_CLI_1 ("gdb-set", "set", 1,
 		    &mi_suppress_notification.cmd_param_changed),
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index e45ddfe31ad..ead3ff3bd10 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/mi/mi-main.c b/gdb/mi/mi-main.c
index dc96032b0d4..4c9340302a7 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2710,6 +2710,33 @@  mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
   }
 }
 
+/* Whether to use the fixed output when printing information about a
+   multi-location breakpoint (see PR 9659).  */
+
+static bool fix_multi_location_breakpoint_output = false;
+
+/* See mi/mi-main.h.  */
+
+void
+mi_cmd_fix_multi_location_breakpoint_output (const char *command, char **argv,
+					     int argc)
+{
+  fix_multi_location_breakpoint_output = true;
+}
+
+/* See mi/mi-main.h.  */
+
+bool
+mi_multi_location_breakpoint_output_fixed (ui_out *uiout)
+{
+  mi_ui_out *mi_uiout = dynamic_cast<mi_ui_out *> (uiout);
+
+  if (mi_uiout == nullptr)
+    return false;
+
+  return mi_uiout->version () >= 3 || fix_multi_location_breakpoint_output;
+}
+
 void
 _initialize_mi_main (void)
 {
diff --git a/gdb/mi/mi-main.h b/gdb/mi/mi-main.h
index a81aa9c2de1..cec9e2dc68a 100644
--- a/gdb/mi/mi-main.h
+++ b/gdb/mi/mi-main.h
@@ -54,5 +54,18 @@  struct mi_suppress_notification
 };
 extern struct mi_suppress_notification mi_suppress_notification;
 
+/* Implementation of -fix-multi-location-breakpoint-output.  */
+
+extern void mi_cmd_fix_multi_location_breakpoint_output (const char *command,
+							 char **argv, int argc);
+
+/* Return whether -break-list, -break-insert, =breakpoint-created and
+   =breakpoint-modified should use the "fixed" output format (see PR
+   9659).
+
+   Return false if UIOUT is not an MI UI.  */
+
+extern bool mi_multi_location_breakpoint_output_fixed (ui_out *uiout);
+
 #endif
 
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..5d0452ad212
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-breakpoint-multiple-locations.exp
@@ -0,0 +1,131 @@ 
+# 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.
+#
+# - EXPECT_FIXED_OUTPUT: If true (non-zero), we expect GDB to output the fixed
+#   output for multi-locations breakpoint, else we expect it to output the
+#   broken pre-mi2 format.
+# - 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 { expect_fixed_output bp_num loc1_en loc2_en } {
+    if $expect_fixed_output {
+	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 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-multi-location-breakpoint-output
+#   command after starting GDB
+# - EXPECT_FIXED_OUTPUT: If true (non-zero), we expect GDB to output the fixed
+#   output for multi-locations breakpoint, else we expect it to output the
+#   broken pre-mi2 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}" {
+	    global MIFLAGS decimal
+	    set MIFLAGS "-i=mi${mi_version}"
+
+	    gdb_exit
+	    if {[mi_gdb_start]} {
+		return
+	    }
+
+	    mi_run_to_main
+
+	    if $use_fix_flag {
+		mi_gdb_test "-fix-multi-location-breakpoint-output" "\\^done" \
+		    "send -fix-multi-location-breakpoint-output"
+	    }
+
+	    # Check the breakpoint-created event.
+	    set pattern [make_breakpoints_pattern $expect_fixed_output 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 $expect_fixed_output 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 $expect_fixed_output 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 $expect_fixed_output 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
+	}
+    }
+}
+
+# Vanilla mi2
+do_test 2 0 0
+
+# mi2 with -fix-multi-location-breakpoint-output
+do_test 2 1 1
+
+# Vanilla mi3
+do_test 3 0 1
+
+# mi3 with -fix-multi-location-breakpoint-output
+do_test 3 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-multi-location-breakpoint-output
+do_test "" 1 1