[PATCH] gdb: Add a new function to display binary output by groups

[PATCH] gdb: Add a new function to display binary output by groups

[Enze Li]

The new function(print_binary_chars_group) could display binary output
by groups when executing the print command with the "m" option, each
group has 4 bits.

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

With this patch applied, we have new print option.
  (gdb) print a
  $1 = 371
  (gdb) print/m a
  $2 = 1 0111 0011

Tested on x86_64-linux.

gdb/ChangeLog:

	* valprint.c (print_binary_chars_group): New function.
	* valprint.h (print_binary_chars_group): Add declaration.
	* printcmd.c (print_scalar_formatted): Use print_binary_chars_group.
---
 gdb/ChangeLog  |  6 +++++
 gdb/printcmd.c |  3 +++
 gdb/valprint.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/valprint.h |  3 +++
 4 files changed, 78 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6d0a0c649100..db8956dffbc2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2020-11-16  Enze Li  <lienze2010@hotmail.com>
+
+	* valprint.c (print_binary_chars_group): New function.
+	* valprint.h (print_binary_chars_group): Add declaration.
+	* printcmd.c (print_scalar_formatted): Use print_binary_chars_group.
+
 2020-11-15  Joel Brobecker  <brobecker@adacore.com>
 
 	* valarith.c (fixed_point_binop): Add BINOP_EQUAL and BINOP_LESS
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 665142446f4b..2961002fdf38 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -476,6 +476,9 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
     case 't':
       print_binary_chars (stream, valaddr, len, byte_order, size > 0);
       break;
+    case 'm':
+      print_binary_chars_group (stream, valaddr, len, byte_order, size > 0);
+      break;
     case 'x':
       print_hex_chars (stream, valaddr, len, byte_order, size > 0);
       break;
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 38ae0bdf0e25..ce95fd67d25a 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -1415,6 +1415,72 @@ print_binary_chars (struct ui_file *stream, const gdb_byte *valaddr,
     fputc_filtered ('0', stream);
 }
 
+void
+print_binary_chars_group (struct ui_file *stream, const gdb_byte *valaddr,
+		    unsigned len, enum bfd_endian byte_order, bool zero_pad)
+{
+  const gdb_byte *p;
+  unsigned int i;
+  int b;
+  bool seen_a_one = false;
+
+  /* Declared "int" so it will be signed.
+     This ensures that right shift will shift in zeros.  */
+
+  const int mask = 0x080;
+
+  if (byte_order == BFD_ENDIAN_BIG)
+    {
+      for (p = valaddr;
+	   p < valaddr + len;
+	   p++)
+	{
+	  /* Every byte has 8 binary characters; peel off
+	     and print from the MSB end.  */
+
+	  for (i = 0; i < (HOST_CHAR_BIT * sizeof (*p)); i++)
+	    {
+	      if (*p & (mask >> i))
+		b = '1';
+	      else
+		b = '0';
+
+	      if (zero_pad || seen_a_one || b == '1')
+		fputc_filtered (b, stream);
+	      if (b == '1')
+		seen_a_one = true;
+	    }
+	}
+    }
+  else
+    {
+      for (p = valaddr + len - 1;
+	   p >= valaddr;
+	   p--)
+	{
+	  for (i = 0; i < (HOST_CHAR_BIT * sizeof (*p)); i++)
+	    {
+	      if (seen_a_one && i%4==0)
+		fputc_filtered (' ', stream);
+	      if (*p & (mask >> i))
+		b = '1';
+	      else
+		b = '0';
+
+	      if (zero_pad || seen_a_one || b == '1')
+		fputc_filtered (b, stream);
+	      if (b == '1')
+		seen_a_one = true;
+	    }
+	}
+    }
+
+  /* When not zero-padding, ensure that something is printed when the
+     input is 0.  */
+  if (!zero_pad && !seen_a_one)
+    fputc_filtered ('0', stream);
+}
+
 /* A helper for print_octal_chars that emits a single octal digit,
    optionally suppressing it if is zero and updating SEEN_A_ONE.  */
 
diff --git a/gdb/valprint.h b/gdb/valprint.h
index ef9ebfa84f06..d75ac507d704 100644
--- a/gdb/valprint.h
+++ b/gdb/valprint.h
@@ -148,6 +148,9 @@ extern void value_print_scalar_formatted
 extern void print_binary_chars (struct ui_file *, const gdb_byte *,
 				unsigned int, enum bfd_endian, bool);
 
+extern void print_binary_chars_group (struct ui_file *, const gdb_byte *,
+				unsigned int, enum bfd_endian, bool);
+
 extern void print_octal_chars (struct ui_file *, const gdb_byte *,
 			       unsigned int, enum bfd_endian);
 
-- 
2.29.2

Re: [PATCH] gdb: Add a new function to display binary output by groups

[Andrew Burgess]

* Enze Li via Gdb-patches <gdb-patches@sourceware.org> [2020-11-16 16:07:18 +0800]:

> The new function(print_binary_chars_group) could display binary output
> by groups when executing the print command with the "m" option, each
> group has 4 bits.

This would need adding to the documentation in gdb/doc/gdb.texinfo, as
well as to the help text for 'x' (gdb/printcmd.c).  It would probably
need adding to NEWS too.  Finally you should definitely add tests to
cover the new functionality.

That said, I wonder if a whole new format option is the right way to
go?  Would a new option 'set print binary-groups on|off' would be just
as good?

My thinking is that this feels like something that a user would pick
the one they prefer, and stick with that, not something where the user
would be constantly switching formats for each print command?  And if
you really do need the other format for a one shot command, there's
always the `with .... ` syntax to allow that.

Just a thought...

Thanks,
Andrew

