Re: [PATCH] sim: unify symbol table handling

[Nick Clifton <nickc@redhat.com>]

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