From patchwork Tue Nov 20 20:30:05 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 30223 Received: (qmail 10956 invoked by alias); 20 Nov 2018 20:30:31 -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 10795 invoked by uid 89); 20 Nov 2018 20:30:25 -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, RCVD_IN_DNSWL_NONE, SPF_PASS, TIME_LIMIT_EXCEEDED autolearn=unavailable version=3.3.2 spammy=knock, presents, disposition, HX-Received:sk:g10-v6m X-HELO: mail-wr1-f66.google.com Received: from mail-wr1-f66.google.com (HELO mail-wr1-f66.google.com) (209.85.221.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 20 Nov 2018 20:30:15 +0000 Received: by mail-wr1-f66.google.com with SMTP id j17-v6so3339941wrq.11 for ; Tue, 20 Nov 2018 12:30:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=44YOemtjzvLtFHGsh/LnsU9+eqFyRDwDbK2o5fIGIxE=; b=XTSRqRqMj1aonGEagUmmxeAllxgO3dm+c6fYvBFGE5vWNDyUzQImDJ9gpsrwgRamnD k76LCkLUASlGeO6Fafi0X3lpGS60synUeM0umibbS/M9GKWw8QDa4o/YyAqLRILXuxBj taaHy0AqexTvqfZrZozkhLRGO4ygEGTqFOYKUF1vo1n2izpjOaaCfkTSt9MitxmIJQTy JTtH7PXZPCPmkODutrN7dmzMuxgA7OKNoSN0xzoJ8VOw7u7F+SP1Lwi9k36S3FyCUln5 tgw0GqIfzCyBihgizuoSskopxgv9UXahnAz1KTGZJ1PeN1GkFwjYtBF6eucKC4WW1Dgo sleQ== Return-Path: Received: from localhost (host81-156-111-139.range81-156.btcentralplus.com. [81.156.111.139]) by smtp.gmail.com with ESMTPSA id q185sm9864163wmg.14.2018.11.20.12.30.12 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 20 Nov 2018 12:30:12 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 1/2] gdb: Respect field width and alignment for 'fmt' fields in CLI output Date: Tue, 20 Nov 2018 20:30:05 +0000 Message-Id: <7ff8ea3a0c985be7d9ea83746d471683347520e0.1542745482.git.andrew.burgess@embecosm.com> In-Reply-To: References: In-Reply-To: References: X-IsSubscribed: yes Currently the method 'cli_ui_out::do_field_fmt' has this comment: /* This is the only field function that does not align. */ The reality is even slightly worse, the 'fmt' field type doesn't respect either the field alignment or the field width. In at least one place in GDB we attempt to work around this lack of respect for field width by adding additional padding manually. But, as is often the case, this is leading to knock on problems. Conside the output for 'info breakpoints' when a breakpoint has multiple locations. This example is taken from the testsuite, from test gdb.opt/inline-break.exp: (gdb) info breakpoints Num Type Disp Enb Address What 1 breakpoint keep y 1.1 y 0x00000000004004ae in func4b at /src/gdb/testsuite/gdb.opt/inline-break.c:64 1.2 y 0x0000000000400682 in func4b at /src/gdb/testsuite/gdb.opt/inline-break.c:64 The miss-alignment of the fields shown here is exactly as GDB currently produces. With this patch 'fmt' style fields are now first written into a temporary buffer, and then written out as a 'string' field. The result is that the field width, and alignment should now be respected. With this patch in place the output from GDB now looks like this: (gdb) info breakpoints Num Type Disp Enb Address What 1 breakpoint keep y 1.1 y 0x00000000004004ae in func4b at /src/gdb/testsuite/gdb.opt/inline-break.c:64 1.2 y 0x0000000000400682 in func4b at /src/gdb/testsuite/gdb.opt/inline-break.c:64 This patch has been tested on x86-64/Linux with no regressions, however, the testsuite doesn't always spot broken output formatting or alignment. I have also audited all uses of 'fmt' fields that I could find, and I don't think there are any other places that specifically try to work around the lack of width/alignment, however, I could have missed something. gdb/ChangeLog: * breakpoint.c (print_one_breakpoint_location): Reduce whitespace, and remove insertion of extra spaces in GDB's output. * cli-out.c (cli_ui_out::do_field_fmt): Update header comment. Layout field into a temporary buffer, and then output it as a string field. gdb/testsuite/ChangeLog: * gdb.opt/inline-break.exp: Add test that info breakpoint output is correctly aligned. --- gdb/ChangeLog | 8 ++++++++ gdb/breakpoint.c | 3 --- gdb/cli-out.c | 8 ++++---- gdb/testsuite/ChangeLog | 5 +++++ gdb/testsuite/gdb.opt/inline-break.exp | 36 ++++++++++++++++++++++++++++++++++ 5 files changed, 53 insertions(+), 7 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 3d254344f2f..6bd456ebbb8 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -6058,16 +6058,13 @@ print_one_breakpoint_location (struct breakpoint *b, else uiout->field_string ("disp", bpdisp_text (b->disposition)); - /* 4 */ annotate_field (3); if (part_of_multiple) uiout->field_string ("enabled", loc->enabled ? "y" : "n"); else uiout->field_fmt ("enabled", "%c", bpenables[(int) b->enable_state]); - uiout->spaces (2); - /* 5 and 6 */ if (b->ops != NULL && b->ops->print_one != NULL) { diff --git a/gdb/cli-out.c b/gdb/cli-out.c index ad0a34ed39f..3fe131fef37 100644 --- a/gdb/cli-out.c +++ b/gdb/cli-out.c @@ -165,7 +165,7 @@ cli_ui_out::do_field_string (int fldno, int width, ui_align align, field_separator (); } -/* This is the only field function that does not align. */ +/* Output field containing ARGS using printf formatting in FORMAT. */ void cli_ui_out::do_field_fmt (int fldno, int width, ui_align align, @@ -175,10 +175,10 @@ cli_ui_out::do_field_fmt (int fldno, int width, ui_align align, if (m_suppress_output) return; - vfprintf_filtered (m_streams.back (), format, args); + string_file stb; + stb.vprintf (format, args); - if (align != ui_noalign) - field_separator (); + do_field_string (fldno, width, align, fldname, stb.string ().c_str ()); } void diff --git a/gdb/testsuite/gdb.opt/inline-break.exp b/gdb/testsuite/gdb.opt/inline-break.exp index aed38ed631f..06a08612371 100644 --- a/gdb/testsuite/gdb.opt/inline-break.exp +++ b/gdb/testsuite/gdb.opt/inline-break.exp @@ -304,4 +304,40 @@ with_test_prefix "address" { "breakpoint hit presents stop at inlined function" } +with_test_prefix "check alignment" { + + clean_restart $binfile + + if {![runto main]} { + untested "could not run to main" + continue + } + + gdb_test "break func4b" \ + "Breakpoint.*at.*func4b.*\\(2 locations\\)" + + set expected_line_length -1 + gdb_test_multiple "info break \$bpnum" "xxxx" { + -re "Num Type Disp Enb Address What\r\n" { + exp_continue + } + -re "($decimal \[^\r\n\]+)\[^\r\n\]+\r\n" { + if {$expected_line_length != -1} { + fail "multiple header lines seen" + } + set expected_line_length [string length $expect_out(1,string)] + exp_continue + } + -re "($decimal\.($decimal) \[^\r\n\]+)$hex\[^\r\n\]+\r\n" { + set len [string length $expect_out(1,string)] + set loc $expect_out(2,string) + gdb_assert {$len == $expected_line_length} \ + "check alignment of location line $loc" + exp_continue + } + -re "$gdb_prompt $" { + } + } +} + unset -nocomplain results