[PATCH 0/2][PR cli/22640] Have ptype support hex display of offsets and sizes

[PATCH 0/2][PR cli/22640] Have ptype support hex display of offsets and sizes

[Lancelot SIX]

Hi,

Here is a series of patch designed to address PR cli/22640.

The idea is to allow 'ptype' to print offsets and sizes of struct
members using hex notation if desired. Currently, gdb only uses the
decimal notation.

Before:

    (gdb) ptype /o struct type_print_options
    /* offset    |  size */  type = struct type_print_options {
    /*    0: 0   |     4 */    unsigned int raw : 1;
    /*    0: 1   |     4 */    unsigned int print_methods : 1;
    /*    0: 2   |     4 */    unsigned int print_typedefs : 1;
    /*    0: 3   |     4 */    unsigned int print_offsets : 1;
    /*    0: 4   |     4 */    unsigned int print_in_hex : 1;
    /* XXX  3-bit hole   */
    /* XXX  3-byte hole  */
    /*    4      |     4 */    int print_nested_type_limit;
    /*    8      |     8 */    typedef_hash_table *local_typedefs;
    /*   16      |     8 */    typedef_hash_table *global_typedefs;
    /*   24      |     8 */    ext_lang_type_printers *global_printers;
    
                               /* total size (bytes):   32 */
                             }

With the patches and hex option on:

    (gdb) ptype /ox struct type_print_options
    /* offset    |  size */  type = struct type_print_options {
    /* 0x00: 0   |  0x04 */    unsigned int raw : 1;
    /* 0x00: 1   |  0x04 */    unsigned int print_methods : 1;
    /* 0x00: 2   |  0x04 */    unsigned int print_typedefs : 1;
    /* 0x00: 3   |  0x04 */    unsigned int print_offsets : 1;
    /* 0x00: 4   |  0x04 */    unsigned int print_in_hex : 1;
    /* XXX  3-bit hole   */
    /* XXX  3-byte hole  */
    /* 0x04      |  0x04 */    int print_nested_type_limit;
    /* 0x08      |  0x08 */    typedef_hash_table *local_typedefs;
    /* 0x10      |  0x08 */    typedef_hash_table *global_typedefs;
    /* 0x18      |  0x08 */    ext_lang_type_printers *global_printers;
    
                               /* total size (bytes):   32 */
                             }

This patch series also allows the user to set the default behavior using
the 'set print type hex' command.

I am happy to discuss any improvement on the commands and options names
if desired.

The first patch of the series only swaps two types declaration (leaving
them unchanged otherwise) because the second will rely on this new order.

My copyright assignment have not yet been finalized, I’ll notify
maintainers as soon as it is.

Lancelot SIX (2):
  typeprint.h: reorder struct declaration
  ptype: add option to use hexadecimal notation

 gdb/c-typeprint.c                        |   8 +-
 gdb/doc/gdb.texinfo                      |  27 ++++++
 gdb/rust-lang.c                          |   2 +-
 gdb/testsuite/gdb.base/ptype-offsets.exp | 114 +++++++++++++++++++++++
 gdb/typeprint.c                          |  72 +++++++++++++-
 gdb/typeprint.h                          |  69 ++++++++------
 6 files changed, 252 insertions(+), 40 deletions(-)

-- 
2.29.2

[PATCH 1/2] typeprint.h: reorder struct declaration

[Lancelot SIX]

Move the declaration of struct type_print_raw_options before struct
print_offset_data to ease upcoming changes. This is a helper commit
intended to make it easier to build a print_offset_data object from
configurations given by a type_print_raw_options.

gdb/ChangeLog:

	* typeprint.h (struct type_print_options): move before
	  print_offsetdata
---
 gdb/typeprint.h | 60 ++++++++++++++++++++++++-------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/gdb/typeprint.h b/gdb/typeprint.h
index d595cbe208..b75fa4d9f2 100644
--- a/gdb/typeprint.h
+++ b/gdb/typeprint.h
@@ -26,6 +26,36 @@ struct ui_file;
 struct typedef_hash_table;
 struct ext_lang_type_printers;
 
