Re: [PATCH/RFC] Parsing auxv entries

[Pedro Alves <palves at redhat dot com>]

On 02/04/2014 02:41 PM, Mark Kettenis wrote:
> The diff below adds a gdbarch method to parse auxv entries.  The
> parsing code currently lives in the target_ops vector, and this poses
> a problem.  Because default_auxv_parse() implements parsing of the
> non-standard auxv entries used by Linux (where the type is stored in a
> 'long' instead of an 'int'), it doesn't work on 64-bit big-endian
> targets that do follow the SVR4 layout.  I've worked around this by
> overriding to_auxv_parse in inf_ptrace_trace() for native BSD targets,
> but that means the core_ops target is still broken.  And as we build
> binaries as PIE by default on OpenBSD now, where auxv parsing is
> essential to find out the load address of the executable, reading core
> dumps on our 64-bit big-endian platforms is pretty much broken right
> now.  And overriding to_auxv_parse for the core_ops target is painful.
> 
> I believe gdbarch is the right place for this functionality.  On many
> platforms the memory layout of the entries is consistent across the
> various sources.  

> But there may be some issues with running 32-bit
> binaries on 64-bit systems in relation to things like /proc/*/auxv.  I
> suppose to_auxv_parse was designed to solve such problems, although I
> fail to see how the current implementation would work in the scenario
> of running 32-bit binarie on a platform where /proc/*/auxv provides
> the auxv entries in 64-bit format.

Yeah.  See the Solaris version:

#if defined (PR_MODEL_NATIVE) && (PR_MODEL_NATIVE == PR_MODEL_LP64)
/* When GDB is built as 64-bit application on Solaris, the auxv data
   is presented in 64-bit format.  We need to provide a custom parser
   to handle that.  */
static int
procfs_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
{

> 
> Thoughts?  OK?

Hmm, thoughts then.

gdbarch does seems the right place, in principle.  Though
things in practice don't look that simple.

Consider for example a 32-bit Solaris gdb, connected to a
64-bit gdbserver.  In that case, the auxv data is presented
to gdbserver in a 64-bit format, and then in turn that's what
gdbserver sends back to GDB in response to TARGET_OBJECT_AUXV.
GDB won't be able to figure out which layout of auxv it's
looking at, unless perhaps it looks at the auxv block
size (ewwwww), or explicitly asks the target/server which
variant it sends.

Not sure what the real proper fix for this would be.  Several
options I see.  There might be more or even better ones.

  #1 - Install a solaris-specific gdbarch parse auxv hook
    that has gdb ask the target which variant of auxv is handed
    to gdb to work with.
  #2 - Hide the fact that the auxv data is presented differently
    depending on the bitness of the superior, by making the target
    do layout translation when returning the TARGET_OBJECT_AUXV
    object (like done with TARGET_OBJECT_SIGNAL_INFO on Linux).
  #3 - Hide the fact that the auxv data is presented differently
    depending on the bitness of the superior, by making the target
    always translate the auxv block to a host and target
    independent format that the core consumes (xml?).

#2 seems tempting; though so does #3, a little.  Dunno, #1 does
too, just a little, perhaps not.

And PowerPC has a similar issue:

 https://sourceware.org/ml/gdb-patches/2009-01/msg00440.html

And that shows that we can't move the auxv parsing to
gdbarch by default on Linux either.  At least, not if we don't
consult the target before the gdbarch hook.  But then, it
sounds like 32-bit gdb against 64-bit gdbserver on ppc might
be similarly broken in some scenarios.  #3 above starts
sounding a little better than #2.

So I swing back -- thought?  :-)

> 
> 
> 2014-02-04  Mark Kettenis  <kettenis@gnu.org>
>  
> 	* gdbarch.sh (parse_auxv): New.
> 	* gdbarch.h: Regenerated.
> 	* gdbarch.c: Regenerated.
> 	* auxv.c (target_auxv_parse): Call gdbarch_parse_auxv if provided.
> 
> diff --git a/gdb/auxv.c b/gdb/auxv.c
> index dd13237..dcc6345 100644
> --- a/gdb/auxv.c
> +++ b/gdb/auxv.c
> @@ -269,8 +269,12 @@ int
>  target_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
>                    gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
>  {
> +  struct gdbarch *gdbarch = target_gdbarch();
>    struct target_ops *t;
>  
> +  if (gdbarch_parse_auxv_p (gdbarch))
> +    return gdbarch_parse_auxv (gdbarch, readptr, endptr, typep, valp);

If this proceeds, can you please rename the hook in accordance
to the target method?  That is, gdbarch_auxv_parse rather than
gdbarch_parse_auxv, so that greps have a better chance of
hitting both?

-- 
Pedro Alves