* [PATCH][gdb] Print user/includes fields for maint commands
@ 2020-03-24 11:22 Tom de Vries
2020-03-24 17:04 ` Simon Marchi
0 siblings, 1 reply; 7+ messages in thread
From: Tom de Vries @ 2020-03-24 11:22 UTC (permalink / raw)
To: gdb-patches
Hi,
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).
OK for trunk?
Thanks,
- Tom
[gdb] Print user/includes fields for maint commands
gdb/ChangeLog:
2020-03-24 Tom de Vries <tdevries@suse.de>
* symmisc.c (dump_symtab_1): Print user and includes fields.
(maintenance_info_symtabs): Same.
---
gdb/symmisc.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 55 insertions(+), 1 deletion(-)
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 3df526bddb..f7a36905f2 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -279,8 +279,12 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
const struct block *b;
int depth;
- fprintf_filtered (outfile, "\nSymtab for file %s\n",
+ fprintf_filtered (outfile, "\nSymtab for file %s",
symtab_to_filename_for_display (symtab));
+ fprintf_filtered (outfile, " at ");
+ gdb_print_host_address (symtab, outfile);
+ fprintf_filtered (outfile, "\n");
+
if (SYMTAB_DIRNAME (symtab) != NULL)
fprintf_filtered (outfile, "Compilation directory is %s\n",
SYMTAB_DIRNAME (symtab));
@@ -371,6 +375,32 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
"\nBlockvector same as owning compunit: %s\n\n",
compunit_filename);
}
+
+ if (symtab == COMPUNIT_FILETABS (SYMTAB_COMPUNIT (symtab)))
+ {
+ struct compunit_symtab *cust = SYMTAB_COMPUNIT (symtab);
+
+ if (cust->user)
+ {
+ fprintf_filtered (outfile, "Compunit user: ");
+ gdb_print_host_address (COMPUNIT_FILETABS (cust->user), outfile);
+ fprintf_filtered (outfile, "\n");
+ }
+ if (cust->includes)
+ {
+ struct compunit_symtab *include;
+ for (i = 0; ; ++i)
+ {
+ include = cust->includes[i];
+ if (!include)
+ break;
+ fprintf_filtered (outfile, "Compunit include: ");
+ gdb_print_host_address (COMPUNIT_FILETABS (include), outfile);
+ fprintf_filtered (outfile, "\n");
+ }
+ }
+ }
+
}
static void
@@ -809,6 +839,30 @@ maintenance_info_symtabs (const char *regexp, int from_tty)
" ((struct blockvector *) %s)\n",
host_address_to_string
(COMPUNIT_BLOCKVECTOR (cust)));
+ printf_filtered (" user"
+ " ((struct compunit_symtab *) %s)\n",
+ cust->user
+ ? host_address_to_string (cust->user)
+ : "(null)");
+ if (cust->includes)
+ {
+ struct compunit_symtab *include;
+ int i;
+
+ printf_filtered (" ( includes\n");
+ for (i = 0; ; ++i)
+ {
+ include = cust->includes[i];
+ if (!include)
+ break;
+ const char *addr
+ = host_address_to_string (include);
+ printf_filtered (" (%s %s)\n",
+ "(struct compunit_symtab *)",
+ addr);
+ }
+ printf_filtered (" )\n");
+ }
printed_compunit_symtab_start = 1;
}
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH][gdb] Print user/includes fields for maint commands
2020-03-24 11:22 [PATCH][gdb] Print user/includes fields for maint commands Tom de Vries
@ 2020-03-24 17:04 ` Simon Marchi
2020-03-24 22:28 ` Tom de Vries
0 siblings, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2020-03-24 17:04 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
On 2020-03-24 7:22 a.m., Tom de Vries wrote:
> diff --git a/gdb/symmisc.c b/gdb/symmisc.c
> index 3df526bddb..f7a36905f2 100644
> --- a/gdb/symmisc.c
> +++ b/gdb/symmisc.c
> @@ -279,8 +279,12 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
> const struct block *b;
> int depth;
>
> - fprintf_filtered (outfile, "\nSymtab for file %s\n",
> + fprintf_filtered (outfile, "\nSymtab for file %s",
> symtab_to_filename_for_display (symtab));
> + fprintf_filtered (outfile, " at ");
> + gdb_print_host_address (symtab, outfile);
> + fprintf_filtered (outfile, "\n");
You could use a single fprintf_filtered call, with host_address_to_string.
> +
> if (SYMTAB_DIRNAME (symtab) != NULL)
> fprintf_filtered (outfile, "Compilation directory is %s\n",
> SYMTAB_DIRNAME (symtab));
> @@ -371,6 +375,32 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
> "\nBlockvector same as owning compunit: %s\n\n",
> compunit_filename);
> }
> +
> + if (symtab == COMPUNIT_FILETABS (SYMTAB_COMPUNIT (symtab)))
I find it hard to understand the intent of this line. Would it be possible to introduce
a helper function (probably in symtab.h)? Something like
/* Return true if this symtab is the "main" symtab of its compunit_symtab. */
static inline bool
is_main_symtab_of_compunit_symtab (struct symtab *symtab)
{
return symtab == COMPUNIT_FILETABS (SYMTAB_COMPUNIT (symtab));
}
> + {
> + struct compunit_symtab *cust = SYMTAB_COMPUNIT (symtab);
> +
> + if (cust->user)
!= nullptr
> + {
> + fprintf_filtered (outfile, "Compunit user: ");
> + gdb_print_host_address (COMPUNIT_FILETABS (cust->user), outfile);
> + fprintf_filtered (outfile, "\n");
Single fprintf_filtered call, with host_address_to_string.
> + }
> + if (cust->includes)
!= nullptr
> + {
> + struct compunit_symtab *include;
Declare this within loop, when it's initialized.
> + for (i = 0; ; ++i)
> + {
> + include = cust->includes[i];
> + if (!include)
include == nullptr
> + break;
> + fprintf_filtered (outfile, "Compunit include: ");
> + gdb_print_host_address (COMPUNIT_FILETABS (include), outfile);
> + fprintf_filtered (outfile, "\n");
Single fprintf_filtered called here too.
> + }
> + }
> + }
> +
> }
>
> static void
> @@ -809,6 +839,30 @@ maintenance_info_symtabs (const char *regexp, int from_tty)
> " ((struct blockvector *) %s)\n",
> host_address_to_string
> (COMPUNIT_BLOCKVECTOR (cust)));
> + printf_filtered (" user"
> + " ((struct compunit_symtab *) %s)\n",
> + cust->user
!= nullptr
> + ? host_address_to_string (cust->user)
> + : "(null)");
> + if (cust->includes)
!= nullptr
> + {
> + struct compunit_symtab *include;
> + int i;
Declare both variables at initialize-time.
> +
> + printf_filtered (" ( includes\n");
> + for (i = 0; ; ++i)
> + {
> + include = cust->includes[i];
> + if (!include)
include == nullptr
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH][gdb] Print user/includes fields for maint commands
2020-03-24 17:04 ` Simon Marchi
@ 2020-03-24 22:28 ` Tom de Vries
2020-03-24 22:41 ` Simon Marchi
0 siblings, 1 reply; 7+ messages in thread
From: Tom de Vries @ 2020-03-24 22:28 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
[-- Attachment #1: Type: text/plain, Size: 154 bytes --]
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
[-- Attachment #2: 0001-gdb-Print-user-includes-fields-for-maint-commands.patch --]
[-- Type: text/x-patch, Size: 5237 bytes --]
[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))
+ {
+ struct compunit_symtab *cust = SYMTAB_COMPUNIT (symtab);
+
+ if (cust->user != nullptr)
+ {
+ const char *addr
+ = host_address_to_string (COMPUNIT_FILETABS (cust->user));
+ fprintf_filtered (outfile, "Compunit user: %s\n", addr);
+ }
+ if (cust->includes != nullptr)
+ for (i = 0; ; ++i)
+ {
+ struct compunit_symtab *include = cust->includes[i];
+ if (include == nullptr)
+ break;
+ const char *addr
+ = host_address_to_string (COMPUNIT_FILETABS (include));
+ fprintf_filtered (outfile, "Compunit include: %s\n", addr);
+ }
+ }
}
static void
@@ -809,6 +833,28 @@ maintenance_info_symtabs (const char *regexp, int from_tty)
" ((struct blockvector *) %s)\n",
host_address_to_string
(COMPUNIT_BLOCKVECTOR (cust)));
+ printf_filtered (" user"
+ " ((struct compunit_symtab *) %s)\n",
+ cust->user != nullptr
+ ? host_address_to_string (cust->user)
+ : "(null)");
+ if (cust->includes != nullptr)
+ {
+ printf_filtered (" ( includes\n");
+ for (int i = 0; ; ++i)
+ {
+ struct compunit_symtab *include
+ = cust->includes[i];
+ if (include == nullptr)
+ break;
+ const char *addr
+ = host_address_to_string (include);
+ printf_filtered (" (%s %s)\n",
+ "(struct compunit_symtab *)",
+ addr);
+ }
+ printf_filtered (" )\n");
+ }
printed_compunit_symtab_start = 1;
}
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 771b5ec5bf..18be5d51b8 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1513,6 +1513,13 @@ extern struct symtab *
extern enum language compunit_language (const struct compunit_symtab *cust);
+/* Return true if this symtab is the "main" symtab of its compunit_symtab. */
+
+static inline bool
+is_main_symtab_of_compunit_symtab (struct symtab *symtab)
+{
+ return symtab == COMPUNIT_FILETABS (SYMTAB_COMPUNIT (symtab));
+}
\f
/* The virtual function table is now an array of structures which have the
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH][gdb] Print user/includes fields for maint commands
2020-03-24 22:28 ` Tom de Vries
@ 2020-03-24 22:41 ` Simon Marchi
2020-03-24 22:55 ` Tom de Vries
0 siblings, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2020-03-24 22:41 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH][gdb] Print user/includes fields for maint commands
2020-03-24 22:41 ` Simon Marchi
@ 2020-03-24 22:55 ` Tom de Vries
2020-03-24 23:14 ` Simon Marchi
0 siblings, 1 reply; 7+ messages in thread
From: Tom de Vries @ 2020-03-24 22:55 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH][gdb] Print user/includes fields for maint commands
2020-03-24 22:55 ` Tom de Vries
@ 2020-03-24 23:14 ` Simon Marchi
2020-03-24 23:15 ` Simon Marchi
0 siblings, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2020-03-24 23:14 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
On 2020-03-24 6:55 p.m., Tom de Vries wrote:
> 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.
Ok makes sense. Perhaps then add a comment to introduce that if, to say what's
it's meant to do.
/* Print info about the user of this compunit_symtab, and the compunit_symtabs included by this one. */
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH][gdb] Print user/includes fields for maint commands
2020-03-24 23:14 ` Simon Marchi
@ 2020-03-24 23:15 ` Simon Marchi
0 siblings, 0 replies; 7+ messages in thread
From: Simon Marchi @ 2020-03-24 23:15 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
On 2020-03-24 7:14 p.m., Simon Marchi wrote:
> On 2020-03-24 6:55 p.m., Tom de Vries wrote:
>> 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.
>
> Ok makes sense. Perhaps then add a comment to introduce that if, to say what's
> it's meant to do.
>
> /* Print info about the user of this compunit_symtab, and the compunit_symtabs included by this one. */
>
> Simon
>
I forgot to mention, the patch is ok with that added.
Thanks,
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-03-24 23:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-24 11:22 [PATCH][gdb] Print user/includes fields for maint commands Tom de Vries
2020-03-24 17:04 ` Simon Marchi
2020-03-24 22:28 ` Tom de Vries
2020-03-24 22:41 ` Simon Marchi
2020-03-24 22:55 ` Tom de Vries
2020-03-24 23:14 ` Simon Marchi
2020-03-24 23:15 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox