From patchwork Wed Mar 13 21:02:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 31846 Received: (qmail 48832 invoked by alias); 13 Mar 2019 21:02:19 -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 48822 invoked by uid 89); 13 Mar 2019 21:02:19 -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, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=tighten X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 13 Mar 2019 21:02:16 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id D32E456065; Wed, 13 Mar 2019 17:02:14 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id Z8rfITrl--DK; Wed, 13 Mar 2019 17:02:14 -0400 (EDT) Received: from murgatroyd (174-29-37-56.hlrn.qwest.net [174.29.37.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPSA id 66BA056032; Wed, 13 Mar 2019 17:02:14 -0400 (EDT) From: Tom Tromey To: Pedro Alves Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [RFC] Don't show "display"s twice in MI References: <20190312190320.19645-1-tromey@adacore.com> <0a23b883-9c80-a9e2-1e3e-3aa8c0b0ce13@redhat.com> <87y35ix0pv.fsf@tromey.com> <3cf86a8c-3448-1826-61bc-dff0faaa0744@redhat.com> Date: Wed, 13 Mar 2019 15:02:13 -0600 In-Reply-To: <3cf86a8c-3448-1826-61bc-dff0faaa0744@redhat.com> (Pedro Alves's message of "Wed, 13 Mar 2019 15:50:33 +0000") Message-ID: <87wol2h4iy.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 >>>>> "Pedro" == Pedro Alves writes: Pedro> Maybe we could simply move the do_display call elsewhere? There are several spots that call print_stop_event, so instead of that, I just added a way for the caller to suppress the displays. I also addressed your other comments and tried to tighten the test case a bit. Let me know what you think. Tom commit 930a25c9826c18d0b0e64f2daf7ced7fb9edc3a7 Author: Tom Tromey Date: Tue Mar 12 12:56:01 2019 -0600 Don't show "display"s twice in MI If you run "gdb -i=mi2" and set a "display", then when "next"ing the displays will be shown twice: ~"1: x = 23\n" ~"7\t printf(\"%d\\n\", x);\n" ~"1: x = 23\n" *stopped,reason="end-stepping-range",frame={addr="0x0000000000400565",func="main",args=[],file="q.c",fullname="/tmp/q.c",line="7"},thread-id="1",stopped-threads="all",core="1" The immediate cause of this is this code in mi_on_normal_stop_1: print_stop_event (mi_uiout); console_interp = interp_lookup (current_ui, INTERP_CONSOLE); if (should_print_stop_to_console (console_interp, tp)) print_stop_event (mi->cli_uiout); ... which obviously prints the stop twice. However, I think the first call to print_stop_event is intended just to emit the MI *stopped notification, which explains why the source line does not show up two times. This patch fixes the bug by changing print_stop_event to only call do_displays for non-MI-like ui-outs. Tested on x86-64 Fedora 29. gdb/ChangeLog 2019-03-13 Tom Tromey * mi/mi-interp.c (mi_on_normal_stop_1): Only show displays once. * infrun.h (print_stop_event): Add "displays" parameter. * infrun.c (print_stop_event): Add "displays" parameter. gdb/testsuite/ChangeLog 2019-03-12 Tom Tromey * gdb.mi/mi2-cli-display.c: New file. * gdb.mi/mi2-cli-display.exp: New file. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 24de651bf92..7273a778424 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2019-03-13 Tom Tromey + + * mi/mi-interp.c (mi_on_normal_stop_1): Only show displays once. + * infrun.h (print_stop_event): Add "displays" parameter. + * infrun.c (print_stop_event): Add "displays" parameter. + 2019-03-13 Tom Tromey * i386-gnu-nat.c (i386_gnu_nat_target::fetch_registers) diff --git a/gdb/infrun.c b/gdb/infrun.c index 3d7f36b4474..550fbe7f5b9 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -7856,7 +7856,7 @@ print_stop_location (struct target_waitstatus *ws) /* See infrun.h. */ void -print_stop_event (struct ui_out *uiout) +print_stop_event (struct ui_out *uiout, bool displays) { struct target_waitstatus last; ptid_t last_ptid; @@ -7870,7 +7870,8 @@ print_stop_event (struct ui_out *uiout) print_stop_location (&last); /* Display the auto-display expressions. */ - do_displays (); + if (displays) + do_displays (); } tp = inferior_thread (); diff --git a/gdb/infrun.h b/gdb/infrun.h index 8f61b75c15b..e53fd81e716 100644 --- a/gdb/infrun.h +++ b/gdb/infrun.h @@ -167,9 +167,10 @@ extern void print_return_value (struct ui_out *uiout, /* Print current location without a level number, if we have changed functions or hit a breakpoint. Print source line if we have one. - If the execution command captured a return value, print it. */ + If the execution command captured a return value, print it. If + DISPLAYS is false, do not call 'do_displays'. */ -extern void print_stop_event (struct ui_out *uiout); +extern void print_stop_event (struct ui_out *uiout, bool displays = true); /* Pretty print the results of target_wait, for debugging purposes. */ diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index f17e09ff04f..3c5a0d8fb78 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -625,10 +625,15 @@ mi_on_normal_stop_1 (struct bpstats *bs, int print_frame) reason = tp->thread_fsm->async_reply_reason (); mi_uiout->field_string ("reason", async_reason_lookup (reason)); } - print_stop_event (mi_uiout); console_interp = interp_lookup (current_ui, INTERP_CONSOLE); - if (should_print_stop_to_console (console_interp, tp)) + /* We only want to print the displays once, and we want it to + look just how it would on the console, so we use this to + decide whether the MI stop should include them. */ + bool console_print = should_print_stop_to_console (console_interp, tp); + print_stop_event (mi_uiout, !console_print); + + if (console_print) print_stop_event (mi->cli_uiout); mi_uiout->field_int ("thread-id", tp->global_num); diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 037f561595a..d71cc4b1924 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2019-03-12 Tom Tromey + + * gdb.mi/mi2-cli-display.c: New file. + * gdb.mi/mi2-cli-display.exp: New file. + 2019-03-13 Simon Marchi * mi-breakpoint-location-ena-dis.exp: Rename to ... diff --git a/gdb/testsuite/gdb.mi/mi2-cli-display.c b/gdb/testsuite/gdb.mi/mi2-cli-display.c new file mode 100644 index 00000000000..552612d9430 --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi2-cli-display.c @@ -0,0 +1,32 @@ +/* Copyright 2019 Free Software Foundation, Inc. + + This file is part of GDB. + + 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 . */ + +int +do_tests (int x) +{ + ++x; + ++x; + ++x; + ++x; + return x; +} + +int +main (void) +{ + return do_tests (23); +} diff --git a/gdb/testsuite/gdb.mi/mi2-cli-display.exp b/gdb/testsuite/gdb.mi/mi2-cli-display.exp new file mode 100644 index 00000000000..e12fe72964e --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi2-cli-display.exp @@ -0,0 +1,86 @@ +# Copyright 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 . + +# Ensure that CLI "display"s aren't double-emitted in MI mode. + +load_lib mi-support.exp +set MIFLAGS "-i=mi2" + +if {[mi_gdb_start]} { + continue +} + +standard_testfile + +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { + untested "failed to compile" + return -1 +} + +mi_delete_breakpoints +mi_gdb_reinitialize_dir $srcdir/$subdir +mi_gdb_load ${binfile} + +mi_runto do_tests + +# A helper procedure that checks for the display and the line number, +# and the following *stopped. X is the expected value of "x" and is +# also used in test names. +proc check_cli_display {x show_source} { + global mi_gdb_prompt + + # Now check for the display and the source line. We don't check + # the source line too closely, since it's not really important + # here, but we do check that the stop happened. + set stop "\\*stopped,reason=.*\r\n$mi_gdb_prompt$" + if {$show_source} { + set src "~\"\[0-9\]+\[^\"\]*\\\\n\"\r\n" + } else { + set src "" + } + set display "~\"1: x = $x\\\\n\"\r\n" + gdb_expect { + -re "^${display}${src}${display}${stop}" { + # This case is the bug: the display is shown twice. + fail "check display and source line x=$x" + } + -re "^${src}${display}${stop}" { + verbose -log "got <<<$expect_out(buffer)>>>" + pass "check display and source line x=$x" + } + -re ".*\r\n$mi_gdb_prompt$" { + verbose -log "got <<<$expect_out(buffer)>>>" + fail "check display and source line x=$x (unexpected output)" + } + timeout { + fail "check display and source line x=$x (timeout)" + } + } +} + +mi_gdb_test "display x" \ + "&\"display x\\\\n\"\r\n~\"1: x = 23\\\\n\"\r\n\\^done" \ + "display x" + +if {![mi_send_resuming_command "interpreter-exec console next" next]} { + pass "next" +} +check_cli_display 24 1 + +# Also check that displays are shown for -exec-next. +if {![mi_send_resuming_command exec-next exec-next]} { + pass "-exec-next" +} +check_cli_display 25 0