This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: combine bfd_lookup_symbol in solib-*.c


On 08/29/2011 07:41 PM, Pedro Alves wrote:

On Thursday 18 August 2011 16:36:46, Yao Qi wrote:

+
+CORE_ADDR
+bfd_lookup_symbol (bfd *abfd, const char *symname,
+                  int (*is_right_sym) (asymbol *, const char *))

All the function does with the passed in SYMNAME is passing it down to the callback. If the interface has been generalized to pass in a "match" callback, then make the symname parameter generic too, like:

   CORE_ADDR
   bfd_lookup_symbol (bfd *abfd,
                   int (*is_right_sym) (asymbol *, void *),
                   void *data);

Please document the parameters.

OK, parameters are documented.



I think it's best to reserve extern symbols that start with "bfd_" to libbfd proper. Maybe gdb_bfd_lookup_symbol would be a good enough alternative. (Or maybe propose the function for bfd proper, though I'd suggest fixing its interface to not assume 0 return is error then)


I agree. Renamed bfd_* functions to gdb_bfd_*.


+/* Lookup the value for a specific symbol.
+
+   An expensive way to lookup the value of a single symbol for
+   bfd's that are only temporary anyway.  This is used by the
+   shared library support to find the address of the debugger
+   notification routine in the shared library.
+
+   The returned symbol may be in a code or data section; functions
+   will normally be in a code section, but may be in a data section
+   if this architecture uses function descriptors.

The way you've generalized the function, since it's the callback's responsibility to pick the the symbol, this comment no longer appears to be in the right place.


OK, removed this comment here.


--
Yao (éå)
        gdb/
        * solib-dsbt.c (bfd_lookup_symbol): Removed.
        (cmp_name): New.
        (enable_break2): Update caller.
        * solib-frv.c (bfd_lookup_symbol): Removed.
        (cmp_name): New.
        (enable_break2): Update caller.
        * solib-pa64.c (bfd_lookup_symbol): Removed.
        (cmp_name): New.
        * solib-svr4.c (bfd_lookup_symbol): Removed.
        (cmp_name_and_sec_flags): New.
        (enable_break): Update caller.
        * solib.c (gdb_bfd_lookup_symbol_from_symtab): New.
        (gdb_bfd_lookup_symbol_from_dyn_symtab): New.
        (gdb_bfd_lookup_symbol): New.
        * solib.h: Functions declarations.
---
 gdb/solib-dsbt.c |   80 +++++---------------------------------------
 gdb/solib-frv.c  |   82 +++++----------------------------------------
 gdb/solib-pa64.c |   48 +++-----------------------
 gdb/solib-svr4.c |   91 +++++---------------------------------------------
 gdb/solib.c      |   97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/solib.h      |   14 ++++++++
 6 files changed, 146 insertions(+), 266 deletions(-)

diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index 2569395..1db4537 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -508,76 +508,6 @@ scan_dyntag (int dyntag, bfd *abfd, CORE_ADDR *ptr)
   return 0;
 }
 
-/* An expensive way to lookup the value of a single symbol for
-   bfd's that are only temporary anyway.  This is used by the
-   shared library support to find the address of the debugger
-   interface structures in the shared library.
-
-   Note that 0 is specifically allowed as an error return (no
-   such symbol).  */
-
-static CORE_ADDR
-bfd_lookup_symbol (bfd *abfd, char *symname)
-{
-  long storage_needed;
-  asymbol *sym;
-  asymbol **symbol_table;
-  unsigned int number_of_symbols;
-  unsigned int i;
-  struct cleanup *back_to;
-  CORE_ADDR symaddr = 0;
-
-  storage_needed = bfd_get_symtab_upper_bound (abfd);
-
-  if (storage_needed > 0)
-    {
-      symbol_table = (asymbol **) xmalloc (storage_needed);
-      back_to = make_cleanup (xfree, symbol_table);
-      number_of_symbols = bfd_canonicalize_symtab (abfd, symbol_table);
-
-      for (i = 0; i < number_of_symbols; i++)
-	{
-	  sym = *symbol_table++;
-	  if (strcmp (sym->name, symname) == 0)
-	    {
-	      /* Bfd symbols are section relative. */
-	      symaddr = sym->value + sym->section->vma;
-	      break;
-	    }
-	}
-      do_cleanups (back_to);
-    }
-
-  if (symaddr)
-    return symaddr;
-
-  /* Look for the symbol in the dynamic string table too.  */
-
-  storage_needed = bfd_get_dynamic_symtab_upper_bound (abfd);
-
-  if (storage_needed > 0)
-    {
-      symbol_table = (asymbol **) xmalloc (storage_needed);
-      back_to = make_cleanup (xfree, symbol_table);
-      number_of_symbols = bfd_canonicalize_dynamic_symtab (abfd, symbol_table);
-
-      for (i = 0; i < number_of_symbols; i++)
-	{
-	  sym = *symbol_table++;
-	  if (strcmp (sym->name, symname) == 0)
-	    {
-	      /* Bfd symbols are section relative. */
-	      symaddr = sym->value + sym->section->vma;
-	      break;
-	    }
-	}
-      do_cleanups (back_to);
-    }
-
-  return symaddr;
-}
-
-
 /* If no open symbol file, attempt to locate and open the main symbol
    file.
 
@@ -852,6 +782,14 @@ enable_break_failure_warning (void)
 	   "and track explicitly loaded dynamic code."));
 }
 
+/* Helper function for gdb_bfd_lookup_symbol.  */
+
+static int
+cmp_name (asymbol *sym, const void *data)
+{
+  return (strcmp (sym->name, (const char *) data) == 0);
+}
+
 /* The dynamic linkers has, as part of its debugger interface, support
    for arranging for the inferior to hit a breakpoint after mapping in
    the shared libraries.  This function enables that breakpoint.
@@ -957,7 +895,7 @@ enable_break2 (void)
 	    info->interp_plt_sect_low + bfd_section_size (tmp_bfd, interp_sect);
 	}
 
-      addr = bfd_lookup_symbol (tmp_bfd, "_dl_debug_addr");
+      addr = gdb_bfd_lookup_symbol (tmp_bfd, cmp_name, "_dl_debug_addr");
       if (addr == 0)
 	{
 	  warning (_("Could not find symbol _dl_debug_addr in dynamic linker"));
diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index e1c16d9..c70dffb 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -239,77 +239,6 @@ static void frv_relocate_main_executable (void);
 static CORE_ADDR main_got (void);
 static int enable_break2 (void);
 
-/* Lookup the value for a specific symbol.
-
-   An expensive way to lookup the value of a single symbol for
-   bfd's that are only temporary anyway.  This is used by the
-   shared library support to find the address of the debugger
-   interface structures in the shared library.
-
-   Note that 0 is specifically allowed as an error return (no
-   such symbol).  */
-
-static CORE_ADDR
-bfd_lookup_symbol (bfd *abfd, char *symname)
-{
-  long storage_needed;
-  asymbol *sym;
-  asymbol **symbol_table;
-  unsigned int number_of_symbols;
-  unsigned int i;
-  struct cleanup *back_to;
-  CORE_ADDR symaddr = 0;
-
-  storage_needed = bfd_get_symtab_upper_bound (abfd);
-
-  if (storage_needed > 0)
-    {
-      symbol_table = (asymbol **) xmalloc (storage_needed);
-      back_to = make_cleanup (xfree, symbol_table);
-      number_of_symbols = bfd_canonicalize_symtab (abfd, symbol_table);
-
-      for (i = 0; i < number_of_symbols; i++)
-	{
-	  sym = *symbol_table++;
-	  if (strcmp (sym->name, symname) == 0)
-	    {
-	      /* Bfd symbols are section relative.  */
-	      symaddr = sym->value + sym->section->vma;
-	      break;
-	    }
-	}
-      do_cleanups (back_to);
-    }
-
-  if (symaddr)
-    return symaddr;
-
-  /* Look for the symbol in the dynamic string table too.  */
-
-  storage_needed = bfd_get_dynamic_symtab_upper_bound (abfd);
-
-  if (storage_needed > 0)
-    {
-      symbol_table = (asymbol **) xmalloc (storage_needed);
-      back_to = make_cleanup (xfree, symbol_table);
-      number_of_symbols = bfd_canonicalize_dynamic_symtab (abfd, symbol_table);
-
-      for (i = 0; i < number_of_symbols; i++)
-	{
-	  sym = *symbol_table++;
-	  if (strcmp (sym->name, symname) == 0)
-	    {
-	      /* Bfd symbols are section relative.  */
-	      symaddr = sym->value + sym->section->vma;
-	      break;
-	    }
-	}
-      do_cleanups (back_to);
-    }
-
-  return symaddr;
-}
-
 /* Implement the "open_symbol_file_object" target_so_ops method.  */
 
 static int
