[PATCH] gdb:Add new 'print binary-groups' feature

[PATCH] gdb:Add new 'print binary-groups' feature

[Enze Li]

Make an introduction of a new print setting that can be set by
'set print binary-groups [on|off]'.  The default value is OFF, and
it would be changed by users manually.

And of course, 'show print binary-groups' is also included in the
patch.

The new feature could display binary values in groups, and each group
has 4 bits.

Here's a GDB session before this patch is applied.
  (gdb) print var_a
  $1 = 371
  (gdb) print/t var_a
  $2 = 101110011

With this patch applied, we have a new print setting to use.
  (gdb) print var_a
  $1 = 371
  (gdb) print/t var_a
  $2 = 101110011
  (gdb) set print binary-groups on
  (gdb) print/t var_a
  $3 = 1 0111 0011

Tested on x86_64-linux(little-endian) and mips-linux(big-endian).

gdb/ChangeLog:

	* NEWS: Metion new commands.
	* printcmd.c (print_scalar_formatted): Add new parameter and pass
	it to print_binary_chars.
	* valprint.c (struct value_print_options) <binary_group_print>:
	New member.
	(show_binary_groups): New function.
	(print_binary_chars): Add new parameter `value_print_option *`
	and use it.
	* valprint.h (struct value_print_options):
	(print_binary_chars): Add new parameter to declaration.

gdb/doc/ChangeLog:

	* gdb.texinfo: Document 'set/show print binary-groups'.

gdb/testsuite/ChangeLog:

	* gdb.base/printcmds.exp: Update to use new
	'print binary-groups' command.
---
 gdb/ChangeLog                        | 13 +++++++++++++
 gdb/NEWS                             |  5 +++++
 gdb/doc/ChangeLog                    |  5 +++++
 gdb/doc/gdb.texinfo                  | 26 ++++++++++++++++++++++++++
 gdb/printcmd.c                       |  2 +-
 gdb/testsuite/ChangeLog              |  5 +++++
 gdb/testsuite/gdb.base/printcmds.exp | 14 ++++++++++++++
 gdb/valprint.c                       | 26 +++++++++++++++++++++++++-
 gdb/valprint.h                       |  6 +++++-
 9 files changed, 99 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index acffde..2e19fd 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,16 @@
+2020-11-29  Enze Li  <lienze2010@hotmail.com>
+
+	* NEWS: Metion new commands.
+	* printcmd.c (print_scalar_formatted): Add new parameter and pass
+	it to print_binary_chars.
+	* valprint.c (struct value_print_options) <binary_group_print>:
+	New member.
+	(show_binary_groups): New function.
+	(print_binary_chars): Add new parameter `value_print_option *`
+	and use it.
+	* valprint.h (struct value_print_options):
+	(print_binary_chars): Add new parameter to declaration.
+
 2020-11-28  Alex Richardson  <Alexander.Richardson@cl.cam.ac.uk>
 
 	* acincludde.m4 (GDB_AC_CHECK_BFD): Include string.h in the test
diff --git a/gdb/NEWS b/gdb/NEWS
index d75992..324fce 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -39,6 +39,11 @@ set debug event-loop
 show debug event-loop
   Control the display of debug output about GDB's event loop.
 
+set print binary-groups [on|off]
+show print binary-groups
+  This controls whether the 'p/t' command will display the binary
+  value in groups, each group has 4 bits.  The default is 'off'.
+
 * Changed commands
 
 break [PROBE_MODIFIER] [LOCATION] [thread THREADNUM]
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 69b079..4c41cb 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2020-11-29  Enze Li  <lienze2010@hotmail.com>
+
+	* gdb.texinfo (Print Settings): Document
+	'set/show print binary-groups'.
+
 2020-11-19  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* gdb.texinfo (Debugging Output): Document 'set/show debug
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 01dcac..b75129 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -9929,6 +9929,10 @@ Related setting: @ref{set print array}.
 Set printing of array indexes.
 Related setting: @ref{set print array-indexes}.
 
