diff mbox

Add "set debug minsyms" command

Message ID fe43f1a7-024e-0e98-c0c5-21ed718b452f@ericsson.com
State New, archived
Headers

Commit Message

Simon Marchi Dec. 21, 2018, 11:07 p.m. UTC 
  On 2018-12-21 4:59 p.m., John Baldwin wrote:
>> diff --git a/gdb/auto-load.c b/gdb/auto-load.c
>> index 33d282afe83..e35fd29426b 100644
>> --- a/gdb/auto-load.c
>> +++ b/gdb/auto-load.c
>> @@ -1175,6 +1175,8 @@ load_auto_scripts_for_objfile (struct objfile *objfile)
>>  static void
>>  auto_load_new_objfile (struct objfile *objfile)
>>  {
>> +  std::vector <int> c;
>> +  c.clear();
>>    if (!objfile)
>>      {
>>        /* OBJFILE is NULL when loading a new "main" symbol-file.  */
> 
> This seems spurious (also not in ChangeLog)?
> 
>> diff --git a/gdb/elfread.c b/gdb/elfread.c
>> index 71e6fcca6ec..359089b166c 100644
>> --- a/gdb/elfread.c
>> +++ b/gdb/elfread.c
>> @@ -249,6 +249,8 @@ elf_symtab_read (minimal_symbol_reader &reader,
>>  	  continue;
>>  	}
>>  
>> +      printf(" --- %s\n", sym->name);
>> +
>>        /* Skip "special" symbols, e.g. ARM mapping symbols.  These are
>>  	 symbols which do not correspond to objects in the symbol table,
>>  	 but have some other target-specific meaning.  */
> 
> Likewise.

Woops, I meant to remove those changes, looks like I amended instead by mistake.

>> diff --git a/gdb/minsyms.c b/gdb/minsyms.c
>> index 0f854422e0f..98ce969eed0 100644
>> --- a/gdb/minsyms.c
>> +++ b/gdb/minsyms.c
>> @@ -1112,6 +1143,11 @@ minimal_symbol_reader::record_full (const char *name, int name_len,
>>    if (ms_type == mst_file_text && startswith (name, "__gnu_compiled"))
>>      return (NULL);
>>  
>> +  if (debug_minsyms)
>> +    printf_unfiltered
>> +      ("minsym: recording minsym type: %-21s  addr: 0x%016llx  section: %-5d  name: %s\n",
>> +       mst_str (ms_type), (long long) address, section, name);
> 
> Maybe plongest() instead of %llx?  Or does plongest not do the leading 0 fill
> you want?
I don't really mind about the leading 0s, it's just that having fixed width, right aligned
and padded with spaces would give something like "0x       1234".  But I ended up using
hex_string.

Here's v2:


From 388db2a2b0299ac769943f7840c3cd4e01a216ac Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Thu, 20 Dec 2018 11:43:52 -0500
Subject: [PATCH] Add debug output for recorded minsyms

While discussing this issue:

  https://sourceware.org/ml/gdb-patches/2018-12/threads.html#00082

I added a printf to be able to quickly see all minimal symbols recorded
by GDB.  I thought it would be useful to have it built-in, for the
future.

  Recording minsym:  mst_data                         0x400780    15  _IO_stdin_used
  Recording minsym:  mst_text                         0x400700    13  __libc_csu_init
  Recording minsym:  mst_bss                          0x601058    25  _end

New since v1:

- Remove unrelated changes
- Don't print field names
- Use hex_string to print addresses
- Don't add a new "set debug" knob, use "set debug symtab-create" instead.
  Output this when the value is >= 2, since there can be quite a lot of
  symbols.

gdb/ChangeLog:

	* minsyms.c (mst_str): New.
	(minimal_symbol_reader::record_full): Add debug output.
---
 gdb/minsyms.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

-- 
2.20.1

Comments

John Baldwin Dec. 22, 2018, 12:17 a.m. UTC | #1 
On 12/21/18 3:07 PM, Simon Marchi wrote:
> Here's v2:
> 
> 
> From 388db2a2b0299ac769943f7840c3cd4e01a216ac Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@ericsson.com>
> Date: Thu, 20 Dec 2018 11:43:52 -0500
> Subject: [PATCH] Add debug output for recorded minsyms
> 
> While discussing this issue:
> 
>   https://sourceware.org/ml/gdb-patches/2018-12/threads.html#00082
> 
> I added a printf to be able to quickly see all minimal symbols recorded
> by GDB.  I thought it would be useful to have it built-in, for the
> future.
> 
>   Recording minsym:  mst_data                         0x400780    15  _IO_stdin_used
>   Recording minsym:  mst_text                         0x400700    13  __libc_csu_init
>   Recording minsym:  mst_bss                          0x601058    25  _end

Looks good to me.
Simon Marchi Dec. 22, 2018, 2:20 a.m. UTC | #2 
On 2018-12-21 19:17, John Baldwin wrote:
> Looks good to me.

Thanks, I pushed it.

Simon
Eli Zaretskii Dec. 22, 2018, 7:28 a.m. UTC | #3 
> Date: Fri, 21 Dec 2018 21:20:44 -0500
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Cc: Simon Marchi <simon.marchi@ericsson.com>, gdb-patches@sourceware.org
> 
> On 2018-12-21 19:17, John Baldwin wrote:
> > Looks good to me.
> 
> Thanks, I pushed it.

Thanks, but please also update the manual and NEWS.
Simon Marchi Dec. 22, 2018, 3:17 p.m. UTC | #4 
On 2018-12-22 02:28, Eli Zaretskii wrote:
>> Date: Fri, 21 Dec 2018 21:20:44 -0500
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>> Cc: Simon Marchi <simon.marchi@ericsson.com>, 
>> gdb-patches@sourceware.org
>> 
>> On 2018-12-21 19:17, John Baldwin wrote:
>> > Looks good to me.
>> 
>> Thanks, I pushed it.
> 
> Thanks, but please also update the manual and NEWS.

It was indeed missing from the original patch, but I ended up not adding 
a new command, so I don't think this is needed.
Tom Tromey Dec. 24, 2018, 8:51 p.m. UTC | #5 
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> It was indeed missing from the original patch, but I ended up not
Simon> adding a new command, so I don't think this is needed.

The manual documents the values for "set debug symtab-create".  I think
a patch is indeed needed since you added a new value.

Also I was wondering why not just use "maint print msymbols".

thanks,
Tom
Simon Marchi Dec. 26, 2018, 4:43 p.m. UTC | #6 
On 2018-12-24 15:51, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> It was indeed missing from the original patch, but I ended up 
> not
> Simon> adding a new command, so I don't think this is needed.
> 
> The manual documents the values for "set debug symtab-create".  I think
> a patch is indeed needed since you added a new value.

Actually that value is already used and documented:

Turns on or off display of debugging messages related to symbol table 
creation. The default is 0 (off). A value of 1 provides basic 
information. A value greater than 1 provides more verbose information.

> Also I was wondering why not just use "maint print msymbols".

Because I didn't know about it :).  Do you think the new debugging 
output is redundant and should be removed?  It's the same data (minimal 
symbols), but in a different order.  With "set debug symtab-create", 
they are shown in the order they are provided by the symbol reader (same 
order as in the file usually), whereas "maint print msymbols" shows them 
as saved in GDB's memory, sorted by address.  I think that the "set 
debug symtab-create" may be useful when debugging the symbol reader, 
whereas "maint print msymbols" may be useful when debugging lookups.

Simon
Eli Zaretskii Dec. 26, 2018, 5:19 p.m. UTC | #7 
> Date: Wed, 26 Dec 2018 11:43:18 -0500
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>, jhb@freebsd.org, gdb-patches@sourceware.org
> 
> > The manual documents the values for "set debug symtab-create".  I think
> > a patch is indeed needed since you added a new value.
> 
> Actually that value is already used and documented:
> 
> Turns on or off display of debugging messages related to symbol table 
> creation. The default is 0 (off). A value of 1 provides basic 
> information. A value greater than 1 provides more verbose information.
> 
> > Also I was wondering why not just use "maint print msymbols".
> 
> Because I didn't know about it :).  Do you think the new debugging 
> output is redundant and should be removed?  It's the same data (minimal 
> symbols), but in a different order.  With "set debug symtab-create", 
> they are shown in the order they are provided by the symbol reader (same 
> order as in the file usually), whereas "maint print msymbols" shows them 
> as saved in GDB's memory, sorted by address.  I think that the "set 
> debug symtab-create" may be useful when debugging the symbol reader, 
> whereas "maint print msymbols" may be useful when debugging lookups.

Btw, when I "set debug symtab-create 1" (or even a very large value),
I see no symbols, just the files from which those symbols are loaded.
Where's the code that shows the symbols?  Could it be that this
happens only on ELF platforms?
Simon Marchi Dec. 26, 2018, 5:24 p.m. UTC | #8 
On 2018-12-26 12:19 p.m., Eli Zaretskii wrote:
> Btw, when I "set debug symtab-create 1" (or even a very large value),
> I see no symbols, just the files from which those symbols are loaded.
> Where's the code that shows the symbols?  Could it be that this
> happens only on ELF platforms?

I use "set debug symtab-create 2" and see lines like:

  Recording minsym:  mst_text                         0x403578     0  EnterCriticalSection

The code that prints this is in minsyms.c line 1143, function
minimal_symbol_reader::record_full [1].  It should be common to all binary formats.

Maybe you are based on an older commit?  This was added last week.

Simon

[1] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/minsyms.c;h=e0f3122a974f5547f732ffd952ee0b56ae8189fb;hb=HEAD#l1142
Eli Zaretskii Dec. 26, 2018, 6:48 p.m. UTC | #9 
> Cc: tom@tromey.com, jhb@freebsd.org, gdb-patches@sourceware.org
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Date: Wed, 26 Dec 2018 12:24:11 -0500
> 
> Maybe you are based on an older commit?  This was added last week.

That explains everything.
Tom Tromey Dec. 29, 2018, 6:55 p.m. UTC | #10 
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

>> Also I was wondering why not just use "maint print msymbols".

Simon> Because I didn't know about it :).  Do you think the new debugging
Simon> output is redundant and should be removed?

It doesn't matter much to me either way.  I tend not to use these,
though I'm not entirely sure why.

Tom
diff mbox

Patch

diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 0f854422e0f..71d630374ac 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -1080,6 +1080,33 @@  minimal_symbol_reader::record (const char *name, CORE_ADDR address,
   record_with_info (name, address, ms_type, section);
 }

+/* Convert an enumerator of type minimal_symbol_type to its string
+   representation.  */
+
+static const char *
+mst_str (minimal_symbol_type t)
+{
+#define MST_TO_STR(x) case x: return #x;
+  switch (t)
+  {
+    MST_TO_STR (mst_unknown);
+    MST_TO_STR (mst_text);
+    MST_TO_STR (mst_text_gnu_ifunc);
+    MST_TO_STR (mst_slot_got_plt);
+    MST_TO_STR (mst_data);
+    MST_TO_STR (mst_bss);
+    MST_TO_STR (mst_abs);
+    MST_TO_STR (mst_solib_trampoline);
+    MST_TO_STR (mst_file_text);
+    MST_TO_STR (mst_file_data);
+    MST_TO_STR (mst_file_bss);
+
+    default:
+      return "mst_???";
+  }
+#undef MST_TO_STR
+}
+
 /* See minsyms.h.  */

 struct minimal_symbol *
@@ -1112,6 +1139,10 @@  minimal_symbol_reader::record_full (const char *name, int name_len,
   if (ms_type == mst_file_text && startswith (name, "__gnu_compiled"))
     return (NULL);

+  if (symtab_create_debug >= 2)
+    printf_unfiltered ("Recording minsym:  %-21s  %18s  %4d  %s\n",
+		       mst_str (ms_type), hex_string (address), section, name);
+
   if (m_msym_bunch_index == BUNCH_SIZE)
     {
       newobj = XCNEW (struct msym_bunch);