[binutils-gdb] gdb: Respect field width and alignment for 'fmt' fields in CLI output

[Andrew Burgess <aburgess at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=1871a62daf0561da0880ba1ad39e8191bc3cf1ac

commit 1871a62daf0561da0880ba1ad39e8191bc3cf1ac
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Tue Nov 20 12:59:18 2018 +0000

    gdb: Respect field width and alignment for 'fmt' fields in CLI output
    
    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   <MULTIPLE>
      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   <MULTIPLE>
      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.

Diff:
---
 gdb/ChangeLog                          |  8 ++++++++
 gdb/breakpoint.c                       |  3 ---
 gdb/cli-out.c                          |  7 +++----
 gdb/testsuite/ChangeLog                |  5 +++++
 gdb/testsuite/gdb.opt/inline-break.exp | 36 ++++++++++++++++++++++++++++++++++
 5 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fd9c168..66fe9c9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2018-11-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* 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.
+
 2018-11-20  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* NEWS: Document the language choice done by
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3d25434..6bd456e 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 ad0a34e..9ffd6f0 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,9 @@ 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);
+  std::string str = string_vprintf (format, args);
 
-  if (align != ui_noalign)
-    field_separator ();
+  do_field_string (fldno, width, align, fldname, str.c_str ());
 }
 
 void
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 8814cd3..093603a 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2018-11-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.opt/inline-break.exp: Add test that info breakpoint output
+	is correctly aligned.
+
 2018-11-20  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* gdb.ada/info_auto_lang.exp: New testcase.
diff --git a/gdb/testsuite/gdb.opt/inline-break.exp b/gdb/testsuite/gdb.opt/inline-break.exp
index aed38ed..06a0861 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\]+)<MULTIPLE>\[^\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