+@item -binary-groups [@code{on}|@code{off}]
+Set whether to print binary values in groups.
+Related setting: @ref{set print binary-groups}.
+
 @item -elements @var{number-of-elements}|@code{unlimited}
 Set limit on string chars or array elements to print.  The value
 @code{unlimited} causes there to be no limit.  Related setting:
@@ -11213,6 +11217,28 @@ Stop printing element indexes when displaying arrays.
 Show whether the index of each element is printed when displaying
 arrays.
 
+@anchor{set print binary-groups}
+@item set print binary-groups
+@itemx set print binary-groups on
+Print binary values in groups when using the print command of
+@value{GDBN} with the option '/t'.  For example, this is what it
+looks like with @code{set print binary-groups on}:
+
+@smallexample
+@group
+(@value{GDBP}) p val_flags
+$1 = 371
+(@value{GDBP}) p/t val_flags
+$2 = 1 0111 0011
+@end group
+@end smallexample
+
+@item set print binary-groups off
+Stop printing binary values in groups.  The default is @code{off}.
+
+@item show print binary-groups
+Show whether to print binary values in groups.
+
 @anchor{set print elements}
 @item set print elements @var{number-of-elements}
 @itemx set print elements unlimited
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index e95b880..402c39 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -475,7 +475,7 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
       break;
 
     case 't':
-      print_binary_chars (stream, valaddr, len, byte_order, size > 0);
+      print_binary_chars (stream, valaddr, len, byte_order, size > 0, options);
       break;
     case 'x':
       print_hex_chars (stream, valaddr, len, byte_order, size > 0);
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 5f62e4..ae1ddb 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-11-29  Enze Li  <lienze2010@hotmail.com>
+
+	* gdb.base/printcmds.exp: Update to use new
+	'print binary-groups' command.
+
 2020-11-24  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
 
 	* gdb.base/condbreak-multi-context.exp: Do not hard-code location
diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
index 08a096..4b62bf 100644
--- a/gdb/testsuite/gdb.base/printcmds.exp
+++ b/gdb/testsuite/gdb.base/printcmds.exp
@@ -690,6 +690,19 @@ proc test_print_char_arrays {} {
     gdb_test_no_output "set print address off" "address off char arrays"
 }
 
+proc test_print_binary_groups {} {
+    gdb_test_no_output "set print binary-groups on"
+    gdb_test "p/t 0"		" = 0"
+    gdb_test "p/t 0x0"		" = 0"
+    gdb_test "p/t 0x30"		" = 11 0000"
+    gdb_test "p/t 0xff"		" = 1111 1111"
+    gdb_test "p/t 0x0f"		" = 1111"
+    gdb_test "p/t 0x01"		" = 1"
+    gdb_test "p/t 0xf0f"	" = 1111 0000 1111"
+    gdb_test "p/t 0x70f"	" = 111 0000 1111"
+    gdb_test_no_output "set print binary-groups off"
+}
+
 proc test_print_string_constants {} {
     global gdb_prompt
 
@@ -1068,6 +1081,7 @@ test_print_int_arrays
 test_print_typedef_arrays
 test_artificial_arrays
 test_print_char_arrays
+test_print_binary_groups
 # We used to do the runto main here.
 test_print_string_constants
 test_print_array_constants
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 50278a..6941e97 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -105,6 +105,7 @@ struct value_print_options user_print_options =
   0,				/* vtblprint */
   1,				/* unionprint */
   1,				/* addressprint */
+  0,				/* binary_group_print */
   0,				/* objectprint */
   PRINT_MAX_DEFAULT,		/* print_max */
   10,				/* repeat_count_threshold */
@@ -245,6 +246,15 @@ show_unionprint (struct ui_file *file, int from_tty,
 		    value);
 }
 
+static void
+show_binary_groups (struct ui_file *file, int from_tty,
+		       struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file,
+		    _("Printing binary values in groups is %s.\n"),
+		    value);
+}
+
 /* If nonzero, causes machine addresses to be printed in certain contexts.  */
 
 static void
