This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] sim: unify symbol table handling
- From: Nick Clifton <nickc at redhat dot com>
- To: Mike Frysinger <vapier at gentoo dot org>, gdb-patches at sourceware dot org
- Date: Mon, 15 Aug 2016 10:22:04 +0100
- Subject: Re: [PATCH] sim: unify symbol table handling
- Authentication-results: sourceware.org; auth=none
- References: <20160813195500.25598-1-vapier@gentoo.org>
Hi Mike,
> Nick: can you double check the aarch64 & msp430 changes ?
> And see if in general this makes sense to you ?
Sure.
> diff --git a/sim/aarch64/interp.c b/sim/aarch64/interp.c
> [...]
> /* Filter out (in place) symbols that are useless for disassembly.
> COUNT is the number of elements in SYMBOLS.
> Return the number of useful symbols. */
>
> -static unsigned long
> -remove_useless_symbols (asymbol **symbols, unsigned long count)
> +static long
> +remove_useless_symbols (asymbol **symbols, long count)
I understand the change to a signed long, but personally I consider
it a mistake. The number of symbols is always going to be a positive
value, and the need for an error value could easily be handled using
-1U instead of -1L. But the problem is endemic to the BFD library's
symbol handling code, so I guess that it will have to stay. *sigh*
> diff --git a/sim/aarch64/memory.h b/sim/aarch64/memory.h
> -extern void mem_add_blk (sim_cpu *, uint64_t, char *, uint64_t, bfd_boolean);
(I have no problem with this part of the patch, but just to note
that it is nothing to do with unifying symbol table handling...)
> +int
> +trace_load_symbols (SIM_DESC sd)
> +{
Wouldn't it make sense for trace_load_symbols to also remove useless
symbols and then sort the table ? Isn't this something that all
sims will want ? [The code for remove_useless_symbols in aarch64/interp.c
is basically generic, not aarch64 specific].
> +bfd_vma
> +trace_sym_value (SIM_DESC sd, const char *name)
> +{
> [...]
> + for (i = 0; i < STATE_PROG_SYMS_COUNT (sd); ++i)
> + if (strcmp (asymbols[i]->name, name) == 0)
> + return bfd_asymbol_value (asymbols[i]);
If there was a flag to say that the symbol table was sorted, then
this lookup could be done using bsearch().
So basically as far as I can see there is nothing wrong with the patch.
Certainly not from an AArch64 of MSP430 point of view.
Cheers
Nick