Re: [PATCH 4/9 v7] Introduce target/symbol.h

[Gary Benson <gbenson at redhat dot com>]

Pedro Alves wrote:
> On 08/29/2014 02:51 PM, Gary Benson wrote:
> > This introduces target/symbol.h.  This file declares a function
> > that the shared code can use and that the clients must implement.
> > It also changes some shared code to use these functions.
> > 
> > This patch differs from the version I posted on August 1
> > (https://sourceware.org/ml/gdb-patches/2014-08/msg00013.html)
> > in that the new function has been renamed as
> > find_minimal_symbol_address and that it lives in minsyms.c for GDB
> > and a new file symbol.c for gdbserver rather than in target.c for
> > both.
> > 
> > gdb/ChangeLog:
> > 
> > 	* target/symbol.h: New file.
> > 	* Makefile.in (HFILES_NO_SRCDIR): Add target/symbol.h.
> > 	* target.h: Include target/symbol.h.
> > 	* minsyms.c (find_minimal_symbol_address): New function.
> > 	* common/agent.c: Include target/symbol.h.
> > 	[!GDBSERVER]: Don't include objfiles.h.
> > 	(agent_look_up_symbols): Use find_minimal_symbol_address.
> 
> Thanks.  Though for the same reason in the old version calling this
> new method target_foo looked wrong, because this is not interacting
> with the target vector abstraction (it looks like something for a
> client vector instead), I don't think this belongs in target/ at
> all.  I think it should go in common/ instead for now.

Ok, I'll put it in common.

> Related, this:
> 
> > 	* target.h: Include target/symbol.h.
> 
> looks wrong to me too, and it isn't clear to me why you needed it.

Probably a hangover from some old version or another.  Removed.

Going to push the updated commit inlined below.

Cheers,
Gary

-- 
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3dadc78..bcc6642 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2014-09-11  Tom Tromey  <tromey@redhat.com>
+	    Gary Benson  <gbenson@redhat.com>
+
+	* common/symbol.h: New file.
+	* Makefile.in (HFILES_NO_SRCDIR): Add common/symbol.h.
+	* minsyms.c (find_minimal_symbol_address): New function.
+	* common/agent.c: Include common/symbol.h.
+	[!GDBSERVER]: Don't include objfiles.h.
+	(agent_look_up_symbols): Use find_minimal_symbol_address.
+
 2014-09-11  Gary Benson  <gbenson@redhat.com>
 
 	* target/target.h (target_stop_ptid, target_continue_ptid):
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 55b9829..f6b9176 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -938,7 +938,7 @@ target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h \
 common/print-utils.h common/rsp-low.h nat/x86-dregs.h x86-linux-nat.h \
 i386-linux-nat.h common/common-defs.h common/errors.h common/common-types.h \
 common/common-debug.h common/cleanups.h common/gdb_setjmp.h \
-common/common-exceptions.h target/target.h
+common/common-exceptions.h target/target.h common/symbol.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
diff --git a/gdb/common/agent.c b/gdb/common/agent.c
index 0ac73a9..433ae2a 100644
--- a/gdb/common/agent.c
+++ b/gdb/common/agent.c
@@ -21,9 +21,9 @@
 #include "server.h"
 #else
 #include "defs.h"
-#include "objfiles.h"
 #endif
 #include "target/target.h"
+#include "common/symbol.h"
 #include <unistd.h>
 #include "agent.h"
 #include "filestuff.h"
@@ -98,18 +98,9 @@ agent_look_up_symbols (void *arg)
     {
       CORE_ADDR *addrp =
 	(CORE_ADDR *) ((char *) &ipa_sym_addrs + symbol_list[i].offset);
-#ifdef GDBSERVER
-
-      if (look_up_one_symbol (symbol_list[i].name, addrp, 1) == 0)
-#else
-      struct bound_minimal_symbol sym =
-	lookup_minimal_symbol (symbol_list[i].name, NULL,
-			       (struct objfile *) arg);
 
-      if (sym.minsym != NULL)
-	*addrp = BMSYMBOL_VALUE_ADDRESS (sym);
-      else
-#endif
+      if (find_minimal_symbol_address (symbol_list[i].name, addrp,
+				       arg) != 0)
 	{
 	  DEBUG_AGENT ("symbol `%s' not found\n", symbol_list[i].name);
 	  return -1;
diff --git a/gdb/common/symbol.h b/gdb/common/symbol.h
new file mode 100644
index 0000000..723d7fa
--- /dev/null
+++ b/gdb/common/symbol.h
@@ -0,0 +1,37 @@
+/* Declarations of common symbol functions.
+
+   Copyright (C) 2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef COMMON_SYMBOL_H
+#define COMMON_SYMBOL_H
+
+struct objfile;
+
+/* Find a symbol that matches NAME.  Limit the search to OBJFILE if
+   OBJFILE is non-NULL and the implementation supports limiting the
+   search to specific object files.  NAME may be mangled or demangled.
+   If a match is found, store the matching symbol's address in ADDR
+   and return zero.  Returns nonzero if no symbol matching NAME is
+   found.  Raise an exception if OBJFILE is non-NULL and the
+   implementation does not support limiting searches to specific
+   object files.  This function must be provided by the client.  */
+
+extern int find_minimal_symbol_address (const char *name, CORE_ADDR *addr,
+					struct objfile *objfile);
+
+#endif /* COMMON_SYMBOL_H */
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 8a5ee1f..0ea5a42 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,10 @@
+2014-09-11  Tom Tromey  <tromey@redhat.com>
+	    Gary Benson  <gbenson@redhat.com>
+
+	* symbol.c: New file.
+	* Makefile.in (SFILES): Add symbol.c.
+	(OBS): Add symbol.o.
+
 2014-09-11  Gary Benson  <gbenson@redhat.com>
 
 	* target.c (target_stop_ptid, target_continue_ptid): New
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 1447e61..074d93d 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -171,7 +171,7 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/nat/mips-linux-watch.c $(srcdir)/common/print-utils.c \
 	$(srcdir)/common/rsp-low.c $(srcdir)/common/errors.c \
 	$(srcdir)/common/common-debug.c $(srcdir)/common/cleanups.c \
-	$(srcdir)/common/common-exceptions.c
+	$(srcdir)/common/common-exceptions.c $(srcdir)/symbol.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -185,7 +185,7 @@ OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o \
       mem-break.o hostio.o event-loop.o tracepoint.o xml-utils.o \
       common-utils.o ptid.o buffer.o format.o filestuff.o dll.o notif.o \
       tdesc.o print-utils.o rsp-low.o errors.o common-debug.o cleanups.o \
-      common-exceptions.o \
+      common-exceptions.o symbol.o \
       $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
 GDBREPLAY_OBS = gdbreplay.o version.o
 GDBSERVER_LIBS = @GDBSERVER_LIBS@
diff --git a/gdb/gdbserver/symbol.c b/gdb/gdbserver/symbol.c
new file mode 100644
index 0000000..d1e6eef
--- /dev/null
+++ b/gdb/gdbserver/symbol.c
@@ -0,0 +1,32 @@
+/* Symbol manipulating routines for the remote server for GDB.
+
+   Copyright (C) 2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "server.h"
+#include "symbol.h"
+
+/* See common/symbol.h.  */
+
+int
+find_minimal_symbol_address (const char *name, CORE_ADDR *addr,
+			     struct objfile *objfile)
+{
+  gdb_assert (objfile == NULL);
+
+  return look_up_one_symbol (name, addr, 1) != 1;
+}
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index fd7fcd9..8eb7c85 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -50,6 +50,7 @@
 #include "cp-support.h"
 #include "language.h"
 #include "cli/cli-utils.h"
+#include "symbol.h"
 
 /* Accumulate the minimal symbols for each objfile in bunches of BUNCH_SIZE.
    At the end, copy them all into one newly allocated location on an objfile's
@@ -299,6 +300,21 @@ lookup_bound_minimal_symbol (const char *name)
   return lookup_minimal_symbol (name, NULL, NULL);
 }
 
+/* See common/symbol.h.  */
+
+int
+find_minimal_symbol_address (const char *name, CORE_ADDR *addr,
+			     struct objfile *objfile)
+{
+  struct bound_minimal_symbol sym
+    = lookup_minimal_symbol (name, NULL, objfile);
+
+  if (sym.minsym != NULL)
+    *addr = BMSYMBOL_VALUE_ADDRESS (sym);
+
+  return sym.minsym == NULL;
+}
+
 /* See minsyms.h.  */
 
 void