@@ -1353,7 +1363,8 @@ print_floating (const gdb_byte *valaddr, struct type *type,
 
 void
 print_binary_chars (struct ui_file *stream, const gdb_byte *valaddr,
-		    unsigned len, enum bfd_endian byte_order, bool zero_pad)
+		    unsigned len, enum bfd_endian byte_order, bool zero_pad,
+		    const struct value_print_options *options)
 {
   const gdb_byte *p;
   unsigned int i;
@@ -1376,6 +1387,8 @@ print_binary_chars (struct ui_file *stream, const gdb_byte *valaddr,
 
 	  for (i = 0; i < (HOST_CHAR_BIT * sizeof (*p)); i++)
 	    {
+	      if (options->binary_group_print && seen_a_one && i%4 == 0)
+		fputc_filtered (' ', stream);
 	      if (*p & (mask >> i))
 		b = '1';
 	      else
@@ -1396,6 +1409,8 @@ print_binary_chars (struct ui_file *stream, const gdb_byte *valaddr,
 	{
 	  for (i = 0; i < (HOST_CHAR_BIT * sizeof (*p)); i++)
 	    {
+	      if (options->binary_group_print && seen_a_one && i%4 == 0)
+		fputc_filtered (' ', stream);
 	      if (*p & (mask >> i))
 		b = '1';
 	      else
@@ -3040,6 +3055,15 @@ static const gdb::option::option_def value_print_option_defs[] = {
     NULL, /* help_doc */
   },
 
+  boolean_option_def {
+    "binary-groups",
+    [] (value_print_options *opt) { return &opt->binary_group_print; },
+    show_binary_groups, /* show_cmd_cb */
+    N_("Set whether to print binary values in groups."),
+    N_("Show whether to print binary values in groups."),
+    NULL, /* help_doc */
+  },
+
   uinteger_option_def {
     "elements",
     [] (value_print_options *opt) { return &opt->print_max; },
diff --git a/gdb/valprint.h b/gdb/valprint.h
index ef9ebf..4ed081 100644
--- a/gdb/valprint.h
+++ b/gdb/valprint.h
@@ -44,6 +44,9 @@ struct value_print_options
   /* Controls printing of addresses.  */
   bool addressprint;
 
+  /* Controls printing of binary by group.  */
+  bool binary_group_print;
+
   /* Controls looking up an object's derived type using what we find
      in its vtables.  */
   bool objectprint;
@@ -146,7 +149,8 @@ extern void value_print_scalar_formatted
    int size, struct ui_file *stream);
 
 extern void print_binary_chars (struct ui_file *, const gdb_byte *,
-				unsigned int, enum bfd_endian, bool);
+				unsigned int, enum bfd_endian, bool,
+				const struct value_print_options *options);
 
 extern void print_octal_chars (struct ui_file *, const gdb_byte *,
 			       unsigned int, enum bfd_endian);
-- 
2.27.0

Re: [PATCH] gdb:Add new 'print binary-groups' feature

[Tom Tromey]

>>>>> ">" == Enze Li via Gdb-patches <gdb-patches@sourceware.org> writes:

>> Make an introduction of a new print setting that can be set by
>> 'set print binary-groups [on|off]'.  The default value is OFF, and
>> it would be changed by users manually.

Seems like a reasonable feature to me.

>> gdb/doc/ChangeLog:

>> 	* gdb.texinfo: Document 'set/show print binary-groups'.

Eli will have to approve the documentation change.
 
>> +	      if (options->binary_group_print && seen_a_one && i%4 == 0)

The gdb style requires spaces around the "%" operator.

>> +	      if (options->binary_group_print && seen_a_one && i%4 == 0)

Here too.

The rest looks good to me.  Thank you for doing this.

I didn't look - do you have a copyright assignment in place with the
FSF?  If not, contact me off-list so we can get started on this.

Tom

Re: [PATCH] gdb:Add new 'print binary-groups' feature

[Eli Zaretskii]

> Date: Sun, 29 Nov 2020 21:00:05 +0800
> From: Enze Li via Gdb-patches <gdb-patches@sourceware.org>
> 
> Make an introduction of a new print setting that can be set by
> 'set print binary-groups [on|off]'.  The default value is OFF, and
> it would be changed by users manually.

Sorry, I missed this.

> And of course, 'show print binary-groups' is also included in the
> patch.

"Binary-groups" is an unusual name.  How about "set print nibbles"
instead?  "Nibble" is accepted terminology for a group of 4 bits.

> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -39,6 +39,11 @@ set debug event-loop
>  show debug event-loop
>    Control the display of debug output about GDB's event loop.
>  
> +set print binary-groups [on|off]
> +show print binary-groups
> +  This controls whether the 'p/t' command will display the binary
> +  value in groups, each group has 4 bits.  The default is 'off'.

This is okay, but please use the full name of the command: "print /t".

> +@item -binary-groups [@code{on}|@code{off}]
> +Set whether to print binary values in groups.

Suggest to clarify:

  Set whether to print binary values in groups of four bits.

> +@anchor{set print binary-groups}
> +@item set print binary-groups
> +@itemx set print binary-groups on
> +Print binary values in groups when using the print command of
                       ^^^^^^^^^
Likewise here.

> +@value{GDBN} with the option '/t'.  For example, this is what it

Please use @samp{/t}, and let Texinfo produce the quotes.

> +@item set print binary-groups off
> +Stop printing binary values in groups.  The default is @code{off}.
                                           ^^^^^^^^^^^^^^^^^^^^^^^^^
"This is the default."

OK with those nits fixed.

Thanks.

Re: [PATCH] gdb:Add new 'print binary-groups' feature

[Pedro Alves]

Isn't this causing a regression in gdb.base/options.exp?  I think it
must, since this adds a new option to "print".  If it isn't (causing
a regression), then I think that testcase has a bug somewhere.

Re: [PATCH] gdb:Add new 'print binary-groups' feature

[Enze Li]

On 2020/12/4 4:13 上午, Tom Tromey wrote:
>>>>>> ">" == Enze Li via Gdb-patches <gdb-patches@sourceware.org> writes:
>>> Make an introduction of a new print setting that can be set by
>>> 'set print binary-groups [on|off]'.  The default value is OFF, and
>>> it would be changed by users manually.
> Seems like a reasonable feature to me.
>
>>> gdb/doc/ChangeLog:
>>> 	* gdb.texinfo: Document 'set/show print binary-groups'.
> Eli will have to approve the documentation change.
>  
>>> +	      if (options->binary_group_print && seen_a_one && i%4 == 0)
> The gdb style requires spaces around the "%" operator.
>
>>> +	      if (options->binary_group_print && seen_a_one && i%4 == 0)
> Here too.
>
> The rest looks good to me.  Thank you for doing this.
>
> I didn't look - do you have a copyright assignment in place with the
> FSF?  If not, contact me off-list so we can get started on this.
>
> Tom

Thanks for the review.

Fixed.

Enze

Re: [PATCH] gdb:Add new 'print binary-groups' feature

[Enze Li]

On 2020/12/4 11:44 下午, Pedro Alves wrote:
> Isn't this causing a regression in gdb.base/options.exp?  I think it
> must, since this adds a new option to "print".  If it isn't (causing
> a regression), then I think that testcase has a bug somewhere.

Thanks for the review.

I'll address this.

Enze

Re: [PATCH] gdb:Add new 'print binary-groups' feature

[Enze Li]

On 12/4/20 4:33 AM, Eli Zaretskii wrote:
>> Date: Sun, 29 Nov 2020 21:00:05 +0800
>> From: Enze Li via Gdb-patches <gdb-patches@sourceware.org>
>>
>> Make an introduction of a new print setting that can be set by
>> 'set print binary-groups [on|off]'.  The default value is OFF, and
>> it would be changed by users manually.
> Sorry, I missed this.
>
>> And of course, 'show print binary-groups' is also included in the
>> patch.
> "Binary-groups" is an unusual name.  How about "set print nibbles"
> instead?  "Nibble" is accepted terminology for a group of 4 bits.

Agreed.  "Nibble" is a better choice.

Thanks for the review.

Enze