+struct type_print_options
+{
+  /* True means that no special printing flags should apply.  */
+  unsigned int raw : 1;
+
+  /* True means print methods in a class.  */
+  unsigned int print_methods : 1;
+
+  /* True means print typedefs in a class.  */
+  unsigned int print_typedefs : 1;
+
+  /* True means to print offsets, a la 'pahole'.  */
+  unsigned int print_offsets : 1;
+
+  /* The number of nested type definitions to print.  -1 == all.  */
+  int print_nested_type_limit;
+
+  /* If not NULL, a local typedef hash table used when printing a
+     type.  */
+  typedef_hash_table *local_typedefs;
+
+  /* If not NULL, a global typedef hash table used when printing a
+     type.  */
+  typedef_hash_table *global_typedefs;
+
+  /* The list of type printers associated with the global typedef
+     table.  This is intentionally opaque.  */
+  struct ext_lang_type_printers *global_printers;
+};
+
 struct print_offset_data
 {
   /* The offset to be applied to bitpos when PRINT_OFFSETS is true.
@@ -73,36 +103,6 @@ struct print_offset_data
 			 const char *for_what);
 };
 
-struct type_print_options
-{
-  /* True means that no special printing flags should apply.  */
-  unsigned int raw : 1;
-
-  /* True means print methods in a class.  */
-  unsigned int print_methods : 1;
-
-  /* True means print typedefs in a class.  */
-  unsigned int print_typedefs : 1;
-
-  /* True means to print offsets, a la 'pahole'.  */
-  unsigned int print_offsets : 1;
-
-  /* The number of nested type definitions to print.  -1 == all.  */
-  int print_nested_type_limit;
-
-  /* If not NULL, a local typedef hash table used when printing a
-     type.  */
-  typedef_hash_table *local_typedefs;
-
-  /* If not NULL, a global typedef hash table used when printing a
-     type.  */
-  typedef_hash_table *global_typedefs;
-
-  /* The list of type printers associated with the global typedef
-     table.  This is intentionally opaque.  */
-  struct ext_lang_type_printers *global_printers;
-};
-
 extern const struct type_print_options type_print_raw_options;
 
 /* A hash table holding typedef_field objects.  This is more
-- 
2.29.2

Re: [PATCH 1/2] typeprint.h: reorder struct declaration

[Lancelot SIX]

Hi

On 31/12/2020 12:53, Lancelot SIX via Gdb-patches wrote:
> Move the declaration of struct type_print_raw_options before struct
> print_offset_data to ease upcoming changes. This is a helper commit
> intended to make it easier to build a print_offset_data object from
> configurations given by a type_print_raw_options.
>
> gdb/ChangeLog:
>
> 	* typeprint.h (struct type_print_options): move before
> 	  print_offsetdata

Just realized I had formatting issues here. I’ll change that ASAP, with 
other elements that might come with reviews.

When doing such modification, do I re-send the entire patches series or 
just the one affected? (I am not yet familiar with working with 
mailing-list based workflows).

BR

Lancelot

> ---
>   gdb/typeprint.h | 60 ++++++++++++++++++++++++-------------------------
>   1 file changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/gdb/typeprint.h b/gdb/typeprint.h
> index d595cbe208..b75fa4d9f2 100644
> --- a/gdb/typeprint.h
> +++ b/gdb/typeprint.h
> @@ -26,6 +26,36 @@ struct ui_file;
>   struct typedef_hash_table;
>   struct ext_lang_type_printers;
>   
> +struct type_print_options
> +{
> +  /* True means that no special printing flags should apply.  */
> +  unsigned int raw : 1;
> +
> +  /* True means print methods in a class.  */
> +  unsigned int print_methods : 1;
> +
> +  /* True means print typedefs in a class.  */
> +  unsigned int print_typedefs : 1;
> +
> +  /* True means to print offsets, a la 'pahole'.  */
> +  unsigned int print_offsets : 1;
> +
> +  /* The number of nested type definitions to print.  -1 == all.  */
> +  int print_nested_type_limit;
> +
> +  /* If not NULL, a local typedef hash table used when printing a
> +     type.  */
> +  typedef_hash_table *local_typedefs;
> +
> +  /* If not NULL, a global typedef hash table used when printing a
> +     type.  */
> +  typedef_hash_table *global_typedefs;
> +
> +  /* The list of type printers associated with the global typedef
> +     table.  This is intentionally opaque.  */
> +  struct ext_lang_type_printers *global_printers;
> +};
> +
>   struct print_offset_data
>   {
>     /* The offset to be applied to bitpos when PRINT_OFFSETS is true.
> @@ -73,36 +103,6 @@ struct print_offset_data
>   			 const char *for_what);
>   };
>   
> -struct type_print_options
> -{
> -  /* True means that no special printing flags should apply.  */
> -  unsigned int raw : 1;
> -
> -  /* True means print methods in a class.  */
> -  unsigned int print_methods : 1;
> -
> -  /* True means print typedefs in a class.  */
> -  unsigned int print_typedefs : 1;
> -
> -  /* True means to print offsets, a la 'pahole'.  */
> -  unsigned int print_offsets : 1;
> -
> -  /* The number of nested type definitions to print.  -1 == all.  */
> -  int print_nested_type_limit;
> -
> -  /* If not NULL, a local typedef hash table used when printing a
> -     type.  */
> -  typedef_hash_table *local_typedefs;
> -
> -  /* If not NULL, a global typedef hash table used when printing a
> -     type.  */
> -  typedef_hash_table *global_typedefs;
> -
> -  /* The list of type printers associated with the global typedef
> -     table.  This is intentionally opaque.  */
> -  struct ext_lang_type_printers *global_printers;
> -};
> -
>   extern const struct type_print_options type_print_raw_options;
>   
>   /* A hash table holding typedef_field objects.  This is more

Re: [PATCH 1/2] typeprint.h: reorder struct declaration

[Andrew Burgess]

* Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org> [2020-12-31 13:01:26 +0000]:

> Hi
> 
> On 31/12/2020 12:53, Lancelot SIX via Gdb-patches wrote:
> > Move the declaration of struct type_print_raw_options before struct
> > print_offset_data to ease upcoming changes. This is a helper commit
> > intended to make it easier to build a print_offset_data object from
> > configurations given by a type_print_raw_options.
> > 
> > gdb/ChangeLog:
> > 
> > 	* typeprint.h (struct type_print_options): move before
> > 	  print_offsetdata
> 
> Just realized I had formatting issues here. I’ll change that ASAP, with
> other elements that might come with reviews.

Good spot :) Just to ensure we're in sync, there's a white space issue
on line #2, and you should capitalise 'Move', and end with a full stop.

> 
> When doing such modification, do I re-send the entire patches series or just
> the one affected? (I am not yet familiar with working with mailing-list
> based workflows).

When it is just one patch changing independent of the others, and if
the changes are not huge (so minor clean ups), I would normally just
reply to the original patch with a new version.

When I've revised multiple patches, or if I make large changes to a
patch, I'll post the whole series again with a v2, v3, etc added.

Otherwise, this first patch looks fine.

Thanks,
Andrew

> 
> BR
> 
> Lancelot
> 
> > ---
> >   gdb/typeprint.h | 60 ++++++++++++++++++++++++-------------------------
> >   1 file changed, 30 insertions(+), 30 deletions(-)
> > 
> > diff --git a/gdb/typeprint.h b/gdb/typeprint.h
> > index d595cbe208..b75fa4d9f2 100644
> > --- a/gdb/typeprint.h
> > +++ b/gdb/typeprint.h
> > @@ -26,6 +26,36 @@ struct ui_file;
> >   struct typedef_hash_table;
> >   struct ext_lang_type_printers;
> > +struct type_print_options
> > +{
> > +  /* True means that no special printing flags should apply.  */
> > +  unsigned int raw : 1;
> > +
> > +  /* True means print methods in a class.  */
> > +  unsigned int print_methods : 1;
> > +
> > +  /* True means print typedefs in a class.  */
> > +  unsigned int print_typedefs : 1;
> > +
> > +  /* True means to print offsets, a la 'pahole'.  */
> > +  unsigned int print_offsets : 1;
> > +
> > +  /* The number of nested type definitions to print.  -1 == all.  */
> > +  int print_nested_type_limit;
> > +
> > +  /* If not NULL, a local typedef hash table used when printing a
> > +     type.  */
> > +  typedef_hash_table *local_typedefs;
> > +
> > +  /* If not NULL, a global typedef hash table used when printing a
> > +     type.  */
> > +  typedef_hash_table *global_typedefs;
> > +
> > +  /* The list of type printers associated with the global typedef
> > +     table.  This is intentionally opaque.  */
> > +  struct ext_lang_type_printers *global_printers;
> > +};
> > +
> >   struct print_offset_data
> >   {
> >     /* The offset to be applied to bitpos when PRINT_OFFSETS is true.
> > @@ -73,36 +103,6 @@ struct print_offset_data
> >   			 const char *for_what);
> >   };
> > -struct type_print_options
> > -{
> > -  /* True means that no special printing flags should apply.  */
> > -  unsigned int raw : 1;
> > -
> > -  /* True means print methods in a class.  */
> > -  unsigned int print_methods : 1;
> > -
> > -  /* True means print typedefs in a class.  */
> > -  unsigned int print_typedefs : 1;
> > -
> > -  /* True means to print offsets, a la 'pahole'.  */
> > -  unsigned int print_offsets : 1;
> > -
> > -  /* The number of nested type definitions to print.  -1 == all.  */
> > -  int print_nested_type_limit;
> > -
> > -  /* If not NULL, a local typedef hash table used when printing a
> > -     type.  */
> > -  typedef_hash_table *local_typedefs;
> > -
> > -  /* If not NULL, a global typedef hash table used when printing a
> > -     type.  */
> > -  typedef_hash_table *global_typedefs;
> > -
> > -  /* The list of type printers associated with the global typedef
> > -     table.  This is intentionally opaque.  */
> > -  struct ext_lang_type_printers *global_printers;
> > -};
> > -
> >   extern const struct type_print_options type_print_raw_options;
> >   /* A hash table holding typedef_field objects.  This is more

[PATCH 2/2] ptype: add option to use hexadecimal notation

[Lancelot SIX]

This commit adds a flag to the ptype command in order to print the
offsets and sizes of struct members using the hexadecimal notation. The
'x' flag ensures use of the hexadecimal notation while the 'X' flag
ensures use of the decimal notation. The default is to use decimal
notation.

Before this patch, gdb only uses decimal notation, as pointed out in bug
cli/22640.

Here is an example of this new behavior with hex output turned on:

	(gdb) ptype /ox struct type_print_options
	/* offset    |  size */  type = struct type_print_options {
	/* 0x00: 0   |  0x04 */    unsigned int raw : 1;
	/* 0x00: 1   |  0x04 */    unsigned int print_methods : 1;
	/* 0x00: 2   |  0x04 */    unsigned int print_typedefs : 1;
	/* 0x00: 3   |  0x04 */    unsigned int print_offsets : 1;
	/* 0x00: 4   |  0x04 */    unsigned int print_in_hex : 1;
	/* XXX  3-bit hole   */
	/* XXX  3-byte hole  */
	/* 0x04      |  0x04 */    int print_nested_type_limit;
	/* 0x08      |  0x08 */    typedef_hash_table *local_typedefs;
	/* 0x10      |  0x08 */    typedef_hash_table *global_typedefs;
	/* 0x18      |  0x08 */    ext_lang_type_printers *global_printers;

	                           /* total size (bytes):   32 */
	                         }

