Re: [PATCH] Add "set debug minsyms" command

John Baldwin
2018-12-21T21:59:55Z

On 12/21/18 1:47 PM, Simon Marchi wrote:
> 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.
>
> The output isn't particularly pretty. I found it more readable when making
> sure the fields were vertically aligned, which results in a lot of space wasted
> in the "type" column (the width is based on the length of the longest
> enumerator):
>
> minsym: recording minsym type: mst_data addr: 0x00000000004047c0 section: 2 name: __rt_psrelocs_end
> minsym: recording minsym type: mst_text addr: 0x0000000000402b88 section: 0 name: exit
>
> But since this is just debugging output, I think it doesn't really
> matter.
>
> Also, I didn't use paddress to print the address, because:
>
> 1. There is no gdbarch handy at this point
> 2. The address may not actually be an address, but any numerical value.
> Printing with paddress could change how it's displayed (e.g. mask
> certain bits) and could be misleading. I think it's better to print
> the actual raw value saved in the minimal symbol.

2) seems compelling to me.

> gdb/ChangeLog:
>
> * minsyms.c: Include cli/cli-cmds.h.
> (debug_minsyms): New.
> (mst_str): New.
> (minimal_symbol_reader::record_full): Add debug output.
> (_initialize_minsyms): New.
> ---
> gdb/auto-load.c | 2 ++
> gdb/elfread.c | 2 ++
> gdb/minsyms.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 52 insertions(+)
>
> 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.

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

--
John Baldwin

Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â