@@ -554,6 +483,14 @@ enable_break_failure_warning (void)
 	   "and track explicitly loaded dynamic code."));
 }
 
+/* Helper function for gdb_bfd_lookup_symbol.  */
+
+static int
+cmp_name (asymbol *sym, const void *data)
+{
+  return (strcmp (sym->name, (const char *) data) == 0);
+}
+
 /* Arrange for dynamic linker to hit breakpoint.
 
    The dynamic linkers has, as part of its debugger interface, support
@@ -680,7 +617,8 @@ enable_break2 (void)
 	    interp_plt_sect_low + bfd_section_size (tmp_bfd, interp_sect);
 	}
 
-      addr = bfd_lookup_symbol (tmp_bfd, "_dl_debug_addr");
+      addr = gdb_bfd_lookup_symbol (tmp_bfd, cmp_name, "_dl_debug_addr");
+
       if (addr == 0)
 	{
 	  warning (_("Could not find symbol _dl_debug_addr "
diff --git a/gdb/solib-pa64.c b/gdb/solib-pa64.c
index cd38728..dae99ba 100644
--- a/gdb/solib-pa64.c
+++ b/gdb/solib-pa64.c
@@ -269,51 +269,14 @@ read_dynamic_info (asection *dyninfo_sect, dld_cache_t *dld_cache_p)
   return 1;
 }
 
-/* bfd_lookup_symbol -- lookup the value for a specific symbol
+/* Helper function for gdb_bfd_lookup_symbol_from_symtab.  */
 