This patch also adds the 'set print type hex' and 'show print type hex'
commands in order to set and inspect the default behavior regarding the
use of decimal or hexadecimal notation when printing struct sizes and
offsets.

Tested using 'runtest -tool gdb ptype-offsets.exp'

gdb/ChangeLog:

	PR cli/22640
	* typeprint.h (struct type_print_options): Add print_in_hex
	flag.
	(struct print_offset_data): Add print_in_hex flag, add a
	constructor accepting a type_print_options* argument.
	* typeprint.c (print_offset_data::print_offset_data): Add
	ctor.
	(print_offset_data::update): Handle print_in_hex flag when
	printing unions.
	(whatis_exp): Handle 'x' and 'X' flags.
	(set_print_offsets_and_sizes_in_hex): Create.
	(show_print_offsets_and_sizes_in_hex): Create.
	(_initialize_typeprint): Update help message for the ptype
	command, register the 'set print type hex' and 'show print type
	hex' commands.
	* c-typeprint.c (c_print_type): Construct the print_offset_data
	object using the type_print_optons parameter.
	(c_type_print_base_struct_union): Construct the print_offset_data
	object using the type_print_optons parameter.
	(c_type_print_base): Construct the print_offset_data object using
	the type_print_optons parameter.
	* rust-lang.c (rust_language::print_type): Construct the
	print_offset_data object using the type_print_optons parameter.

