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

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

[Tom de Vries]

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;
 		      }
 

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

[Simon Marchi]

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

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

[Tom de Vries]

[-- 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

Re: [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

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

[Tom de Vries]

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

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

[Simon Marchi]

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

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

[Simon Marchi]

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