-   An expensive way to lookup the value of a single symbol for
-   bfd's that are only temporary anyway.  This is used by the
-   shared library support to find the address of the debugger
-   interface structures in the shared library.
-
-   Note that 0 is specifically allowed as an error return (no
-   such symbol).  */
-
-static CORE_ADDR
-bfd_lookup_symbol (bfd *abfd, char *symname)
+static int
+cmp_name (asymbol *sym, const void *data)
 {
-  unsigned int storage_needed;
-  asymbol *sym;
-  asymbol **symbol_table;
-  unsigned int number_of_symbols;
-  unsigned int i;
-  struct cleanup *back_to;
-  CORE_ADDR symaddr = 0;
-
-  storage_needed = bfd_get_symtab_upper_bound (abfd);
-
-  if (storage_needed > 0)
-    {
-      symbol_table = (asymbol **) xmalloc (storage_needed);
-      back_to = make_cleanup (xfree, symbol_table);
-      number_of_symbols = bfd_canonicalize_symtab (abfd, symbol_table);
-
-      for (i = 0; i < number_of_symbols; i++)
-	{
-	  sym = *symbol_table++;
-	  if (strcmp (sym->name, symname) == 0)
-	    {
-	      /* Bfd symbols are section relative.  */
-	      symaddr = sym->value + sym->section->vma;
-	      break;
-	    }
-	}
-      do_cleanups (back_to);
-    }
-  return (symaddr);
+  return (strcmp (sym->name, (const char *) data) == 0);
 }
 
-
 /* This hook gets called just before the first instruction in the
    inferior process is executed.
 
@@ -421,7 +384,8 @@ manpage for methods to privately map shared library text."));
 	 routine.  */
       load_addr = regcache_read_pc (get_current_regcache ())
 		  - tmp_bfd->start_address;
-      sym_addr = bfd_lookup_symbol (tmp_bfd, "__dld_break");
+      sym_addr = bfd_lookup_symbol_from_symtab (tmp_bfd, cmp_name,
+						"__dld_break");
       sym_addr = load_addr + sym_addr + 4;
       
       /* Create the shared library breakpoint.  */
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index f643a24..13cad46 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -370,86 +370,6 @@ get_svr4_info (void)
 
 static int match_main (const char *);
 
