From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTPS id 90764385842D for ; Wed, 27 Oct 2021 18:43:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 90764385842D Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-387-BqTLEQIzNNS7WAWRGe10Cw-1; Wed, 27 Oct 2021 14:43:11 -0400 X-MC-Unique: BqTLEQIzNNS7WAWRGe10Cw-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id DDDE210A8E00; Wed, 27 Oct 2021 18:43:10 +0000 (UTC) Received: from [10.97.116.76] (ovpn-116-76.gru2.redhat.com [10.97.116.76]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 30DA590BD; Wed, 27 Oct 2021 18:43:08 +0000 (UTC) Message-ID: <65dc721a-fa58-524a-0134-ec4724187284@redhat.com> Date: Wed, 27 Oct 2021 15:43:04 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 Subject: Re: [PATCH v5] gdb: Add new 'print nibbles' feature To: Enze Li , gdb-patches@sourceware.org References: From: Bruno Larsen In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-14.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Oct 2021 18:43:15 -0000 On 10/7/21 11:44, Enze Li via Gdb-patches wrote: > Make an introduction of a new print setting that can be set by 'set > print nibbles [on|off]'. The default value is OFF, and it would be > changed by users manually. > > Of course, 'show print nibbles' is also included in the patch. > > The new feature could display binary values in groups, and each group > has four bits. > > The motivation behind this work is to enable users to more quickly > distinguish the position of any bit on the binary value. > > Here's a GDB session before this patch is applied. > (gdb) print var_a > $1 = 1230 > (gdb) print/t var_a > $2 = 10011001110 > > With this patch applied, we have a new print setting to use. > (gdb) print var_a > $1 = 1230 > (gdb) print/t var_a > $2 = 10011001110 > (gdb) set print nibbles on > (gdb) print/t a > $3 = 100 1100 1110 > > Tested on x86_64-linux(little-endian) and mips-linux(big-endian). > > gdb/ChangeLog: > 2021-09-03 Enze Li > > * NEWS: Metion new command. > * printcmd.c (print_scalar_formatted): Add new parameter and pass > it to print_binary_chars. > * valprint.c (struct value_print_options) : > 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: > 2021-09-03 Enze Li > > * gdb.texinfo (Print Settings): Document 'print nibbles'. > > gdb/testsuite/ChangeLog: > 2021-09-03 Enze Li > > * gdb.base/printcmds.exp: Update to use new > 'print nibbles' command. > * gdb.base/options.exp: Add -nibbles in the print completion list. > --- > gdb/NEWS | 5 ++++ > gdb/doc/gdb.texinfo | 35 ++++++++++++++++++++++++---- > gdb/printcmd.c | 2 +- > gdb/testsuite/gdb.base/options.exp | 1 + > gdb/testsuite/gdb.base/printcmds.exp | 14 +++++++++++ > gdb/valprint.c | 28 +++++++++++++++++++++- > gdb/valprint.h | 6 ++++- > 7 files changed, 84 insertions(+), 7 deletions(-) > > diff --git a/gdb/NEWS b/gdb/NEWS > index 7ca1f842cb1..ef1127da27b 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -170,6 +170,11 @@ set debug event-loop > show debug event-loop > Control the display of debug output about GDB's event loop. > > +set print nibbles [on|off] > +show print nibbles > + This controls whether the 'print/t' command will display binary values > + in groups of four bits, known as "nibbles". The default is 'off'. > + > set print memory-tag-violations > show print memory-tag-violations > Control whether to display additional information about memory tag violations > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index a6f207a41a7..d59fbe13457 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -2141,10 +2141,10 @@ on @code{-} after the command name. For example: > > @smallexample > (@value{GDBP}) print -@key{TAB}@key{TAB} > --address -max-depth -raw-values -union > --array -null-stop -repeats -vtbl > --array-indexes -object -static-members > --elements -pretty -symbol > +-address -max-depth -pretty -symbol > +-array -nibbles -raw-values -union > +-array-indexes -null-stop -repeats -vtbl > +-elements -object -static-members > @end smallexample > > Completion will in some cases guide you with a suggestion of what kind > @@ -10003,6 +10003,10 @@ Set limit on string chars or array elements to print. The value > Set the threshold after which nested structures are replaced with > ellipsis. Related setting: @ref{set print max-depth}. > > +@item -nibbles [@code{on}|@code{off}] > +Set whether to print binary values in groups of four bits, known > +as ``nibbles''. @xref{set print nibbles}. > + > @item -null-stop [@code{on}|@code{off}] > Set printing of char arrays to stop at first null char. Related > setting: @ref{set print null-stop}. > @@ -11361,6 +11365,29 @@ Stop printing element indexes when displaying arrays. > Show whether the index of each element is printed when displaying > arrays. > > +@anchor{set print nibbles} > +@item set print nibbles > +@itemx set print nibbles on > +@cindex print binary values in groups of four bits > +Print binary values in groups of four bits, known as @dfn{nibbles}, > +when using the print command of @value{GDBN} with the option @samp{/t}. > +For example, this is what it looks like with @code{set print nibbles on}: > + > +@smallexample > +@group > +(@value{GDBP}) print val_flags > +$1 = 1230 > +(@value{GDBP}) print/t val_flags > +$2 = 100 1100 1110 > +@end group > +@end smallexample > + > +@item set print nibbles off > +Don't printing binary values in groups. This is the default. > + > +@item show print nibbles > +Show whether to print binary values in groups of four bits. > + > @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 2fe3f4b0cc5..62713f1a91f 100644 > --- a/gdb/printcmd.c > +++ b/gdb/printcmd.c > @@ -490,7 +490,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/gdb.base/options.exp b/gdb/testsuite/gdb.base/options.exp > index 94372aa9fc7..1a680a3baf4 100644 > --- a/gdb/testsuite/gdb.base/options.exp > +++ b/gdb/testsuite/gdb.base/options.exp > @@ -168,6 +168,7 @@ proc_with_prefix test-print {{prefix ""}} { > "-elements" > "-max-depth" > "-memory-tag-violations" > + "-nibbles" > "-null-stop" > "-object" > "-pretty" > diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp > index b2f90aaff10..75abe79a70d 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_nibbles {} { > + gdb_test_no_output "set print nibbles 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 nibbles off" > +} > + > proc test_print_string_constants {} { > global gdb_prompt > > @@ -1067,6 +1080,7 @@ test_print_int_arrays > test_print_typedef_arrays > test_artificial_arrays > test_print_char_arrays > +test_print_nibbles > # 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 c6ea0d82e40..1e79cc73beb 100644 > --- a/gdb/valprint.c > +++ b/gdb/valprint.c > @@ -108,6 +108,7 @@ struct value_print_options user_print_options = > 0, /* vtblprint */ > 1, /* unionprint */ > 1, /* addressprint */ > + false, /* nibblesprint */ > 0, /* objectprint */ > PRINT_MAX_DEFAULT, /* print_max */ > 10, /* repeat_count_threshold */ > @@ -260,6 +261,17 @@ show_unionprint (struct ui_file *file, int from_tty, > value); > } > > +/* Controls the format of printing binary values. */ > + > +static void > +show_nibbles (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 > @@ -1367,7 +1379,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; > @@ -1390,6 +1403,8 @@ print_binary_chars (struct ui_file *stream, const gdb_byte *valaddr, > > for (i = 0; i < (HOST_CHAR_BIT * sizeof (*p)); i++) > { > + if (options->nibblesprint && seen_a_one && i % 4 == 0) > + fputc_filtered (' ', stream); > if (*p & (mask >> i)) > b = '1'; > else > @@ -1410,6 +1425,8 @@ print_binary_chars (struct ui_file *stream, const gdb_byte *valaddr, > { > for (i = 0; i < (HOST_CHAR_BIT * sizeof (*p)); i++) > { > + if (options->nibblesprint && seen_a_one && i % 4 == 0) > + fputc_filtered (' ', stream); > if (*p & (mask >> i)) > b = '1'; > else > @@ -3016,6 +3033,15 @@ static const gdb::option::option_def value_print_option_defs[] = { > NULL, /* help_doc */ > }, > > + boolean_option_def { > + "nibbles", > + [] (value_print_options *opt) { return &opt->nibblesprint; }, > + show_nibbles, /* show_cmd_cb */ > + N_("Set whether to print binary values in groups of four bits."), > + N_("Show whether to print binary values in groups of four bits."), > + 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 e1dae2cc8fc..bb19c41ab68 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 nibbles. */ > + bool nibblesprint; > + > /* Controls looking up an object's derived type using what we find > in its vtables. */ > bool objectprint; > @@ -149,7 +152,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); > This code LGTM, and doesn't introduce any regressions as far as I could see. That said, I'm a new contributor, so I'm not sure this means all that much. -- Cheers! Bruno Larsen