> 
> Here's a GDB session before this patch is applied.
>   (gdb) print a
>   $1 = 371
>   (gdb) print/t a
>   $2 = 101110011
> 
> With this patch applied, we have new print option.
>   (gdb) print a
>   $1 = 371
>   (gdb) print/m a
>   $2 = 1 0111 0011
> 
> Tested on x86_64-linux.
> 
> gdb/ChangeLog:
> 
> 	* valprint.c (print_binary_chars_group): New function.
> 	* valprint.h (print_binary_chars_group): Add declaration.
> 	* printcmd.c (print_scalar_formatted): Use print_binary_chars_group.
> ---
>  gdb/ChangeLog  |  6 +++++
>  gdb/printcmd.c |  3 +++
>  gdb/valprint.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  gdb/valprint.h |  3 +++
>  4 files changed, 78 insertions(+)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 6d0a0c649100..db8956dffbc2 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,9 @@
> +2020-11-16  Enze Li  <lienze2010@hotmail.com>
> +
> +	* valprint.c (print_binary_chars_group): New function.
> +	* valprint.h (print_binary_chars_group): Add declaration.
> +	* printcmd.c (print_scalar_formatted): Use print_binary_chars_group.
> +
>  2020-11-15  Joel Brobecker  <brobecker@adacore.com>
>  
>  	* valarith.c (fixed_point_binop): Add BINOP_EQUAL and BINOP_LESS
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index 665142446f4b..2961002fdf38 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -476,6 +476,9 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
>      case 't':
>        print_binary_chars (stream, valaddr, len, byte_order, size > 0);
>        break;
> +    case 'm':
> +      print_binary_chars_group (stream, valaddr, len, byte_order, size > 0);
> +      break;
>      case 'x':
>        print_hex_chars (stream, valaddr, len, byte_order, size > 0);
>        break;
> diff --git a/gdb/valprint.c b/gdb/valprint.c
> index 38ae0bdf0e25..ce95fd67d25a 100644
> --- a/gdb/valprint.c
> +++ b/gdb/valprint.c
> @@ -1415,6 +1415,72 @@ print_binary_chars (struct ui_file *stream, const gdb_byte *valaddr,
>      fputc_filtered ('0', stream);
>  }
>  
> +void
> +print_binary_chars_group (struct ui_file *stream, const gdb_byte *valaddr,
> +		    unsigned len, enum bfd_endian byte_order, bool zero_pad)
> +{
> +  const gdb_byte *p;
> +  unsigned int i;
> +  int b;
> +  bool seen_a_one = false;
> +
> +  /* Declared "int" so it will be signed.
> +     This ensures that right shift will shift in zeros.  */
> +
> +  const int mask = 0x080;
> +
> +  if (byte_order == BFD_ENDIAN_BIG)
> +    {
> +      for (p = valaddr;
> +	   p < valaddr + len;
> +	   p++)
> +	{
> +	  /* Every byte has 8 binary characters; peel off
> +	     and print from the MSB end.  */
> +
> +	  for (i = 0; i < (HOST_CHAR_BIT * sizeof (*p)); i++)
> +	    {
> +	      if (*p & (mask >> i))
> +		b = '1';
> +	      else
> +		b = '0';
> +
> +	      if (zero_pad || seen_a_one || b == '1')
> +		fputc_filtered (b, stream);
> +	      if (b == '1')
> +		seen_a_one = true;
> +	    }
> +	}
> +    }
> +  else
> +    {
> +      for (p = valaddr + len - 1;
> +	   p >= valaddr;
> +	   p--)
> +	{
> +	  for (i = 0; i < (HOST_CHAR_BIT * sizeof (*p)); i++)
> +	    {
> +	      if (seen_a_one && i%4==0)
> +		fputc_filtered (' ', stream);
> +	      if (*p & (mask >> i))
> +		b = '1';
> +	      else
> +		b = '0';
> +
> +	      if (zero_pad || seen_a_one || b == '1')
> +		fputc_filtered (b, stream);
> +	      if (b == '1')
> +		seen_a_one = true;
> +	    }
> +	}
> +    }
> +
> +  /* When not zero-padding, ensure that something is printed when the
> +     input is 0.  */
> +  if (!zero_pad && !seen_a_one)
> +    fputc_filtered ('0', stream);
> +}
> +
>  /* A helper for print_octal_chars that emits a single octal digit,
>     optionally suppressing it if is zero and updating SEEN_A_ONE.  */
>  
> diff --git a/gdb/valprint.h b/gdb/valprint.h
> index ef9ebfa84f06..d75ac507d704 100644
> --- a/gdb/valprint.h
> +++ b/gdb/valprint.h
> @@ -148,6 +148,9 @@ extern void value_print_scalar_formatted
>  extern void print_binary_chars (struct ui_file *, const gdb_byte *,
>  				unsigned int, enum bfd_endian, bool);
>  
> +extern void print_binary_chars_group (struct ui_file *, const gdb_byte *,
> +				unsigned int, enum bfd_endian, bool);
> +
>  extern void print_octal_chars (struct ui_file *, const gdb_byte *,
>  			       unsigned int, enum bfd_endian);
>  
> -- 
> 2.29.2
> 

Re: [PATCH] gdb: Add a new function to display binary output by groups

[Enze Li]

Hi Andrew,

Thank you very much for your suggestion. I'll try to improve it.

Thanks,
Enze

On 11/16/2020 6:14 PM, Andrew Burgess wrote:
> * Enze Li via Gdb-patches <gdb-patches@sourceware.org> [2020-11-16 16:07:18 +0800]:
>
>> The new function(print_binary_chars_group) could display binary output
>> by groups when executing the print command with the "m" option, each
>> group has 4 bits.
> This would need adding to the documentation in gdb/doc/gdb.texinfo, as
> well as to the help text for 'x' (gdb/printcmd.c).  It would probably
> need adding to NEWS too.  Finally you should definitely add tests to
> cover the new functionality.
>
> That said, I wonder if a whole new format option is the right way to
> go?  Would a new option 'set print binary-groups on|off' would be just
> as good?
>
> My thinking is that this feels like something that a user would pick
> the one they prefer, and stick with that, not something where the user
> would be constantly switching formats for each print command?  And if
> you really do need the other format for a one shot command, there's
> always the `with .... ` syntax to allow that.
>
> Just a thought...
>
> Thanks,
> Andrew