-/* Lookup the value for a specific symbol.
-
-   An expensive way to lookup the value of a single symbol for
-   bfd's that are only temporary anyway.  This is used by the
-   shared library support to find the address of the debugger
-   notification routine in the shared library.
-
-   The returned symbol may be in a code or data section; functions
-   will normally be in a code section, but may be in a data section
-   if this architecture uses function descriptors.
-
-   Note that 0 is specifically allowed as an error return (no
-   such symbol).  */
-
-static CORE_ADDR
-bfd_lookup_symbol (bfd *abfd, const char *symname)
-{
-  long storage_needed;
-  asymbol *sym;
-  asymbol **symbol_table;
-  unsigned int number_of_symbols;
-  unsigned int i;
-  struct cleanup *back_to;
-  CORE_ADDR symaddr = 0;
-
-  storage_needed = bfd_get_symtab_upper_bound (abfd);
-
-  if (storage_needed > 0)
-    {
-      symbol_table = (asymbol **) xmalloc (storage_needed);
-      back_to = make_cleanup (xfree, symbol_table);
-      number_of_symbols = bfd_canonicalize_symtab (abfd, symbol_table);
-
-      for (i = 0; i < number_of_symbols; i++)
-	{
-	  sym = *symbol_table++;
-	  if (strcmp (sym->name, symname) == 0
-              && (sym->section->flags & (SEC_CODE | SEC_DATA)) != 0)
-	    {
-	      /* BFD symbols are section relative.  */
-	      symaddr = sym->value + sym->section->vma;
-	      break;
-	    }
-	}
-      do_cleanups (back_to);
-    }
-
-  if (symaddr)
-    return symaddr;
-
-  /* On FreeBSD, the dynamic linker is stripped by default.  So we'll
-     have to check the dynamic string table too.  */
-
-  storage_needed = bfd_get_dynamic_symtab_upper_bound (abfd);
-
-  if (storage_needed > 0)
-    {
-      symbol_table = (asymbol **) xmalloc (storage_needed);
-      back_to = make_cleanup (xfree, symbol_table);
-      number_of_symbols = bfd_canonicalize_dynamic_symtab (abfd, symbol_table);
-
-      for (i = 0; i < number_of_symbols; i++)
-	{
-	  sym = *symbol_table++;
-
-	  if (strcmp (sym->name, symname) == 0
-              && (sym->section->flags & (SEC_CODE | SEC_DATA)) != 0)
-	    {
-	      /* BFD symbols are section relative.  */
-	      symaddr = sym->value + sym->section->vma;
-	      break;
-	    }
-	}
-      do_cleanups (back_to);
-    }
-
-  return symaddr;
-}
-
-
 /* Read program header TYPE from inferior memory.  The header is found
    by scanning the OS auxillary vector.
 
@@ -1253,6 +1173,14 @@ exec_entry_point (struct bfd *abfd, struct target_ops *targ)
 					     targ);
 }
 
+/* Helper function for gdb_bfd_lookup_symbol.  */
+
+static int
+cmp_name_and_sec_flags (asymbol *sym, const void *data)
+{
+  return (strcmp (sym->name, (const char *) data) == 0
+	  && (sym->section->flags & (SEC_CODE | SEC_DATA)) != 0);
+}
 /* Arrange for dynamic linker to hit breakpoint.
 
    Both the SunOS and the SVR4 dynamic linkers have, as part of their
@@ -1501,7 +1429,8 @@ enable_break (struct svr4_info *info, int from_tty)
       /* Now try to set a breakpoint in the dynamic linker.  */
       for (bkpt_namep = solib_break_names; *bkpt_namep != NULL; bkpt_namep++)
 	{
-	  sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep);
+	  sym_addr = gdb_bfd_lookup_symbol (tmp_bfd, cmp_name_and_sec_flags,
+					    (const void *) *bkpt_namep);
 	  if (sym_addr != 0)
 	    break;
 	}
diff --git a/gdb/solib.c b/gdb/solib.c
index f843723..2c0548e 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -1336,6 +1336,103 @@ solib_global_lookup (const struct objfile *objfile,
   return NULL;
 }
 
