[PATCH/RFC] auxv entries

[PATCH/RFC] auxv entries

[Mark Kettenis]

Many, if not all, processor-specific ELF ABI documents have the
following definition of auxv entries:

typedef struct
{
       int   a_type;
       union {
              long a_val;
              void *a_ptr;
              void (*a_fcn)();
       } a_un;
} auxv_t;

This is not the layout that default_auxv_parse() uses though, which is
wrong for big-endian 64-bit systems.  The attached diff fixes this,
making the assumption that a_val is "naturally" aligned.

I have some slight worries though about how this ever worked on
big-endian 64-bit Linux systems, hence the RFC.


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* auxv.c (default_auxv_parse): Change code to reflect standard
	auxv_t layout.

Index: auxv.c
===================================================================
RCS file: /cvs/src/src/gdb/auxv.c,v
retrieving revision 1.14
diff -u -p -r1.14 auxv.c
--- auxv.c 11 Sep 2008 14:29:21 -0000 1.14
+++ auxv.c 14 Sep 2008 21:20:03 -0000
@@ -82,20 +82,22 @@ int
 default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
 		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
 {
-  const int sizeof_auxv_field = gdbarch_ptr_bit (target_gdbarch)
-				/ TARGET_CHAR_BIT;
+  struct type *builtin_int = builtin_type (target_gdbarch)->builtin_int;
+  struct type *builtin_long = builtin_type (target_gdbarch)->builtin_long;
+  const int sizeof_auxv_type = TYPE_LENGTH(builtin_int);
+  const int sizeof_auxv_val = TYPE_LENGTH(builtin_long);
   gdb_byte *ptr = *readptr;
 
   if (endptr == ptr)
     return 0;
 
-  if (endptr - ptr < sizeof_auxv_field * 2)
+  if (endptr - ptr < sizeof_auxv_val * 2)
     return -1;
 
-  *typep = extract_unsigned_integer (ptr, sizeof_auxv_field);
-  ptr += sizeof_auxv_field;
-  *valp = extract_unsigned_integer (ptr, sizeof_auxv_field);
-  ptr += sizeof_auxv_field;
+  *typep = extract_unsigned_integer (ptr, sizeof_auxv_type);
+  ptr += sizeof_auxv_val;	/* Alignment.  */
+  *valp = extract_unsigned_integer (ptr, sizeof_auxv_val);
+  ptr += sizeof_auxv_val;
 
   *readptr = ptr;
   return 1;

Re: [PATCH/RFC] auxv entries

[Thiago Jung Bauermann]

El dom, 14-09-2008 a las 23:27 +0200, Mark Kettenis escribió:
> Many, if not all, processor-specific ELF ABI documents have the
> following definition of auxv entries:
> 
> typedef struct
> {
>        int   a_type;
>        union {
>               long a_val;
>               void *a_ptr;
>               void (*a_fcn)();
>        } a_un;
> } auxv_t;

This is what I see in <elf.h> on ppc64-linux:


> 
> This is not the layout that default_auxv_parse() uses though, which is
> wrong for big-endian 64-bit systems.  The attached diff fixes this,
> making the assumption that a_val is "naturally" aligned.
> 
> I have some slight worries though about how this ever worked on
> big-endian 64-bit Linux systems, hence the RFC.
> 
> 
> Index: ChangeLog
> from  Mark Kettenis  <kettenis@gnu.org>
> 
> 	* auxv.c (default_auxv_parse): Change code to reflect standard
> 	auxv_t layout.
> 
> Index: auxv.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/auxv.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 auxv.c
> --- auxv.c 11 Sep 2008 14:29:21 -0000 1.14
> +++ auxv.c 14 Sep 2008 21:20:03 -0000
> @@ -82,20 +82,22 @@ int
>  default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
>  		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
>  {
> -  const int sizeof_auxv_field = gdbarch_ptr_bit (target_gdbarch)
> -				/ TARGET_CHAR_BIT;
> +  struct type *builtin_int = builtin_type (target_gdbarch)->builtin_int;
> +  struct type *builtin_long = builtin_type (target_gdbarch)->builtin_long;
> +  const int sizeof_auxv_type = TYPE_LENGTH(builtin_int);
> +  const int sizeof_auxv_val = TYPE_LENGTH(builtin_long);
>    gdb_byte *ptr = *readptr;
>  
>    if (endptr == ptr)
>      return 0;
>  
> -  if (endptr - ptr < sizeof_auxv_field * 2)
> +  if (endptr - ptr < sizeof_auxv_val * 2)
>      return -1;
>  
> -  *typep = extract_unsigned_integer (ptr, sizeof_auxv_field);
> -  ptr += sizeof_auxv_field;
> -  *valp = extract_unsigned_integer (ptr, sizeof_auxv_field);
> -  ptr += sizeof_auxv_field;
> +  *typep = extract_unsigned_integer (ptr, sizeof_auxv_type);
> +  ptr += sizeof_auxv_val;	/* Alignment.  */
> +  *valp = extract_unsigned_integer (ptr, sizeof_auxv_val);
> +  ptr += sizeof_auxv_val;
>  
>    *readptr = ptr;
>    return 1;
> 
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [PATCH/RFC] auxv entries

[Thiago Jung Bauermann]

(I'd really, really like to talk to the person who thought it would be a
good idea to make <ctrl>+<enter> a shortcut to send the message.)

El dom, 14-09-2008 a las 23:27 +0200, Mark Kettenis escribió:
> Many, if not all, processor-specific ELF ABI documents have the
> following definition of auxv entries:
> 
> typedef struct
> {
>        int   a_type;
>        union {
>               long a_val;
>               void *a_ptr;
>               void (*a_fcn)();
>        } a_un;
> } auxv_t;

I have this in <elf.h> in ppc64-linux:

typedef struct
{
  uint64_t a_type;              /* Entry type */
  union
    {
      uint64_t a_val;           /* Integer value */
      /* We use to have pointer elements added here.  We cannot do that,
         though, since it does not work when using 32-bit definitions
         on 64-bit platforms and vice versa.  */
    } a_un;
} Elf64_auxv_t;

So default_auxv_parse seems right to me.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [PATCH/RFC] auxv entries

[Mark Kettenis]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1460 bytes --]

> From: Thiago Jung Bauermann <bauerman@br.ibm.com>
> Date: Sun, 14 Sep 2008 21:33:04 -0300
> 
> (I'd really, really like to talk to the person who thought it would be a
> good idea to make <ctrl>+<enter> a shortcut to send the message.)

Heh

> El dom, 14-09-2008 a las 23:27 +0200, Mark Kettenis escribió:
> > Many, if not all, processor-specific ELF ABI documents have the
> > following definition of auxv entries:
> > 
> > typedef struct
> > {
> >        int   a_type;
> >        union {
> >               long a_val;
> >               void *a_ptr;
> >               void (*a_fcn)();
> >        } a_un;
> > } auxv_t;
> 
> I have this in <elf.h> in ppc64-linux:
> 
> typedef struct
> {
>   uint64_t a_type;              /* Entry type */
>   union
>     {
>       uint64_t a_val;           /* Integer value */
>       /* We use to have pointer elements added here.  We cannot do that,
>          though, since it does not work when using 32-bit definitions
>          on 64-bit platforms and vice versa.  */
>     } a_un;
> } Elf64_auxv_t;
> 
> So default_auxv_parse seems right to me.

Hmm, so it looks like Linux doesn't follow the 64-bit PowerPC ABI
here.  At least version 1.7 of that spec has the "standard" form of
the struct with a_type being an int.

OpenBSD and Solaris both use the "standard" form, so I'd argue that
Linux is the odd one out here.  I'll see if I can come up with a diff
that overrides the standard layout for 64-bit PPC linux.

Re: [PATCH/RFC] auxv entries

[Mark Kettenis]

> Date: Mon, 15 Sep 2008 11:20:31 +0200 (CEST)
> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> 
> > From: Thiago Jung Bauermann <bauerman@br.ibm.com>
> > Date: Sun, 14 Sep 2008 21:33:04 -0300
> > 
> > I have this in <elf.h> in ppc64-linux:
> > 
> > typedef struct
> > {
> >   uint64_t a_type;              /* Entry type */
> >   union
> >     {
> >       uint64_t a_val;           /* Integer value */
> >       /* We use to have pointer elements added here.  We cannot do that,
> >          though, since it does not work when using 32-bit definitions
> >          on 64-bit platforms and vice versa.  */
> >     } a_un;
> > } Elf64_auxv_t;
> > 
> > So default_auxv_parse seems right to me.
> 
> Hmm, so it looks like Linux doesn't follow the 64-bit PowerPC ABI
> here.  At least version 1.7 of that spec has the "standard" form of
> the struct with a_type being an int.
> 
> OpenBSD and Solaris both use the "standard" form, so I'd argue that
> Linux is the odd one out here.  I'll see if I can come up with a diff
> that overrides the standard layout for 64-bit PPC linux.
> 

Here is the promised diff.  Tested on Linux on amd64 (which doesn't
say a lot since it is little-endian).

ok?


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* auxv.c (default_auxv_parse): Change code to reflect standard
	auxv_t layout.
	* linux-nat.c (linux_nat_auxv_parse): New function.
	(linux_nat_add_target): Install linux_nat_auxv_parse.

Index: auxv.c
===================================================================
RCS file: /cvs/src/src/gdb/auxv.c,v
retrieving revision 1.14
diff -u -p -r1.14 auxv.c
--- auxv.c 11 Sep 2008 14:29:21 -0000 1.14
+++ auxv.c 15 Sep 2008 20:17:42 -0000
@@ -82,20 +82,22 @@ int
 default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
 		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
 {
-  const int sizeof_auxv_field = gdbarch_ptr_bit (target_gdbarch)
-				/ TARGET_CHAR_BIT;
+  struct type *builtin_int = builtin_type (target_gdbarch)->builtin_int;
+  struct type *builtin_long = builtin_type (target_gdbarch)->builtin_long;
+  const int sizeof_auxv_type = TYPE_LENGTH(builtin_int);
+  const int sizeof_auxv_val = TYPE_LENGTH(builtin_long);
   gdb_byte *ptr = *readptr;
 
   if (endptr == ptr)
     return 0;
 
-  if (endptr - ptr < sizeof_auxv_field * 2)
+  if (endptr - ptr < sizeof_auxv_val * 2)
     return -1;
 
-  *typep = extract_unsigned_integer (ptr, sizeof_auxv_field);
-  ptr += sizeof_auxv_field;
-  *valp = extract_unsigned_integer (ptr, sizeof_auxv_field);
-  ptr += sizeof_auxv_field;
+  *typep = extract_unsigned_integer (ptr, sizeof_auxv_type);
+  ptr += sizeof_auxv_val;	/* Alignment.  */
+  *valp = extract_unsigned_integer (ptr, sizeof_auxv_val);
+  ptr += sizeof_auxv_val;
 
   *readptr = ptr;
   return 1;
Index: linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-nat.c,v
retrieving revision 1.102
diff -u -p -r1.102 linux-nat.c
--- linux-nat.c 8 Sep 2008 21:51:18 -0000 1.102
+++ linux-nat.c 15 Sep 2008 20:17:44 -0000
@@ -4353,6 +4353,29 @@ send_sigint_callback (struct lwp_info *l
   return 0;
 }
 
+static int
+linux_nat_auxv_parse(struct target_ops *ops, gdb_byte **readptr,
+		     gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
+{
+  struct type *builtin_long = builtin_type (target_gdbarch)->builtin_long;
+  const int sizeof_auxv_field = TYPE_LENGTH(builtin_long);
+  gdb_byte *ptr = *readptr;
+
+  if (endptr == ptr)
+    return 0;
+
+  if (endptr - ptr < sizeof_auxv_field * 2)
+    return -1;
+
+  *typep = extract_unsigned_integer (ptr, sizeof_auxv_field);
+  ptr += sizeof_auxv_field;
+  *valp = extract_unsigned_integer (ptr, sizeof_auxv_field);
+  ptr += sizeof_auxv_field;
+
+  *readptr = ptr;
+  return 1;
+}
+
 static void
 linux_nat_stop (ptid_t ptid)
 {
@@ -4401,6 +4424,10 @@ linux_nat_add_target (struct target_ops 
   t->to_terminal_inferior = linux_nat_terminal_inferior;
   t->to_terminal_ours = linux_nat_terminal_ours;
 
+  /* The Linux folks messed up and didn't follow the ELF ABI on
+     various 64-bit architectures.  */
+  t->to_auxv_parse = linux_nat_auxv_parse;
+
   /* Methods for non-stop support.  */
   t->to_stop = linux_nat_stop;
 

Re: [PATCH/RFC] auxv entries

[Daniel Jacobowitz]

On Mon, Sep 15, 2008 at 10:22:37PM +0200, Mark Kettenis wrote:
> Here is the promised diff.  Tested on Linux on amd64 (which doesn't
> say a lot since it is little-endian).
> 
> ok?

I think the layout of the aux vector is a strange combination of an
architecture property and a target property.  We can read it via
gdbserver too; the remote protocol manual says it's just 'Access the
target's "auxiliary vector"'.  The comments on procfs_auxv_parse
imply that it depends on the host GDB in native Solaris debugging.
But on Linux it depends on the debuggee, not the debugger.

Is there some way we can differentiate based on gdbarch or osabi?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH/RFC] auxv entries

[Thiago Jung Bauermann]

El lun, 15-09-2008 a las 22:22 +0200, Mark Kettenis escribió:
> Here is the promised diff.  Tested on Linux on amd64 (which doesn't
> say a lot since it is little-endian).

Not sure if you're still interested, but I had already fired the
regression test anyway:

This patch introduces the following regressions in ppc64-linux:

 PASS: gdb.base/auxv.exp: generate native core dump
 PASS: gdb.base/auxv.exp: load core file for info auxv on native core dump
 PASS: gdb.base/auxv.exp: info auxv on native core dump
-PASS: gdb.base/auxv.exp: matching auxv data from live and core
+FAIL: gdb.base/auxv.exp: matching auxv data from live and core
 PASS: gdb.base/auxv.exp: load core file for info auxv on gcore-created dump
 PASS: gdb.base/auxv.exp: info auxv on gcore-created dump
-PASS: gdb.base/auxv.exp: matching auxv data from live and gcore
+FAIL: gdb.base/auxv.exp: matching auxv data from live and gcore

Here's the gdb.log for the failing run:

info auxv^M
0    AT_NULL              End of vector                  0x16^M
(gdb) PASS: gdb.base/auxv.exp: info auxv on native core dump
FAIL: gdb.base/auxv.exp: matching auxv data from live and core

Versus the gdb.log for the successful run:

info auxv^M
22   AT_IGNOREPPC         Entry should be ignored        22^M                                                                  22   AT_IGNOREPPC         Entry should be ignored        22^M
19   AT_DCACHEBSIZE       Data cache block size          128^M
20   AT_ICACHEBSIZE       Instruction cache block size   128^M
21   AT_UCACHEBSIZE       Unified cache block size       0^M
33   AT_SYSINFO_EHDR      System-supplied DSO's ELF header 0x100000^M
16   AT_HWCAP             Machine-dependent CPU capability hints 0xdc080000^M
6    AT_PAGESZ            System page size               4096^M
17   AT_CLKTCK            Frequency of times()           100^M
3    AT_PHDR              Program headers for program    0x10000040^M
4    AT_PHENT             Size of program header entry   56^M                                                                  5    AT_PHNUM             Number of program headers      9^M
7    AT_BASE              Base address of interpreter    0x40000000000^M
8    AT_FLAGS             Flags                          0x0^M
9    AT_ENTRY             Entry point of program         0x10010b98^M
11   AT_UID               Real user ID                   1001^M
12   AT_EUID              Effective user ID              1001^M
13   AT_GID               Real group ID                  100^M
14   AT_EGID              Effective group ID             100^M
23   AT_SECURE            Boolean, was exec setuid-like? 0^M
15   AT_PLATFORM          String identifying platform    0xfffffa62210 "ppc970"^M
0    AT_NULL              End of vector                  0x0^M
(gdb) PASS: gdb.base/auxv.exp: info auxv on native core dump
PASS: gdb.base/auxv.exp: matching auxv data from live and core

 
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [PATCH/RFC] auxv entries

[Daniel Jacobowitz]

On Sun, Sep 14, 2008 at 11:27:09PM +0200, Mark Kettenis wrote:
> Many, if not all, processor-specific ELF ABI documents have the
> following definition of auxv entries:
> 
> typedef struct
> {
>        int   a_type;
>        union {
>               long a_val;
>               void *a_ptr;
>               void (*a_fcn)();
>        } a_un;
> } auxv_t;
> 
> This is not the layout that default_auxv_parse() uses though, which is
> wrong for big-endian 64-bit systems.  The attached diff fixes this,
> making the assumption that a_val is "naturally" aligned.

If I remember right, this is a difference between Linux and other
systems (including Solaris).  Someone here was looking at
this in context of sparc-solaris2.10... Vladimir, was it you?

-- 
Daniel Jacobowitz
CodeSourcery