gdb/doc/ChangeLog:

	PR cli/22640
	* gdb.texinfo (Symbols): Describe the 'x' and 'X' flags of the
	ptype command, describe 'set print type hex' and 'show print
	type hex' commands.

gdb/testsuite/ChangeLog:

	PR cli/22640
	* gdb.base/ptype-offsets.exp: Add tests to verify the behavior
	of 'ptype/ox' and 'ptype/oX'. Check that 'set print type hex'
	changes the default behavior of 'ptype/o'.
---
 gdb/c-typeprint.c                        |   8 +-
 gdb/doc/gdb.texinfo                      |  27 ++++++
 gdb/rust-lang.c                          |   2 +-
 gdb/testsuite/gdb.base/ptype-offsets.exp | 114 +++++++++++++++++++++++
 gdb/typeprint.c                          |  72 +++++++++++++-
 gdb/typeprint.h                          |   9 ++
 6 files changed, 222 insertions(+), 10 deletions(-)

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index b6f1eee760..521e7d1538 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -171,7 +171,7 @@ c_print_type (struct type *type,
 	      int show, int level,
 	      const struct type_print_options *flags)
 {
-  struct print_offset_data podata;
+  struct print_offset_data podata (flags);
 
   c_print_type_1 (type, varstring, stream, show, level,
 		  current_language->la_language, flags, &podata);
@@ -188,7 +188,7 @@ c_print_type (struct type *type,
 	      enum language language,
 	      const struct type_print_options *flags)
 {
-  struct print_offset_data podata;
+  struct print_offset_data podata (flags);
 
   c_print_type_1 (type, varstring, stream, show, level, language, flags,
 		  &podata);
@@ -1149,7 +1149,7 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
       int len = type->num_fields ();
       vptr_fieldno = get_vptr_fieldno (type, &basetype);
 
-      struct print_offset_data local_podata;
+      struct print_offset_data local_podata (flags);
 
       for (int i = TYPE_N_BASECLASSES (type); i < len; i++)
 	{
@@ -1724,7 +1724,7 @@ c_type_print_base (struct type *type, struct ui_file *stream,
 		   int show, int level,
 		   const struct type_print_options *flags)
 {
-  struct print_offset_data podata;
+  struct print_offset_data podata (flags);
 
   c_type_print_base_1 (type, stream, show, level,
 		       current_language->la_language, flags, &podata);
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 93e722881a..35f7b6bf62 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18748,6 +18748,25 @@ types.
 This command shows the current setting of typedef display when
 printing classes.
 
+@kindex set print type hex
+@item set print type hex
+@itemx set print type hex on
+@itemx set print type hex off
+
+When @value{GDBN} prints sizes and offsets of struct members, decimal or
+hexadecimal notations can be used.  Selecting one or the other can be
+done either by passing the appropriate flag to @code{ptype}, or by using
+@command{set print type hex}.  Specifying @code{on} causes @value{GDBN}
+to print sizes and offsets of struct members using hexadecimal notation.
+Specifying @code{off} causes @value{GDBN} to print sizes and offsets of
+struct members using decimal notation; this is the default.
+
+@kindex show print type hex
+@item show print type hex
+This command show the current setting used to configure whether decimal or
+hexadecimal notation should be used when printing sized and offsets of
+struct members.
+
 @kindex info address
 @cindex address of a symbol
 @item info address @var{symbol}
@@ -18860,6 +18879,14 @@ exists in case you change the default with @command{set print type typedefs}.
 Print the offsets and sizes of fields in a struct, similar to what the
 @command{pahole} tool does.  This option implies the @code{/tm} flags.
 
+@item x
+Use hexadecimal notation when printing offsets and sizes of fields in a
+struct.
+
+@item X
+Use decimal notation when printing offsets and sizes of fields in a
+struct.
+
 For example, given the following declarations:
 
 @smallexample
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index cda739760c..a73d41f8a6 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -1906,7 +1906,7 @@ rust_language::print_type (struct type *type, const char *varstring,
 			   struct ui_file *stream, int show, int level,
 			   const struct type_print_options *flags) const
 {
-  print_offset_data podata;
+  print_offset_data podata (flags);
   rust_internal_print_type (type, varstring, stream, show, level,
 			    flags, false, &podata);
 }
diff --git a/gdb/testsuite/gdb.base/ptype-offsets.exp b/gdb/testsuite/gdb.base/ptype-offsets.exp
index 39b467bf9e..0ff24e0981 100644
--- a/gdb/testsuite/gdb.base/ptype-offsets.exp
+++ b/gdb/testsuite/gdb.base/ptype-offsets.exp
@@ -57,6 +57,31 @@ gdb_test "ptype /o struct abc" \
 "                           /* total size (bytes):   56 */" \
 "                         \}"]]
 
+# test "ptype /ox"
+gdb_test "ptype /ox struct abc" \
+    [string_to_regexp [multi_line \
+"/* offset    |  size */  type = struct abc {" \
+"                         public:" \
+"/* 0x08      |  0x08 */    void *field1;" \
+"/* 0x16: 0   |  0x04 */    unsigned int field2 : 1;" \
+"/* XXX  7-bit hole   */" \
+"/* XXX  3-byte hole  */" \
+"/* 0x14      |  0x04 */    int field3;" \
+"/* 0x18      |  0x01 */    signed char field4;" \
+"/* XXX  7-byte hole  */" \
+"/* 0x20      |  0x08 */    uint64_t field5;" \
+"/* 0x28      |  0x08 */    union \{" \
+"/*              0x08 */        void *field6;" \
+"/*              0x04 */        int field7;" \
+"" \
+"                               /* total size (bytes):    8 */" \
+"                           \} field8;" \
+"/* 0x30      |  0x02 */    my_int_type field9;" \
+"/* XXX  6-byte padding  */" \
+"" \
+"                           /* total size (bytes):   56 */" \
+"                         \}"]]
+
 # Test "ptype /oTM".
 gdb_test "ptype /oTM struct abc" \
     [string_to_regexp [multi_line \
@@ -141,6 +166,36 @@ gdb_test "ptype /o struct pqr" \
 "                           /* total size (bytes):   56 */" \
 "                         \}"]]
 
+# Test nested struct with /x
+gdb_test "ptype /ox struct pqr" \
+    [string_to_regexp [multi_line \
+"/* offset    |  size */  type = struct pqr \{" \
+"/* 0x00      |  0x04 */    int ff1;" \
+"/* XXX  4-byte hole  */" \
+"/* 0x08      |  0x28 */    struct xyz \{" \
+"/* 0x08      |  0x04 */        int f1;" \
+"/* 0x0c      |  0x01 */        signed char f2;" \
+"/* XXX  3-byte hole  */" \
+"/* 0x10      |  0x08 */        void *f3;" \
+"/* 0x18      |  0x18 */        struct tuv \{" \
+"/* 0x18      |  0x04 */            int a1;" \
+"/* XXX  4-byte hole  */" \
+"/* 0x20      |  0x08 */            signed char *a2;" \
+"/* 0x28      |  0x04 */            int a3;" \
+"/* XXX  4-byte padding  */" \
+"" \
+"                                   /* total size (bytes):   24 */" \
+"                               \} f4;" \
+"" \
+"                               /* total size (bytes):   40 */" \
+"                           \} ff2;" \
+"/* 0x30      |  0x01 */    signed char ff3;" \
+"/* XXX  7-byte padding  */" \
+"" \
+"                           /* total size (bytes):   56 */" \
+"                         \}"]]
+
+
 # Test that the offset is properly reset when we are printing a union
 # and go inside two inner structs.
 # This also tests a struct inside a struct inside a union.
@@ -340,3 +395,62 @@ gdb_test "ptype/o static_member" \
 "" \
 "                           /* total size (bytes):    4 */" \
 "                         \}"]]