+/* Lookup the value for a specific symbol from dynamic symbol table.  Look
+   up symbol from ABFD.  MATCH_SYM is a callback function to determine
+   whether to pick up a symbol.  DATA is the input of this callback
+   function.  Return NULL if symbol is not found.  */
+
+CORE_ADDR
+gdb_bfd_lookup_symbol_from_symtab (bfd *abfd,
+				   int (*match_sym) (asymbol *, const void *),
+				   const void *data)
+{
+  long storage_needed = bfd_get_symtab_upper_bound (abfd);
+  CORE_ADDR symaddr = 0;
+
+  if (storage_needed > 0)
+    {
+      unsigned int i;
+
+      asymbol **symbol_table = (asymbol **) xmalloc (storage_needed);
+      struct cleanup *back_to = make_cleanup (xfree, symbol_table);
+      unsigned int number_of_symbols =
+	bfd_canonicalize_symtab (abfd, symbol_table);
+
+      for (i = 0; i < number_of_symbols; i++)
+	{
+	  asymbol *sym  = *symbol_table++;
+
+	  if (match_sym (sym, data))
+	    {
+	      /* BFD symbols are section relative.  */
+	      symaddr = sym->value + sym->section->vma;
+	      break;
+	    }
+	}
+      do_cleanups (back_to);
+    }
+
+  return symaddr;
+}
+
+/* Lookup the value for a specific symbol from symbol table.  Look up symbol
+   from ABFD.  MATCH_SYM is a callback function to determine whether to pick
+   up a symbol.  DATA is the input of this callback function.  Return NULL
+   if symbol is not found.  */
+
+static CORE_ADDR
+bfd_lookup_symbol_from_dyn_symtab (bfd *abfd,
+				   int (*match_sym) (asymbol *,
+							const void *),
+				   const void *data)
+{
+  long storage_needed = bfd_get_dynamic_symtab_upper_bound (abfd);
+  CORE_ADDR symaddr = 0;
+
+  if (storage_needed > 0)
+    {
+      unsigned int i;
+      asymbol **symbol_table = (asymbol **) xmalloc (storage_needed);
+      struct cleanup *back_to = make_cleanup (xfree, symbol_table);
+      unsigned int number_of_symbols =
+	bfd_canonicalize_dynamic_symtab (abfd, symbol_table);
+
+      for (i = 0; i < number_of_symbols; i++)
+	{
+	  asymbol *sym = *symbol_table++;
+
+	  if (match_sym (sym, data))
+	    {
+	      /* BFD symbols are section relative.  */
+	      symaddr = sym->value + sym->section->vma;
+	      break;
+	    }
+	}
+      do_cleanups (back_to);
+    }
+  return symaddr;
+}
+
+/* Lookup the value for a specific symbol from symbol table and dynamic
+   symbol table.  Look up symbol from ABFD.  MATCH_SYM is a callback
+   function to determine whether to pick up a symbol.  DATA is the
+   input of this callback function.  Return NULL if symbol is not
+   found.  */
+
+CORE_ADDR
+gdb_bfd_lookup_symbol (bfd *abfd,
+		       int (*match_sym) (asymbol *, const void *),
+		       const void *data)
+{
+  CORE_ADDR symaddr = gdb_bfd_lookup_symbol_from_symtab (abfd, match_sym, data);
+
+  /* On FreeBSD, the dynamic linker is stripped by default.  So we'll
+     have to check the dynamic string table too.  */
+  if (symaddr == 0)
+    symaddr = bfd_lookup_symbol_from_dyn_symtab (abfd, match_sym, data);
+
+  return symaddr;
+}
 
 extern initialize_file_ftype _initialize_solib; /* -Wmissing-prototypes */
 
diff --git a/gdb/solib.h b/gdb/solib.h
index c473d85..54640b2 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -78,4 +78,18 @@ extern void set_solib_ops (struct gdbarch *gdbarch,
 
 extern int libpthread_name_p (const char *name);
 
+/* Look up symbol from both symbol table and dynamic string table.  */
+
+extern CORE_ADDR gdb_bfd_lookup_symbol (bfd *abfd,
+					int (*match_sym) (asymbol *,
+							  const void *),
+					const void *data);
+
+/* Look up symbol from symbol table.  */
+
+extern CORE_ADDR bfd_lookup_symbol_from_symtab (bfd *abfd,
+						int (*match_sym) (asymbol *,
+								  const void *),
+						const void *data);
+
 #endif /* SOLIB_H */
-- 
1.7.0.4


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]