diff mbox

sim: unify symbol table handling

Message ID 20160813195500.25598-1-vapier@gentoo.org
State New
Headers show

Commit Message

Mike Frysinger Aug. 13, 2016, 7:55 p.m. UTC
Nick: can you double check the aarch64 & msp430 changes ?
And see if in general this makes sense to you ?
---
 sim/aarch64/interp.c    | 38 +++++++--------------
 sim/aarch64/memory.c    |  7 ++--
 sim/aarch64/memory.h    |  3 --
 sim/aarch64/simulator.c |  7 ++--
 sim/aarch64/simulator.h |  3 +-
 sim/common/sim-base.h   |  4 +++
 sim/common/sim-trace.c  | 89 ++++++++++++++++++++++++++++++++++---------------
 sim/common/sim-trace.h  |  4 +++
 sim/lm32/sim-if.c       | 24 ++++---------
 sim/m68hc11/interp.c    | 49 +++++++--------------------
 sim/msp430/msp430-sim.c | 46 ++-----------------------
 sim/msp430/sim-main.h   |  8 -----
 12 files changed, 112 insertions(+), 170 deletions(-)

Comments

Nick Clifton Aug. 15, 2016, 9:22 a.m. UTC | #1
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
Mike Frysinger Aug. 15, 2016, 1:31 p.m. UTC | #2
On 15 Aug 2016 10:22, Nick Clifton wrote:
> Hi Mike,
> > 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...)

true, but it was collateral damage.  by removing the (now unused &
unneeded) bfd.h include from this header, this prototype broke (as
it uses bfd_boolean).  so i had to drop it as well.

> > +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].

i hadn't fully digested this bit of logic ... i was focusing on the bits
the core already handled, and were requiring custom clean ups/callbacks
in the non-common code.  you're right though that we could update the
core to include this code for the benefit of all.

> > +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.

thanks.  i'll merge this as-is, and add your ideas to my TODO list :).
-mike
Trevor Saunders Aug. 15, 2016, 2:29 p.m. UTC | #3
On Mon, Aug 15, 2016 at 10:22:04AM +0100, Nick Clifton wrote:
> 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*

or can we convert sim to C++ and use maybe<unsigned long> ? ;)

Trev
Mike Frysinger Aug. 15, 2016, 4:57 p.m. UTC | #4
On 15 Aug 2016 10:29, Trevor Saunders wrote:
> On Mon, Aug 15, 2016 at 10:22:04AM +0100, Nick Clifton wrote:
> > 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*
> 
> or can we convert sim to C++ and use maybe<unsigned long> ? ;)

wouldn't really help when the underlying bfd lib uses long everywhere.

that said, while conceptually it makes sense, the chances of working
with an ELF that has more than 2^31 symbols seems extremely unlikely.
-mike
diff mbox

Patch

diff --git a/sim/aarch64/interp.c b/sim/aarch64/interp.c
index 2a3ff2622a1b..2c72c136d050 100644
--- a/sim/aarch64/interp.c
+++ b/sim/aarch64/interp.c
@@ -28,6 +28,7 @@ 
 #include <stdlib.h>
 
 #include "ansidecl.h"
+#include "bfd.h"
 #include "gdb/callback.h"
 #include "gdb/remote-sim.h"
 #include "gdb/signals.h"
@@ -38,15 +39,12 @@ 
 #include "memory.h"
 #include "simulator.h"
 
-static unsigned long            symcount = 0;
-static asymbol **               symtab = NULL;
-
 /* 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)
 {
   asymbol **in_ptr  = symbols;
   asymbol **out_ptr = symbols;
@@ -87,8 +85,10 @@  compare_symbols (const void *ap, const void *bp)
 
 /* Find the name of the function at ADDR.  */
 const char *
-aarch64_get_func (uint64_t addr)
+aarch64_get_func (SIM_DESC sd, uint64_t addr)
 {
+  long symcount = STATE_PROG_SYMS_COUNT (sd);
+  asymbol **symtab = STATE_PROG_SYMS (sd);
   int  min, max;
 
   min = -1;
@@ -118,24 +118,11 @@  aarch64_get_func (uint64_t addr)
   return "";
 }
 
