From patchwork Fri Jan 11 00:15:34 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 31029 Received: (qmail 30958 invoked by alias); 11 Jan 2019 00:15:48 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 30866 invoked by uid 89); 11 Jan 2019 00:15:47 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, SPF_PASS autolearn=ham version=3.3.2 spammy=sc, dfn, wishing, unaffected X-HELO: sessmg22.ericsson.net Received: from sessmg22.ericsson.net (HELO sessmg22.ericsson.net) (193.180.251.58) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 11 Jan 2019 00:15:40 +0000 DKIM-Signature: v=1; a=rsa-sha256; d=ericsson.com; s=mailgw201801; c=relaxed/relaxed; q=dns/txt; i=@ericsson.com; t=1547165737; x=1549757737; h=From:Sender:Reply-To:Subject:Date:Message-ID:To:CC:MIME-Version:Content-Type: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=PxiXr3iSamMy5XYQUl8FcjAr67R2ahu9X3Q1OGsJUhE=; b=Tmsh5dKKtSar8ny82tSWuETWc8iLOOu36Yv4hnb+pDpdseOsz743UuYOHm2cJ6cd QTgvf4YJdmhr6OsPNbfPeZsJ513SKxJlySzlWfZTauJYlC2tLSkpVTBE8GJebmNM jvwe4zIblws58TY+CGdGyJSj6KBg8T65F4GNnLGJLMs=; Received: from ESESBMB501.ericsson.se (Unknown_Domain [153.88.183.114]) by sessmg22.ericsson.net (Symantec Mail Security) with SMTP id 48.E4.26412.920E73C5; Fri, 11 Jan 2019 01:15:37 +0100 (CET) Received: from ESESSMR501.ericsson.se (153.88.183.108) by ESESBMB501.ericsson.se (153.88.183.168) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3; Fri, 11 Jan 2019 01:15:37 +0100 Received: from ESESBMB505.ericsson.se (153.88.183.172) by ESESSMR501.ericsson.se (153.88.183.108) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3; Fri, 11 Jan 2019 01:15:37 +0100 Received: from NAM03-CO1-obe.outbound.protection.outlook.com (153.88.183.157) by ESESBMB505.ericsson.se (153.88.183.172) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3 via Frontend Transport; Fri, 11 Jan 2019 01:15:36 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ericsson.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=PxiXr3iSamMy5XYQUl8FcjAr67R2ahu9X3Q1OGsJUhE=; b=l4m6rvMkGZQatwngdGoNmEykxyBkpv59TEWDBxKMC/nGNVu3HjR7++F3wRaEMzCZxmTQX+CQ+SXRHl7/W0TECXjGMCe47wSkML9dpNrZqeXrKpX14Lx71fSYDHdRZiRbsw3NGU65RQGGQIlyNUvzs8UapPTAwcLL1ez4LBjmbbE= Received: from BYAPR15MB2390.namprd15.prod.outlook.com (52.135.198.30) by BYAPR15MB2981.namprd15.prod.outlook.com (20.178.237.206) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1516.15; Fri, 11 Jan 2019 00:15:34 +0000 Received: from BYAPR15MB2390.namprd15.prod.outlook.com ([fe80::a4b4:ea6a:6321:191e]) by BYAPR15MB2390.namprd15.prod.outlook.com ([fe80::a4b4:ea6a:6321:191e%3]) with mapi id 15.20.1495.011; Fri, 11 Jan 2019 00:15:34 +0000 From: Simon Marchi To: "gdb-patches@sourceware.org" CC: "palves@redhat.com" , Simon Marchi Subject: [PATCH] Fix MI output for multi-location breakpoints Date: Fri, 11 Jan 2019 00:15:34 +0000 Message-ID: <20190111001516.4761-1-simon.marchi@ericsson.com> received-spf: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts) authentication-results: spf=none (sender IP is ) smtp.mailfrom=simon.marchi@ericsson.com; MIME-Version: 1.0 Return-Path: simon.marchi@ericsson.com X-IsSubscribed: yes [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="",...},{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="",...},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 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 . - -# 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 . + +# 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