[PATCH][gdb] Print user/includes fields for maint commands
Tom de Vries
tdevries@suse.de
Tue Mar 24 22:55:09 GMT 2020
On 24-03-2020 23:41, Simon Marchi wrote:
> 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`?
>
The first if-else covers printing the block info.
If we'd merge the if-else and the following if, we'd have:
- the true clause related to block info
- the code related to user/includes
- the false clause related to block info
which I find confusing.
> 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).
>
Perhaps later, right now I don't feel knowledgeable enough in this area
to do refactoring.
Thanks,
- Tom
More information about the Gdb-patches
mailing list