[PATCH][gdb] Print user/includes fields for maint commands

[Simon Marchi]

On 2020-03-24 6:28 p.m., Tom de Vries wrote:
> On 24-03-2020 18:04, Simon Marchi wrote:
> 
> Hi,
> 
> thanks for the review, I've followed up on all the comments.
> 
> Updated patch attached below.
> 
> Thanks,
> - Tom
> 
> 
> 0001-gdb-Print-user-includes-fields-for-maint-commands.patch
> 
> [gdb] Print user/includes fields for maint commands
> 
> The type struct compunit_symtab contains two fields (disregarding field next)
> that express relations with other compunit_symtabs: user and includes.
> 
> These fields are currently not printed with "maint info symtabs" and
> "maint print symbols".
> 
> Fix this such that for "maint info symtabs" we print:
> ...
>    { ((struct compunit_symtab *) 0x23e8450)
>      debugformat DWARF 2
>      producer (null)
>      dirname (null)
>      blockvector ((struct blockvector *) 0x23e8590)
> +    user ((struct compunit_symtab *) 0x2336280)
> +    ( includes
> +      ((struct compunit_symtab *) 0x23e85e0)
> +      ((struct compunit_symtab *) 0x23e8960)
> +    )
>          { symtab <unknown> ((struct symtab *) 0x23e85b0)
>            fullname (null)
>            linetable ((struct linetable *) 0x0)
>          }
>    }
> ...
> 
> And for "maint print symbols" we print:
> ...
> -Symtab for file <unknown>
> +Symtab for file <unknown> at 0x23e85b0
>  Read from object file /data/gdb_versions/devel/a.out (0x233ccf0)
>  Language: c
> 
>  Blockvector:
> 
>  block #000, object at 0x23e8530, 0 syms/buckets in 0x0..0x0
>    block #001, object at 0x23e84d0 under 0x23e8530, 0 syms/buckets in 0x0..0x0
> 
> +Compunit user: 0x2336300
> +Compunit include: 0x23e8900
> +Compunit include: 0x23dd970
> ...
> Note: for user and includes we don't list the actual compunit_symtab address,
> but instead the corresponding symtab address, which allows us to find that
> symtab elsewhere in the output (given that we also now print the address of
> symtabs).
> 
> gdb/ChangeLog:
> 
> 2020-03-24  Tom de Vries  <tdevries@suse.de>
> 
> 	* symtab.h (is_main_symtab_of_compunit_symtab): New function.
> 	* symmisc.c (dump_symtab_1): Print user and includes fields.
> 	(maintenance_info_symtabs): Same.
> 
> ---
>  gdb/symmisc.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++---
>  gdb/symtab.h  |  7 +++++++
>  2 files changed, 56 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/symmisc.c b/gdb/symmisc.c
> index 3df526bddb..6ee1b5edfd 100644
> --- a/gdb/symmisc.c
> +++ b/gdb/symmisc.c
> @@ -279,8 +279,10 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
>    const struct block *b;
>    int depth;
>  
> -  fprintf_filtered (outfile, "\nSymtab for file %s\n",
> -		    symtab_to_filename_for_display (symtab));
> +  fprintf_filtered (outfile, "\nSymtab for file %s at %s\n",
> +		    symtab_to_filename_for_display (symtab),
> +		    host_address_to_string (symtab));
> +
>    if (SYMTAB_DIRNAME (symtab) != NULL)
>      fprintf_filtered (outfile, "Compilation directory is %s\n",
>  		      SYMTAB_DIRNAME (symtab));
> @@ -308,7 +310,7 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
>      }
>    /* Now print the block info, but only for compunit symtabs since we will
>       print lots of duplicate info otherwise.  */
> -  if (symtab == COMPUNIT_FILETABS (SYMTAB_COMPUNIT (symtab)))
> +  if (is_main_symtab_of_compunit_symtab (symtab))
>      {
>        fprintf_filtered (outfile, "\nBlockvector:\n\n");
>        bv = SYMTAB_BLOCKVECTOR (symtab);
> @@ -371,6 +373,28 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
>  			"\nBlockvector same as owning compunit: %s\n\n",
>  			compunit_filename);
>      }
> +
> +  if (is_main_symtab_of_compunit_symtab (symtab))
> +    {

Hmm, if this is the exact same condition as the previous `if`, what's the point of
having a separate `if`?

Now that I read this code a bit more, I realize that it's written in this way because
it dates from the time where there wasn't a distinction between compunit_symtabs and
symtabs.  Nowadays, it would probably be preferable to print the compunit_symtab
information at some level higher, in maintenance_print_symbols.  If you want to take the
time to do things right, you can try do to that, otherwise I'm also fine with this
patch (one the question above about the `if` is answered).

Simon