-uint64_t
-aarch64_get_sym_value (const char *name)
-{
-  unsigned long i;
-
-  for (i = 0; i < symcount; i++)
-    if (strcmp (bfd_asymbol_name (symtab[i]), name) == 0)
-      return bfd_asymbol_value (symtab[i]);
-
-  return 0;
-}
-
 SIM_RC
 sim_create_inferior (SIM_DESC sd, struct bfd *abfd,
 		     char * const *argv, char * const *env)
 {
   sim_cpu *cpu = STATE_CPU (sd, 0);
-  long storage = 0;
   bfd_vma addr = 0;
 
   if (abfd != NULL)
@@ -154,14 +141,13 @@  sim_create_inferior (SIM_DESC sd, struct bfd *abfd,
       STATE_PROG_ARGV (sd) = dupargv (argv);
     }
 
-  if (abfd != NULL)
-    storage = bfd_get_symtab_upper_bound (abfd);
-  if (storage > 0)
+  if (trace_load_symbols (sd))
     {
-      symtab = (asymbol **) xmalloc (storage);
-      symcount = bfd_canonicalize_symtab (abfd, symtab);
-      symcount = remove_useless_symbols (symtab, symcount);
-      qsort (symtab, symcount, sizeof (asymbol *), compare_symbols);
+      STATE_PROG_SYMS_COUNT (sd) =
+	remove_useless_symbols (STATE_PROG_SYMS (sd),
+				STATE_PROG_SYMS_COUNT (sd));
+      qsort (STATE_PROG_SYMS (sd), STATE_PROG_SYMS_COUNT (sd),
+	     sizeof (asymbol *), compare_symbols);
     }
 
   aarch64_init (cpu, addr);
diff --git a/sim/aarch64/memory.c b/sim/aarch64/memory.c
index 94c549fa0ad3..744a76dd7b69 100644
--- a/sim/aarch64/memory.c
+++ b/sim/aarch64/memory.c
@@ -25,10 +25,7 @@ 
 #include <stdlib.h>
 #include <string.h>
 
-#include "bfd.h"
 #include "libiberty.h"
-#include "elf/internal.h"
-#include "elf/common.h"
 
 #include "memory.h"
 #include "simulator.h"
@@ -163,10 +160,10 @@  aarch64_get_mem_ptr (sim_cpu *cpu, uint64_t address)
 uint64_t
 aarch64_get_heap_start (sim_cpu *cpu)
 {
-  uint64_t heap = aarch64_get_sym_value ("end");
+  uint64_t heap = trace_sym_value (CPU_STATE (cpu), "end");
 
   if (heap == 0)
-    heap = aarch64_get_sym_value ("_end");
+    heap = trace_sym_value (CPU_STATE (cpu), "_end");
   if (heap == 0)
     {
       heap = STACK_TOP - 0x100000;
diff --git a/sim/aarch64/memory.h b/sim/aarch64/memory.h
index 3f639734082c..5a0a4ad3d882 100644
--- a/sim/aarch64/memory.h
+++ b/sim/aarch64/memory.h
@@ -23,7 +23,6 @@ 
 #define _MEMORY_H
 
 #include <sys/types.h>
-#include "bfd.h"
 #include "simulator.h"
 
 extern void         aarch64_get_mem_long_double (sim_cpu *, uint64_t, FRegister *);
@@ -53,6 +52,4 @@  extern void         aarch64_set_mem_s8  (sim_cpu *, uint64_t, int8_t);
 extern uint64_t     aarch64_get_heap_start (sim_cpu *);
 extern uint64_t     aarch64_get_stack_start (sim_cpu *);
 
-extern void         mem_add_blk (sim_cpu *, uint64_t, char *, uint64_t, bfd_boolean);
-
 #endif /* _MEMORY_H */
diff --git a/sim/aarch64/simulator.c b/sim/aarch64/simulator.c
index 67e61e71706b..aad2ed36fd0d 100644
--- a/sim/aarch64/simulator.c
+++ b/sim/aarch64/simulator.c
@@ -28,6 +28,7 @@ 
 #include <time.h>
 #include <limits.h>
 
+#include "bfd.h"
 #include "simulator.h"
 #include "cpustate.h"
 #include "memory.h"
@@ -13163,7 +13164,8 @@  bl (sim_cpu *cpu, int32_t offset)
 		    " %*scall %" PRIx64 " [%s]"
 		    " [args: %" PRIx64 " %" PRIx64 " %" PRIx64 "]",
 		    stack_depth, " ", aarch64_get_next_PC (cpu),
-		    aarch64_get_func (aarch64_get_next_PC (cpu)),
+		    aarch64_get_func (CPU_STATE (cpu),
+				      aarch64_get_next_PC (cpu)),
 		    aarch64_get_reg_u64 (cpu, 0, NO_SP),
 		    aarch64_get_reg_u64 (cpu, 1, NO_SP),
 		    aarch64_get_reg_u64 (cpu, 2, NO_SP)
@@ -13202,7 +13204,8 @@  blr (sim_cpu *cpu)
 		    " %*scall %" PRIx64 " [%s]"
 		    " [args: %" PRIx64 " %" PRIx64 " %" PRIx64 "]",
 		    stack_depth, " ", aarch64_get_next_PC (cpu),
-		    aarch64_get_func (aarch64_get_next_PC (cpu)),
+		    aarch64_get_func (CPU_STATE (cpu),
+				      aarch64_get_next_PC (cpu)),
 		    aarch64_get_reg_u64 (cpu, 0, NO_SP),
 		    aarch64_get_reg_u64 (cpu, 1, NO_SP),
 		    aarch64_get_reg_u64 (cpu, 2, NO_SP)
diff --git a/sim/aarch64/simulator.h b/sim/aarch64/simulator.h
index 128d242f22b2..fbee1ce41d2e 100644
--- a/sim/aarch64/simulator.h
+++ b/sim/aarch64/simulator.h
@@ -46,8 +46,7 @@  extern void         aarch64_init (sim_cpu *, uint64_t);
    hit an error.  */
 
 extern void         aarch64_run (SIM_DESC);
-extern const char * aarch64_get_func (uint64_t);
-extern uint64_t     aarch64_get_sym_value (const char *);
+extern const char * aarch64_get_func (SIM_DESC, uint64_t);
 extern void         aarch64_init_LIT_table (void);
 
 #endif /* _SIMULATOR_H */
diff --git a/sim/common/sim-base.h b/sim/common/sim-base.h
index 350b352cc4c6..76167ebb8726 100644
--- a/sim/common/sim-base.h
+++ b/sim/common/sim-base.h
@@ -160,6 +160,10 @@  typedef struct {
   struct bfd_symbol **prog_syms;
 #define STATE_PROG_SYMS(sd) ((sd)->base.prog_syms)
 
+  /* Number of prog_syms symbols.  */
+  long prog_syms_count;
+#define STATE_PROG_SYMS_COUNT(sd) ((sd)->base.prog_syms_count)
+
   /* The program's text section.  */
   struct bfd_section *text_section;
   /* Starting and ending text section addresses from the bfd.  */
diff --git a/sim/common/sim-trace.c b/sim/common/sim-trace.c
index e299cf857d0a..5e84e13deca0 100644
--- a/sim/common/sim-trace.c
+++ b/sim/common/sim-trace.c
@@ -510,6 +510,9 @@  trace_uninstall (SIM_DESC sd)
 	    fclose (cfile);
 	}
     }
+
+  if (STATE_PROG_SYMS (sd))
+    free (STATE_PROG_SYMS (sd));
 }
 
 /* compute the nr of trace data units consumed by data */
@@ -685,6 +688,57 @@  trace_results (SIM_DESC sd,
   trace_printf (sd, cpu, "\n");
 }
 
+int
+trace_load_symbols (SIM_DESC sd)
+{
+  bfd *abfd;
+  asymbol **asymbols;
+  long symsize;
+  long symbol_count;
+
+  /* Already loaded, so nothing to do.  */
+  if (STATE_PROG_SYMS (sd))
+    return 1;
+
+  abfd = STATE_PROG_BFD (sd);
+  if (abfd == NULL)
+    return 0;
+
+  symsize = bfd_get_symtab_upper_bound (abfd);
+  if (symsize < 0)
+    return 0;
+
+  asymbols = xmalloc (symsize);
+  symbol_count = bfd_canonicalize_symtab (abfd, asymbols);
+  if (symbol_count < 0)
+    {
+      free (asymbols);
+      return 0;
+    }
+
+  STATE_PROG_SYMS (sd) = asymbols;
+  STATE_PROG_SYMS_COUNT (sd) = symbol_count;
+  return 1;
+}
+
+bfd_vma
+trace_sym_value (SIM_DESC sd, const char *name)
+{
+  asymbol **asymbols;
+  long i;
+
+  if (!trace_load_symbols (sd))
+    return -1;
+
+  asymbols = STATE_PROG_SYMS (sd);
+
+  for (i = 0; i < STATE_PROG_SYMS_COUNT (sd); ++i)
+    if (strcmp (asymbols[i]->name, name) == 0)
+      return bfd_asymbol_value (asymbols[i]);
+
+  return -1;
+}
+
 void
 trace_prefix (SIM_DESC sd,
 	      sim_cpu *cpu,
@@ -744,9 +798,9 @@  trace_prefix (SIM_DESC sd,
     {
       char buf[256];
       buf[0] = 0;
-      if (STATE_TEXT_SECTION (CPU_STATE (cpu))
-	  && pc >= STATE_TEXT_START (CPU_STATE (cpu))
-	  && pc < STATE_TEXT_END (CPU_STATE (cpu)))
+      if (STATE_TEXT_SECTION (sd)
+	  && pc >= STATE_TEXT_START (sd)
+	  && pc < STATE_TEXT_END (sd))
 	{
 	  const char *pc_filename = (const char *)0;
 	  const char *pc_function = (const char *)0;
@@ -754,31 +808,14 @@  trace_prefix (SIM_DESC sd,
 	  bfd *abfd;
 	  asymbol **asymbols;
 
-	  abfd = STATE_PROG_BFD (CPU_STATE (cpu));
-	  asymbols = STATE_PROG_SYMS (CPU_STATE (cpu));
-	  if (asymbols == NULL)
-	    {
-	      long symsize;
-	      long symbol_count;
+	  if (!trace_load_symbols (sd))
+	    sim_engine_abort (sd, cpu, cia, "could not load symbols");
 
-	      symsize = bfd_get_symtab_upper_bound (abfd);
-	      if (symsize < 0)
-		{
-		  sim_engine_abort (sd, cpu, cia, "could not read symbols");
-		}
-	      asymbols = (asymbol **) xmalloc (symsize);
-	      symbol_count = bfd_canonicalize_symtab (abfd, asymbols);
-	      if (symbol_count < 0)
-		{
-		  sim_engine_abort (sd, cpu, cia, "could not canonicalize symbols");
-		}
-	      STATE_PROG_SYMS (CPU_STATE (cpu)) = asymbols;
-	    }
+	  abfd = STATE_PROG_BFD (sd);
+	  asymbols = STATE_PROG_SYMS (sd);
 
-	  if (bfd_find_nearest_line (abfd,
-				     STATE_TEXT_SECTION (CPU_STATE (cpu)),
-				     asymbols,
-				     pc - STATE_TEXT_START (CPU_STATE (cpu)),
+	  if (bfd_find_nearest_line (abfd, STATE_TEXT_SECTION (sd), asymbols,
+				     pc - STATE_TEXT_START (sd),
 				     &pc_filename, &pc_function, &pc_linenum))
 	    {
 	      char *p = buf;
diff --git a/sim/common/sim-trace.h b/sim/common/sim-trace.h
index 40de5bf5a21c..d3392ae8abd8 100644
--- a/sim/common/sim-trace.h
+++ b/sim/common/sim-trace.h
@@ -671,6 +671,10 @@  extern void trace_vprintf (SIM_DESC, sim_cpu *, const char *, va_list);
 /* Non-zero if "--debug-insn" specified.  */
 #define DEBUG_INSN_P(cpu) DEBUG_P (cpu, DEBUG_INSN_IDX)
 
+/* Symbol related helpers.  */
+int trace_load_symbols (SIM_DESC);
+bfd_vma trace_sym_value (SIM_DESC, const char *name);
+
 extern void sim_debug_printf (sim_cpu *, const char *, ...)
      __attribute__((format (printf, 2, 3)));
 
diff --git a/sim/lm32/sim-if.c b/sim/lm32/sim-if.c
index 860c1e63f1ff..aad3a35a3076 100644
--- a/sim/lm32/sim-if.c
+++ b/sim/lm32/sim-if.c
@@ -71,27 +71,15 @@  find_base (bfd *prog_bfd)
 }
 
 static unsigned long
-find_limit (bfd *prog_bfd)
+find_limit (SIM_DESC sd)
 {
-  struct bfd_symbol **asymbols;
-  long symsize;
-  long symbol_count;
-  long s;
+  bfd_vma addr;
 
-  symsize = bfd_get_symtab_upper_bound (prog_bfd);
-  if (symsize < 0)
-    return 0;
-  asymbols = (asymbol **) xmalloc (symsize);
-  symbol_count = bfd_canonicalize_symtab (prog_bfd, asymbols);
-  if (symbol_count < 0)
+  addr = trace_sym_value (sd, "_fstack");
+  if (addr == -1)
     return 0;
 
-  for (s = 0; s < symbol_count; s++)
-    {
-      if (!strcmp (asymbols[s]->name, "_fstack"))
-	return (asymbols[s]->value + 65536) & ~(0xffffUL);
-    }
-  return 0;
+  return (addr + 65536) & ~(0xffffUL);
 }
 
 /* Create an instance of the simulator.  */
@@ -159,7 +147,7 @@  sim_open (kind, callback, abfd, argv)
 	{
 	  /* It doesn't, so we should try to allocate enough memory to hold program.  */
 	  base = find_base (STATE_PROG_BFD (sd));
-	  limit = find_limit (STATE_PROG_BFD (sd));
+	  limit = find_limit (sd);
 	  if (limit == 0)
 	    {
 	      sim_io_eprintf (sd,
diff --git a/sim/m68hc11/interp.c b/sim/m68hc11/interp.c
index ab205714306b..8a806566ba6f 100644
--- a/sim/m68hc11/interp.c
+++ b/sim/m68hc11/interp.c
@@ -292,50 +292,25 @@  sim_hw_configure (SIM_DESC sd)
 /* Get the memory bank parameters by looking at the global symbols
    defined by the linker.  */
 static int
-sim_get_bank_parameters (SIM_DESC sd, bfd* abfd)
+sim_get_bank_parameters (SIM_DESC sd)
 {
   sim_cpu *cpu;
-  long symsize;
-  long symbol_count, i;
   unsigned size;
-  asymbol** asymbols;
-  asymbol** current;
+  bfd_vma addr;
 
   cpu = STATE_CPU (sd, 0);
 
-  symsize = bfd_get_symtab_upper_bound (abfd);
-  if (symsize < 0)
-    {
-      sim_io_eprintf (sd, "Cannot read symbols of program");
-      return 0;
-    }
-  asymbols = (asymbol **) xmalloc (symsize);
-  symbol_count = bfd_canonicalize_symtab (abfd, asymbols);
-  if (symbol_count < 0)
-    {
-      sim_io_eprintf (sd, "Cannot read symbols of program");
-      return 0;
-    }
+  addr = trace_sym_value (sd, BFD_M68HC11_BANK_START_NAME);
+  if (addr != -1)
+    cpu->bank_start = addr;
 
-  size = 0;
-  for (i = 0, current = asymbols; i < symbol_count; i++, current++)
-    {
-      const char* name = bfd_asymbol_name (*current);
+  size = trace_sym_value (sd, BFD_M68HC11_BANK_SIZE_NAME);
+  if (size == -1)
+    size = 0;
 
-      if (strcmp (name, BFD_M68HC11_BANK_START_NAME) == 0)
-        {
-          cpu->bank_start = bfd_asymbol_value (*current);
-        }
-      else if (strcmp (name, BFD_M68HC11_BANK_SIZE_NAME) == 0)
-        {
-          size = bfd_asymbol_value (*current);
-        }
-      else if (strcmp (name, BFD_M68HC11_BANK_VIRTUAL_NAME) == 0)
-        {
-          cpu->bank_virtual = bfd_asymbol_value (*current);
-        }
-    }
-  free (asymbols);
+  addr = trace_sym_value (sd, BFD_M68HC11_BANK_VIRTUAL_NAME);
+  if (addr != -1)
+    cpu->bank_virtual = addr;
 
   cpu->bank_end = cpu->bank_start + size;
   cpu->bank_shift = 0;
@@ -387,7 +362,7 @@  sim_prepare_for_program (SIM_DESC sd, bfd* abfd)
 
       if (elf_flags & E_M68HC12_BANKS)
         {
-          if (sim_get_bank_parameters (sd, abfd) != 0)
+          if (sim_get_bank_parameters (sd) != 0)
             sim_io_eprintf (sd, "Memory bank parameters are not initialized\n");
         }
     }
diff --git a/sim/msp430/msp430-sim.c b/sim/msp430/msp430-sim.c
index 5a6b3edca657..4971cb88d2b1 100644
--- a/sim/msp430/msp430-sim.c
+++ b/sim/msp430/msp430-sim.c
@@ -26,7 +26,6 @@ 
 #include <inttypes.h>
 #include <unistd.h>
 #include <assert.h>
-#include "bfd.h"
 #include "opcode/msp430-decode.h"
 #include "sim-main.h"
 #include "sim-syscall.h"
@@ -44,39 +43,6 @@  msp430_pc_store (SIM_CPU *cpu, sim_cia newpc)
   cpu->state.regs[0] = newpc;
 }
 
-static long
-lookup_symbol (SIM_DESC sd, const char *name)
-{
-  struct bfd *abfd = STATE_PROG_BFD (sd);
-  asymbol **symbol_table = STATE_SYMBOL_TABLE (sd);
-  long number_of_symbols = STATE_NUM_SYMBOLS (sd);
-  long i;
-
-  if (abfd == NULL)
-    return -1;
-
-  if (symbol_table == NULL)
-    {
-      long storage_needed;
-
-      storage_needed = bfd_get_symtab_upper_bound (abfd);
-      if (storage_needed <= 0)
-	return -1;
-
-      STATE_SYMBOL_TABLE (sd) = symbol_table = xmalloc (storage_needed);
-      STATE_NUM_SYMBOLS (sd) = number_of_symbols =
-	bfd_canonicalize_symtab (abfd, symbol_table);
-    }
-
-  for (i = 0; i < number_of_symbols; i++)
-    if (strcmp (symbol_table[i]->name, name) == 0)
-      {
-	long val = symbol_table[i]->section->vma + symbol_table[i]->value;
-	return val;
-      }
-  return -1;
-}
-
 static int
 msp430_reg_fetch (SIM_CPU *cpu, int regno, unsigned char *buf, int len)
 {
@@ -207,20 +173,14 @@  sim_open (SIM_OPEN_KIND kind,
   assert (MAX_NR_PROCESSORS == 1);
   msp430_initialize_cpu (sd, MSP430_CPU (sd));
 
-  MSP430_CPU (sd)->state.cio_breakpoint = lookup_symbol (sd, "C$$IO$$");
-  MSP430_CPU (sd)->state.cio_buffer = lookup_symbol (sd, "__CIOBUF__");
+  MSP430_CPU (sd)->state.cio_breakpoint = trace_sym_value (sd, "C$$IO$$");
+  MSP430_CPU (sd)->state.cio_buffer = trace_sym_value (sd, "__CIOBUF__");
   if (MSP430_CPU (sd)->state.cio_buffer == -1)
-    MSP430_CPU (sd)->state.cio_buffer = lookup_symbol (sd, "_CIOBUF_");
+    MSP430_CPU (sd)->state.cio_buffer = trace_sym_value (sd, "_CIOBUF_");
 
   return sd;
 }
 
-void
-msp430_sim_close (SIM_DESC sd, int quitting)
-{
-  free (STATE_SYMBOL_TABLE (sd));
-}
-
 SIM_RC
 sim_create_inferior (SIM_DESC sd,
 		     struct bfd *abfd,
diff --git a/sim/msp430/sim-main.h b/sim/msp430/sim-main.h
index 4a2ab22eafeb..da48ec6cf810 100644
--- a/sim/msp430/sim-main.h
+++ b/sim/msp430/sim-main.h
@@ -37,11 +37,6 @@  struct sim_state
 {
   sim_cpu *cpu[MAX_NR_PROCESSORS];
 
-  asymbol **symbol_table;
-  long number_of_symbols;
-#define STATE_SYMBOL_TABLE(sd)   ((sd)->symbol_table)
-#define STATE_NUM_SYMBOLS(sd) ((sd)->number_of_symbols)
-  
   /* Simulator specific members.  */
   sim_state_base base;
 };
@@ -54,7 +49,4 @@  struct sim_state
 #include "sim-engine.h"
 #include "sim-options.h"
 
-extern void msp430_sim_close (SIM_DESC sd, int quitting);
-#define SIM_CLOSE_HOOK(...) msp430_sim_close (__VA_ARGS__)
-
 #endif /* _MSP430_MAIN_SIM_H_ */