+
+with_test_prefix "with_hex_default" {
+  # Test setting default display to hex
+  send_gdb "set print type hex on\n"
+  gdb_test "show print type hex" \
+           "Display of struct members offsets and sizes in hexadecimal is on"
+
+  # test "ptype /o" is now equivalent to "ptype /ox"
+  gdb_test "ptype /o struct abc" \
+      [string_to_regexp [multi_line \
+  "/* offset    |  size */  type = struct abc \{" \
+  "                         public:" \
+  "/* 0x08      |  0x08 */    void *field1;" \
+  "/* 0x16: 0   |  0x04 */    unsigned int field2 : 1;" \
+  "/* XXX  7-bit hole   */" \
+  "/* XXX  3-byte hole  */" \
+  "/* 0x14      |  0x04 */    int field3;" \
+  "/* 0x18      |  0x01 */    signed char field4;" \
+  "/* XXX  7-byte hole  */" \
+  "/* 0x20      |  0x08 */    uint64_t field5;" \
+  "/* 0x28      |  0x08 */    union \{" \
+  "/*              0x08 */        void *field6;" \
+  "/*              0x04 */        int field7;" \
+  "" \
+  "                               /* total size (bytes):    8 */" \
+  "                           \} field8;" \
+  "/* 0x30      |  0x02 */    my_int_type field9;" \
+  "/* XXX  6-byte padding  */" \
+  "" \
+  "                           /* total size (bytes):   56 */" \
+  "                         \}"]]
+
+  gdb_test "ptype /oX struct abc" \
+      [string_to_regexp [multi_line \
+  "/* offset    |  size */  type = struct abc \{" \
+  "                         public:" \
+  "/*    8      |     8 */    void *field1;" \
+  "/*   16: 0   |     4 */    unsigned int field2 : 1;" \
+  "/* XXX  7-bit hole   */" \
+  "/* XXX  3-byte hole  */" \
+  "/*   20      |     4 */    int field3;" \
+  "/*   24      |     1 */    signed char field4;" \
+  "/* XXX  7-byte hole  */" \
+  "/*   32      |     8 */    uint64_t field5;" \
+  "/*   40      |     8 */    union \{" \
+  "/*                 8 */        void *field6;" \
+  "/*                 4 */        int field7;" \
+  "" \
+  "                               /* total size (bytes):    8 */" \
+  "                           \} field8;" \
+  "/*   48      |     2 */    my_int_type field9;" \
+  "/* XXX  6-byte padding  */" \
+  "" \
+  "                           /* total size (bytes):   56 */" \
+  "                         \}"]]
+
+  # restore
+  send_gdb "set print type hex off\n"
+}
diff --git a/gdb/typeprint.c b/gdb/typeprint.c
index 49877710fa..a92d787f0a 100644
--- a/gdb/typeprint.c
+++ b/gdb/typeprint.c
@@ -44,6 +44,7 @@ const struct type_print_options type_print_raw_options =
   1,				/* print_methods */
   1,				/* print_typedefs */
   0,				/* print_offsets */
