[06/11,v5] Add target/symbol.h

  This adds 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.

gdb/
2014-08-01  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* target/symbol.h: New file.
	* Makefile.in (HFILES_NO_SRCDIR): Add target/symbol.h.
	* target.h: Include target/symbol.h.
	* target.c (target_look_up_symbol): New function.
	* common/agent.c: Include target/symbol.h.
	[!GDBSERVER]: Don't include objfiles.h.
	(agent_look_up_symbols): Use target_look_up_symbol.

gdb/gdbserver/
2014-08-01  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* target.c: Include target/symbol.h.
	(target_look_up_symbol): New function.
---
 gdb/ChangeLog           |   11 +++++++++++
 gdb/Makefile.in         |    2 +-
 gdb/common/agent.c      |   14 ++------------
 gdb/gdbserver/ChangeLog |    6 ++++++
 gdb/gdbserver/target.c  |   12 ++++++++++++
 gdb/target.c            |   14 ++++++++++++++
 gdb/target.h            |    1 +
 gdb/target/symbol.h     |   36 ++++++++++++++++++++++++++++++++++++
 8 files changed, 83 insertions(+), 13 deletions(-)
 create mode 100644 gdb/target/symbol.h
  

Gary Benson writes:
 > This adds 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.
 > 
 > gdb/
 > 2014-08-01  Tom Tromey  <tromey@redhat.com>
 > 	    Gary Benson  <gbenson@redhat.com>
 > 
 > 	* target/symbol.h: New file.
 > 	* Makefile.in (HFILES_NO_SRCDIR): Add target/symbol.h.
 > 	* target.h: Include target/symbol.h.
 > 	* target.c (target_look_up_symbol): New function.
 > 	* common/agent.c: Include target/symbol.h.
 > 	[!GDBSERVER]: Don't include objfiles.h.
 > 	(agent_look_up_symbols): Use target_look_up_symbol.
 > 
 > gdb/gdbserver/
 > 2014-08-01  Tom Tromey  <tromey@redhat.com>
 > 	    Gary Benson  <gbenson@redhat.com>
 > 
 > 	* target.c: Include target/symbol.h.
 > 	(target_look_up_symbol): New function.
 > [...]
 > diff --git a/gdb/target/symbol.h b/gdb/target/symbol.h
 > new file mode 100644
 > index 0000000..bb37b72
 > --- /dev/null
 > +++ b/gdb/target/symbol.h
 > @@ -0,0 +1,36 @@
 > +/* Declarations of target symbol functions.
 > +
 > +   Copyright (C) 1986-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 TARGET_SYMBOL_H
 > +#define TARGET_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.  If a match is found, store the
 > +   matching symbol's address in ADDR and return nonzero.  Return zero
 > +   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.  */
 > +
 > +extern int target_look_up_symbol (const char *name, CORE_ADDR *addr,
 > +				  struct objfile *objfile);
 > +
 > +#endif /* TARGET_SYMBOL_H */
 > -- 
 > 1.7.1

Can this comment spell out that either a mangled or demangled
form of the symbol is allowed for NAME?
[assuming that that is indeed the case]

Also, the target/target.h memory routines return zero for success
and a non-zero error code for failure.  E.g.,

+/* Read LEN bytes of target memory at address MEMADDR, placing the
+   results in GDB's memory at MYADDR.  Return zero for success,
+   nonzero if any error occurs.  Implementations of this function may
+   define and use their own error codes, but functions in the common,
+   nat and target directories must treat the return code as opaque.
+   No guarantee is made about the contents of the data at MYADDR if
+   any error occurs.  */
+
+extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
+			       ssize_t len);

Do we want to be consistent here, and have the same results for
target_look_up_symbol? [and throughout the target API]
  

Doug Evans wrote:
> Gary Benson writes:
> > +/* 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.  If a match is found, store the
> > +   matching symbol's address in ADDR and return nonzero.  Return zero
> > +   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.  */
> > +
> > +extern int target_look_up_symbol (const char *name, CORE_ADDR *addr,
> > +				  struct objfile *objfile);
> 
> Can this comment spell out that either a mangled or demangled
> form of the symbol is allowed for NAME?
> [assuming that that is indeed the case]

It is the case.  I'll update the comment.

> Also, the target/target.h memory routines return zero for success
> and a non-zero error code for failure.  E.g.,
> 
> +/* Read LEN bytes of target memory at address MEMADDR, placing the
> +   results in GDB's memory at MYADDR.  Return zero for success,
> +   nonzero if any error occurs.  Implementations of this function may
> +   define and use their own error codes, but functions in the common,
> +   nat and target directories must treat the return code as opaque.
> +   No guarantee is made about the contents of the data at MYADDR if
> +   any error occurs.  */
> +
> +extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
> +			       ssize_t len);
> 
> Do we want to be consistent here, and have the same results for
> target_look_up_symbol? [and throughout the target API]

That seems reasonable to me, I'll make the change.

Thanks,
Gary
  

On 08/01/2014 11:19 AM, Gary Benson wrote:
> This adds 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.

A small parens:

I have to say that calling this new method target_foo looks kind of
awkward to me.  Unlike other target methods and helpers, that extract
info out of the target or tell the target to do something,
this goes in the other direction -- this is the target/backend/server
calling back to the client/symbol side for something.  Put another way,
seems like this method would never ultimately go through target_ops.

Thanks,
Pedro Alves
  

Pedro Alves wrote:
> On 08/01/2014 11:19 AM, Gary Benson wrote:
> > This adds 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.
> 
> A small parens:
> 
> I have to say that calling this new method target_foo looks kind
> of awkward to me.  Unlike other target methods and helpers, that
> extract info out of the target or tell the target to do something,
> this goes in the other direction -- this is the target/backend/
> server calling back to the client/symbol side for something.  Put
> another way, seems like this method would never ultimately go
> through target_ops.

Can you suggest a more suitable name?

Thanks,
Gary
  

On 08/20/2014 01:14 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 08/01/2014 11:19 AM, Gary Benson wrote:
>>> This adds 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.
>>
>> A small parens:
>>
>> I have to say that calling this new method target_foo looks kind
>> of awkward to me.  Unlike other target methods and helpers, that
>> extract info out of the target or tell the target to do something,
>> this goes in the other direction -- this is the target/backend/
>> server calling back to the client/symbol side for something.  Put
>> another way, seems like this method would never ultimately go
>> through target_ops.
> 
> Can you suggest a more suitable name?

find_minimal_symbol_address ?

I'd suggest moving this out of target.c too.  E.g., to a
new gdbserver/symbol.c or some such.