+  0,				/* print_in_hex */
   0,				/* print_nested_type_limit  */
   NULL,				/* local_typedefs */
   NULL,				/* global_table */
@@ -58,6 +59,7 @@ static struct type_print_options default_ptype_flags =
   1,				/* print_methods */
   1,				/* print_typedefs */
   0,				/* print_offsets */
+  0,				/* print_in_hex */
   0,				/* print_nested_type_limit  */
   NULL,				/* local_typedefs */
   NULL,				/* global_table */
@@ -70,6 +72,13 @@ static struct type_print_options default_ptype_flags =
 
 const int print_offset_data::indentation = 23;
 
+/* See typeprint.h.  */
+
+print_offset_data::print_offset_data(const struct type_print_options *flags)
+{
+  if (flags != nullptr)
+    print_in_hex = flags->print_in_hex;
+}
 
 /* See typeprint.h.  */
 
@@ -122,7 +131,9 @@ print_offset_data::update (struct type *type, unsigned int field_idx,
       /* Since union fields don't have the concept of offsets, we just
 	 print their sizes.  */
       fprintf_filtered (stream, "/*              %4s */",
-			pulongest (TYPE_LENGTH (ftype)));
+			(print_in_hex ?
+			 hex_string_custom (TYPE_LENGTH (ftype), 2) :
+			 pulongest (TYPE_LENGTH (ftype))));
       return;
     }
 
@@ -140,20 +151,23 @@ print_offset_data::update (struct type *type, unsigned int field_idx,
 
       unsigned real_bitpos = bitpos + offset_bitpos;
 
-      fprintf_filtered (stream, "/* %4u:%2u", real_bitpos / TARGET_CHAR_BIT,
+      fprintf_filtered (stream,
+			(print_in_hex ? "/* 0x%02u:%2u" : "/* %4u:%2u"),
+			real_bitpos / TARGET_CHAR_BIT,
 			real_bitpos % TARGET_CHAR_BIT);
     }
   else
     {
       /* The position of the field, relative to the beginning of the
 	 struct.  */
-      fprintf_filtered (stream, "/* %4u",
+      fprintf_filtered (stream, (print_in_hex ?  "/* 0x%02x" : "/* %4u"),
 			(bitpos + offset_bitpos) / TARGET_CHAR_BIT);
 
       fprintf_filtered (stream, "   ");
     }
 
-  fprintf_filtered (stream, "   |  %4u */", fieldsize_byte);
+  fprintf_filtered (stream, (print_in_hex ? "   |  0x%02x */" : "   |  %4u */"),
+		    fieldsize_byte);
 
   end_bitpos = bitpos + fieldsize_bit;
 }
@@ -468,6 +482,12 @@ whatis_exp (const char *exp, int show)
 		      }
 		    break;
 		  }
+		case 'x':
+		  flags.print_in_hex = 1;
+		  break;
+		case 'X':
+		  flags.print_in_hex = 0;
+		  break;
 		default:
 		  error (_("unrecognized flag '%c'"), *exp);
 		}
@@ -763,6 +783,35 @@ show_print_type_nested_types  (struct ui_file *file, int from_tty,
     }
 }
 
+/* When printing structs, offsets and sizes of members can be displayed using
+   decimal notation or hexadecimal notation. By default, Decimal notation is
+   used. */
+
+static bool print_offsets_and_sizes_in_hex = false;
+
+/* Set the flags that instructs if sizes and offsets of struct members are
+   displayed in hexadecimal or decimal notation. */
+
+static void
+set_print_offsets_and_sizes_in_hex (const char *args,
+				    int from_tty, struct cmd_list_element *c)
+{
+  default_ptype_flags.print_in_hex = print_offsets_and_sizes_in_hex;
+}
+
+/* Display whether struct members sizes and offsets are printed
+   using decimal or hexadecimal notation */
+
+static void
+show_print_offsets_and_sizes_in_hex (struct ui_file *file, int from_tty,
+				     struct cmd_list_element *c,
+				     const char *value)
+{
+  fprintf_filtered (file, _("\
+Display of struct members offsets and sizes in hexadecimal is %s\n"),
+		    value);
+}
+
 void _initialize_typeprint ();
 void
 _initialize_typeprint ()
@@ -784,7 +833,11 @@ Available FLAGS are:\n\
   /M    print methods defined in a class\n\
   /t    do not print typedefs defined in a class\n\
   /T    print typedefs defined in a class\n\
-  /o    print offsets and sizes of fields in a struct (like pahole)"));
+  /o    print offsets and sizes of fields in a struct (like pahole)\n\
+  /x    use hexadecimal notation when displaying sizes and offsets\n\
+        of struct members\n\
+  /X    use decimal notation when displaying sizes and offsets\n\
+        of struct members "));
   set_cmd_completer (c, expression_completer);
 
   c = add_com ("whatis", class_vars, whatis_command,
@@ -825,6 +878,15 @@ Show the number of recursive nested type definitions to print."), NULL,
 				       set_print_type_nested_types,
 				       show_print_type_nested_types,
 				       &setprinttypelist, &showprinttypelist);
+
+  add_setshow_boolean_cmd ("hex", no_class, &print_offsets_and_sizes_in_hex,
+			   _("Set printing of struct members sizes and offsets \
+			     using hex notation"),
+			   _("Show printing of struct members sizes and offsets \
+			     using hex notation"), nullptr,
+			   set_print_offsets_and_sizes_in_hex,
+			   show_print_offsets_and_sizes_in_hex,
+			   &setprinttypelist, &showprinttypelist);
 }
 
 /* Print <not allocated> status to stream STREAM.  */
diff --git a/gdb/typeprint.h b/gdb/typeprint.h
index b75fa4d9f2..343d404b15 100644
--- a/gdb/typeprint.h
+++ b/gdb/typeprint.h
@@ -40,6 +40,9 @@ struct type_print_options
   /* True means to print offsets, a la 'pahole'.  */
   unsigned int print_offsets : 1;
 
+  /* True means to print offsets in hex, otherwise use decimal */
+  unsigned int print_in_hex : 1;
+
   /* The number of nested type definitions to print.  -1 == all.  */
   int print_nested_type_limit;
 
@@ -58,6 +61,10 @@ struct type_print_options
 
 struct print_offset_data
 {
+  /* Indicate if the offset and size fields should be printed in decimal
+     (default) or hexadecimal. */
+  bool print_in_hex  = false;
+
   /* The offset to be applied to bitpos when PRINT_OFFSETS is true.
      This is needed for when we are printing nested structs and want
      to make sure that the printed offset for each field carries over
@@ -92,6 +99,8 @@ struct print_offset_data
      certain field.  */
   static const int indentation;
 
+  explicit print_offset_data (const struct type_print_options *flags);
+
 private:
 
   /* Helper function for ptype/o implementation that prints
-- 
2.29.2

Re: [PATCH 2/2] ptype: add option to use hexadecimal notation

[John Baldwin]

On 12/31/20 4:53 AM, Lancelot SIX via Gdb-patches wrote:
> This commit adds a flag to the ptype command in order to print the
> offsets and sizes of struct members using the hexadecimal notation. The
> 'x' flag ensures use of the hexadecimal notation while the 'X' flag
> ensures use of the decimal notation. The default is to use decimal
> notation.
> 
> Before this patch, gdb only uses decimal notation, as pointed out in bug
> cli/22640.
> 
> Here is an example of this new behavior with hex output turned on:
> 
> 	(gdb) ptype /ox struct type_print_options
> 	/* offset    |  size */  type = struct type_print_options {
> 	/* 0x00: 0   |  0x04 */    unsigned int raw : 1;
> 	/* 0x00: 1   |  0x04 */    unsigned int print_methods : 1;
> 	/* 0x00: 2   |  0x04 */    unsigned int print_typedefs : 1;
> 	/* 0x00: 3   |  0x04 */    unsigned int print_offsets : 1;
> 	/* 0x00: 4   |  0x04 */    unsigned int print_in_hex : 1;
> 	/* XXX  3-bit hole   */
> 	/* XXX  3-byte hole  */
> 	/* 0x04      |  0x04 */    int print_nested_type_limit;
> 	/* 0x08      |  0x08 */    typedef_hash_table *local_typedefs;
> 	/* 0x10      |  0x08 */    typedef_hash_table *global_typedefs;
> 	/* 0x18      |  0x08 */    ext_lang_type_printers *global_printers;
> 
> 	                           /* total size (bytes):   32 */
> 	                         }
> 
> This patch also adds the 'set print type hex' and 'show print type hex'
> commands in order to set and inspect the default behavior regarding the
> use of decimal or hexadecimal notation when printing struct sizes and
> offsets.
> 
> Tested using 'runtest -tool gdb ptype-offsets.exp'

Thanks for working on this.  In general I think this looks fine in
terms of new print flags, etc.  My only concern might be that a
single byte worth of space might be a bit short.  I frequently
use ptype with some larger data structures whose sizes in hex
run to 3 digits and would distort the formatting as a result.

What would you think of using 6 character fields for hex values
such as 0x%04x?  It would mean the entire display would shift left
by 4 characters and the header would have to be adjusted.  Alternatively
if you are really concerned about horizontal space you could perhaps
only expand the offset portion to 4 hex digits?

The other possible consideration is to not print leading zeroes
but instead use '%6s' with hex_string() for the hex fields.

-- 
John Baldwin

Re: [PATCH 2/2] ptype: add option to use hexadecimal notation

[Paul Koning]

> On Apr 13, 2021, at 7:18 PM, John Baldwin <jhb@FreeBSD.org> wrote:
> 
> On 12/31/20 4:53 AM, Lancelot SIX via Gdb-patches wrote:
>> This commit adds a flag to the ptype command in order to print the
>> offsets and sizes of struct members using the hexadecimal notation. The
>> 'x' flag ensures use of the hexadecimal notation while the 'X' flag
>> ensures use of the decimal notation. The default is to use decimal
>> notation.

"X" for decimal is rather non-obvious.  How about "d" instead?  Usually X means uppercase hexadecimal.

	paul

Re: [PATCH 2/2] ptype: add option to use hexadecimal notation

[Lancelot SIX]

On Tue, Apr 13, 2021 at 08:08:58PM -0400, Paul Koning wrote:
> 
> 
> > On Apr 13, 2021, at 7:18 PM, John Baldwin <jhb@FreeBSD.org> wrote:
> > 
> > On 12/31/20 4:53 AM, Lancelot SIX via Gdb-patches wrote:
> >> This commit adds a flag to the ptype command in order to print the
> >> offsets and sizes of struct members using the hexadecimal notation. The
> >> 'x' flag ensures use of the hexadecimal notation while the 'X' flag
> >> ensures use of the decimal notation. The default is to use decimal
> >> notation.
> 
> "X" for decimal is rather non-obvious.  How about "d" instead?  Usually X means uppercase hexadecimal.
> 
> 	paul
> 
> 

Hi,

I went for the x/X since other options use the lowercase/uppercase to
activate/deactivate flags (m/M or t/T).  If this is more confusing than
helping, I am fine using 'd' to force decimal notation.  I am just a bit
concerned that with those 2 distinct letters, one could expects to find
two flags ('set print type hex' and 'set print type decimal'), where
only one actually exists.  Is this me overthinking it?

Lancelot.

Re: [PATCH 2/2] ptype: add option to use hexadecimal notation

[Paul Koning]

> On Apr 14, 2021, at 6:45 PM, Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org> wrote:
> 
> On Tue, Apr 13, 2021 at 08:08:58PM -0400, Paul Koning wrote:
>> 
>> 
>>> On Apr 13, 2021, at 7:18 PM, John Baldwin <jhb@FreeBSD.org> wrote:
>>> 
>>> On 12/31/20 4:53 AM, Lancelot SIX via Gdb-patches wrote:
>>>> This commit adds a flag to the ptype command in order to print the
>>>> offsets and sizes of struct members using the hexadecimal notation. The
>>>> 'x' flag ensures use of the hexadecimal notation while the 'X' flag
>>>> ensures use of the decimal notation. The default is to use decimal
>>>> notation.
>> 
>> "X" for decimal is rather non-obvious.  How about "d" instead?  Usually X means uppercase hexadecimal.
> 
> I went for the x/X since other options use the lowercase/uppercase to
> activate/deactivate flags (m/M or t/T).  If this is more confusing than
> helping, I am fine using 'd' to force decimal notation.  I am just a bit
> concerned that with those 2 distinct letters, one could expects to find
> two flags ('set print type hex' and 'set print type decimal'), where
> only one actually exists.  Is this me overthinking it?
> 
> Lancelot.

I didn't realize there's an existing pattern like that.  Given that precedent, it seems that what you're proposing is fine and matches what's already done elsewhere.  

	paul

Re: [PATCH 2/2] ptype: add option to use hexadecimal notation

[Lancelot SIX]

On Tue, Apr 13, 2021 at 04:18:24PM -0700, John Baldwin wrote:
> On 12/31/20 4:53 AM, Lancelot SIX via Gdb-patches wrote:
> > This commit adds a flag to the ptype command in order to print the
> > offsets and sizes of struct members using the hexadecimal notation. The
> > 'x' flag ensures use of the hexadecimal notation while the 'X' flag
> > ensures use of the decimal notation. The default is to use decimal
> > notation.
> > 
> > Before this patch, gdb only uses decimal notation, as pointed out in bug
> > cli/22640.
> > 
> > Here is an example of this new behavior with hex output turned on:
> > 
> > 	(gdb) ptype /ox struct type_print_options
> > 	/* offset    |  size */  type = struct type_print_options {
> > 	/* 0x00: 0   |  0x04 */    unsigned int raw : 1;
> > 	/* 0x00: 1   |  0x04 */    unsigned int print_methods : 1;
> > 	/* 0x00: 2   |  0x04 */    unsigned int print_typedefs : 1;
> > 	/* 0x00: 3   |  0x04 */    unsigned int print_offsets : 1;
> > 	/* 0x00: 4   |  0x04 */    unsigned int print_in_hex : 1;
> > 	/* XXX  3-bit hole   */
> > 	/* XXX  3-byte hole  */
> > 	/* 0x04      |  0x04 */    int print_nested_type_limit;
> > 	/* 0x08      |  0x08 */    typedef_hash_table *local_typedefs;
> > 	/* 0x10      |  0x08 */    typedef_hash_table *global_typedefs;
> > 	/* 0x18      |  0x08 */    ext_lang_type_printers *global_printers;
> > 
> > 	                           /* total size (bytes):   32 */
> > 	                         }
> > 
> > This patch also adds the 'set print type hex' and 'show print type hex'
> > commands in order to set and inspect the default behavior regarding the
> > use of decimal or hexadecimal notation when printing struct sizes and
> > offsets.
> > 
> > Tested using 'runtest -tool gdb ptype-offsets.exp'
> 
> Thanks for working on this.  In general I think this looks fine in
> terms of new print flags, etc.  My only concern might be that a
> single byte worth of space might be a bit short.  I frequently
> use ptype with some larger data structures whose sizes in hex
> run to 3 digits and would distort the formatting as a result.
> 
> What would you think of using 6 character fields for hex values
> such as 0x%04x?  It would mean the entire display would shift left
> by 4 characters and the header would have to be adjusted.  Alternatively
> if you are really concerned about horizontal space you could perhaps
> only expand the offset portion to 4 hex digits?
> 
> The other possible consideration is to not print leading zeroes
> but instead use '%6s' with hex_string() for the hex fields.
> 
> -- 
> John Baldwin

Hi,

Thanks for the feedback.

For my initial take at this I kept the overall horizontal layout
unchanged, but it is true that the 0x prefix consumes quite some
space. I’ll increase the horizontal space and prepare a new version
(with the 0x%04x approach I think).

Lancelot.