[PATCH] Add SVR4 psABI specific parser for AUXV entries

[PATCH] Add SVR4 psABI specific parser for AUXV entries

[Kamil Rytarowski]

NetBSD and OpenBSD always use an int to store the type as
defined in the SVR4 psABI specifications rather than long
as assumed by the default parser.

Define svr4_auxv_parse() that shares code with default_auxv_parse().

Remove obsd_auxv_parse() and switch OpenBSD to svr4_auxv_parse().
Remove not fully accurate comment from obsd-tdep.c.

Use svr4_auxv_parse() on NetBSD.

gdb/ChangeLog:

	* auxv.h (svr4_auxv_parse): New.
	* auxv.c (default_auxv_parse): Split into default_auxv_parse
	and generic_auxv_parse.
	(svr4_auxv_parse): Add.
	* obsd-tdep.c: Include "auxv.h".
	(obsd_auxv_parse): Remove.
	(obsd_init_abi): Remove comment.
	(obsd_init_abi): Change set_gdbarch_auxv_parse passed argument
	from `obsd_auxv_parse' to `svr4_auxv_parse'.
	* nbsd-tdep.c: Include "auxv.h".
	(nbsd_init_abi): Call set_gdbarch_auxv_parse.
---
 gdb/ChangeLog   | 14 ++++++++++++
 gdb/auxv.c      | 60 ++++++++++++++++++++++++++++++++++++-------------
 gdb/auxv.h      |  7 ++++++
 gdb/nbsd-tdep.c |  2 ++
 gdb/obsd-tdep.c | 30 ++-----------------------
 5 files changed, 70 insertions(+), 43 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 255262a2f27..1bb3c0892f2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,17 @@
+2020-04-07  Kamil Rytarowski  <n54@gmx.com>
+
+	* auxv.h (svr4_auxv_parse): New.
+	* auxv.c (default_auxv_parse): Split into default_auxv_parse
+	and generic_auxv_parse.
+	(svr4_auxv_parse): Add.
+	* obsd-tdep.c: Include "auxv.h".
+	(obsd_auxv_parse): Remove.
+	(obsd_init_abi): Remove comment.
+	(obsd_init_abi): Change set_gdbarch_auxv_parse passed argument
+	from `obsd_auxv_parse' to `svr4_auxv_parse'.
+	* nbsd-tdep.c: Include "auxv.h".
+	(nbsd_init_abi): Call set_gdbarch_auxv_parse.
+
 2020-04-07  Kamil Rytarowski  <n54@gmx.com>

 	* nbsd-tdep.c: Include "objfiles.h".
diff --git a/gdb/auxv.c b/gdb/auxv.c
index c13d7a22eb9..240f32236b3 100644
--- a/gdb/auxv.c
+++ b/gdb/auxv.c
@@ -248,34 +248,64 @@ memory_xfer_auxv (struct target_ops *ops,
   return procfs_xfer_auxv (readbuf, writebuf, offset, len, xfered_len);
 }

-/* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
-   Return 0 if *READPTR is already at the end of the buffer.
-   Return -1 if there is insufficient buffer for a whole entry.
-   Return 1 if an entry was read into *TYPEP and *VALP.  */
-int
-default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
-		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
+/* Generic auxv_parse body shared with default_auxv_parse() and
+   svr4_auxv_parse().  */
+
+static int
+generic_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
+		    gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp,
+		    int sizeof_auxv_type)
 {
-  const int sizeof_auxv_field = gdbarch_ptr_bit (target_gdbarch ())
-				/ TARGET_CHAR_BIT;
-  const enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
+  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
+  const int sizeof_auxv_val = TYPE_LENGTH (ptr_type);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   gdb_byte *ptr = *readptr;

   if (endptr == ptr)
     return 0;

-  if (endptr - ptr < sizeof_auxv_field * 2)
+  if (endptr - ptr < 2 * sizeof_auxv_val)
     return -1;

-  *typep = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
-  ptr += sizeof_auxv_field;
-  *valp = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
-  ptr += sizeof_auxv_field;
+  *typep = extract_unsigned_integer (ptr, sizeof_auxv_type, byte_order);
+  ptr += sizeof_auxv_val;      /* Alignment.  */
+  *valp = extract_unsigned_integer (ptr, sizeof_auxv_val, byte_order);
+  ptr += sizeof_auxv_val;

   *readptr = ptr;
   return 1;
 }

+/* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
+   Return 0 if *READPTR is already at the end of the buffer.
+   Return -1 if there is insufficient buffer for a whole entry.
+   Return 1 if an entry was read into *TYPEP and *VALP.  */
+int
+default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
+		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
+{
+  struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
+  const int sizeof_auxv_type = TYPE_LENGTH (ptr_type);
+
+  return generic_auxv_parse (target_gdbarch (), readptr, endptr, typep, valp,
+			     sizeof_auxv_type);
+}
+
+/* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
+   Return 0 if *READPTR is already at the end of the buffer.
+   Return -1 if there is insufficient buffer for a whole entry.
+   Return 1 if an entry was read into *TYPEP and *VALP.  */
+int
+svr4_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
+		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
+{
+  struct type *int_type = builtin_type (gdbarch)->builtin_int;
+  const int sizeof_auxv_type = TYPE_LENGTH (int_type);
+
+  return generic_auxv_parse (gdbarch, readptr, endptr, typep, valp,
+			     sizeof_auxv_type);
+}
+
 /* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
    Return 0 if *READPTR is already at the end of the buffer.
    Return -1 if there is insufficient buffer for a whole entry.
diff --git a/gdb/auxv.h b/gdb/auxv.h
index a5a932ec80e..0056d39955c 100644
--- a/gdb/auxv.h
+++ b/gdb/auxv.h
@@ -31,6 +31,13 @@ extern int default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
 			       gdb_byte *endptr, CORE_ADDR *typep,
 			       CORE_ADDR *valp);

+/* The SVR4 psABI implementation of to_auxv_parse, that uses an int to
+   store the type rather than long as assumed by the default parser.  */
+
+extern int svr4_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
+			       gdb_byte *endptr, CORE_ADDR *typep,
+			       CORE_ADDR *valp);
+
 /* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
    Return 0 if *READPTR is already at the end of the buffer.
    Return -1 if there is insufficient buffer for a whole entry.
diff --git a/gdb/nbsd-tdep.c b/gdb/nbsd-tdep.c
index 1d7230feef8..158a43bebaa 100644
--- a/gdb/nbsd-tdep.c
+++ b/gdb/nbsd-tdep.c
@@ -20,6 +20,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

 #include "defs.h"
+#include "auxv.h"
 #include "solib-svr4.h"
 #include "nbsd-tdep.h"
 #include "gdbarch.h"
@@ -362,4 +363,5 @@ nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_gdb_signal_from_target (gdbarch, nbsd_gdb_signal_from_target);
   set_gdbarch_gdb_signal_to_target (gdbarch, nbsd_gdb_signal_to_target);
   set_gdbarch_skip_solib_resolver (gdbarch, nbsd_skip_solib_resolver);
+  set_gdbarch_auxv_parse (gdbarch, svr4_auxv_parse);
 }
diff --git a/gdb/obsd-tdep.c b/gdb/obsd-tdep.c
index 1c1f574ef19..f2c4d298851 100644
--- a/gdb/obsd-tdep.c
+++ b/gdb/obsd-tdep.c
@@ -18,6 +18,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

 #include "defs.h"
+#include "auxv.h"
 #include "frame.h"
 #include "symtab.h"
 #include "objfiles.h"
@@ -289,32 +290,6 @@ obsd_gdb_signal_to_target (struct gdbarch *gdbarch,
   return -1;
 }

-static int
-obsd_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
-		 gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
-{
-  struct type *int_type = builtin_type (gdbarch)->builtin_int;
-  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
-  const int sizeof_auxv_type = TYPE_LENGTH (int_type);
-  const int sizeof_auxv_val = TYPE_LENGTH (ptr_type);
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  gdb_byte *ptr = *readptr;
-
-  if (endptr == ptr)
-    return 0;
-
-  if (endptr - ptr < 2 * sizeof_auxv_val)
-    return -1;
-
-  *typep = extract_unsigned_integer (ptr, sizeof_auxv_type, byte_order);
-  ptr += sizeof_auxv_val;	/* Alignment.  */
-  *valp = extract_unsigned_integer (ptr, sizeof_auxv_val, byte_order);
-  ptr += sizeof_auxv_val;
-
-  *readptr = ptr;
-  return 1;
-}
-
 void
 obsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -323,6 +298,5 @@ obsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_gdb_signal_to_target (gdbarch,
 				    obsd_gdb_signal_to_target);

-  /* Unlike Linux, OpenBSD actually follows the ELF standard.  */
-  set_gdbarch_auxv_parse (gdbarch, obsd_auxv_parse);
+  set_gdbarch_auxv_parse (gdbarch, svr4_auxv_parse);
 }
--
2.25.0

Re: [PATCH] Add SVR4 psABI specific parser for AUXV entries

[Simon Marchi]

On 2020-04-07 10:14 p.m., Kamil Rytarowski wrote:
> NetBSD and OpenBSD always use an int to store the type as
> defined in the SVR4 psABI specifications rather than long
> as assumed by the default parser.
> 
> Define svr4_auxv_parse() that shares code with default_auxv_parse().
> 
> Remove obsd_auxv_parse() and switch OpenBSD to svr4_auxv_parse().
> Remove not fully accurate comment from obsd-tdep.c.
> 
> Use svr4_auxv_parse() on NetBSD.
> 
> gdb/ChangeLog:
> 
> 	* auxv.h (svr4_auxv_parse): New.
> 	* auxv.c (default_auxv_parse): Split into default_auxv_parse
> 	and generic_auxv_parse.
> 	(svr4_auxv_parse): Add.
> 	* obsd-tdep.c: Include "auxv.h".
> 	(obsd_auxv_parse): Remove.
> 	(obsd_init_abi): Remove comment.
> 	(obsd_init_abi): Change set_gdbarch_auxv_parse passed argument
> 	from `obsd_auxv_parse' to `svr4_auxv_parse'.
> 	* nbsd-tdep.c: Include "auxv.h".
> 	(nbsd_init_abi): Call set_gdbarch_auxv_parse.

Thanks, just a few minor comments.

> diff --git a/gdb/auxv.c b/gdb/auxv.c
> index c13d7a22eb9..240f32236b3 100644
> --- a/gdb/auxv.c
> +++ b/gdb/auxv.c
> @@ -248,34 +248,64 @@ memory_xfer_auxv (struct target_ops *ops,
>    return procfs_xfer_auxv (readbuf, writebuf, offset, len, xfered_len);
>  }
> 
> -/* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
> -   Return 0 if *READPTR is already at the end of the buffer.
> -   Return -1 if there is insufficient buffer for a whole entry.
> -   Return 1 if an entry was read into *TYPEP and *VALP.  */
> -int
> -default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
> -		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
> +/* Generic auxv_parse body shared with default_auxv_parse() and
> +   svr4_auxv_parse().  */

Instead of saying that it's used by this and that function (which is doomed
to become obsolete very easily anyway), say what is the particularity of this
function, compared to other auxv_parse functions: it takes the size of the
auxv type field as a parameter.

> +
> +static int
> +generic_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
> +		    gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp,
> +		    int sizeof_auxv_type)
>  {
> -  const int sizeof_auxv_field = gdbarch_ptr_bit (target_gdbarch ())
> -				/ TARGET_CHAR_BIT;
> -  const enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
> +  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
> +  const int sizeof_auxv_val = TYPE_LENGTH (ptr_type);
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>    gdb_byte *ptr = *readptr;
> 
>    if (endptr == ptr)
>      return 0;
> 
> -  if (endptr - ptr < sizeof_auxv_field * 2)
> +  if (endptr - ptr < 2 * sizeof_auxv_val)
>      return -1;
> 
> -  *typep = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
> -  ptr += sizeof_auxv_field;
> -  *valp = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
> -  ptr += sizeof_auxv_field;
> +  *typep = extract_unsigned_integer (ptr, sizeof_auxv_type, byte_order);
> +  ptr += sizeof_auxv_val;      /* Alignment.  */

Instead of just "Alignment", please add a comment explaining that even if the
auxv type takes less space than an auxv value, there is padding after the type
such that the value is aligned on a multiple of its size (and this is why we
advance by `sizeof_auxv_val` and not `sizeof_auxv_type`.  I think that can
be surprising for somebody who does not know this fact.

> +  *valp = extract_unsigned_integer (ptr, sizeof_auxv_val, byte_order);
> +  ptr += sizeof_auxv_val;
> 
>    *readptr = ptr;
>    return 1;
>  }
> 
> +/* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
> +   Return 0 if *READPTR is already at the end of the buffer.
> +   Return -1 if there is insufficient buffer for a whole entry.
> +   Return 1 if an entry was read into *TYPEP and *VALP.  */

Since this is the implementation of an exported function, replace the comment
here with:

  /* See auxv.h.  */

and document in the header file.  I think the documentation you have put in the
header file for svr4_auxv_parse is good, it explains what the function does and
how it's different from default_auxv_parse.  I don't think it's particularly
useful to describe all parameters here, since it's a function that implements
an interface.  The parameters should be documented where the interface is
defined.

> +int
> +default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
> +		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)

This last line is not properly aligned on the parenthesis.

> +{
> +  struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
> +  const int sizeof_auxv_type = TYPE_LENGTH (ptr_type);
> +
> +  return generic_auxv_parse (target_gdbarch (), readptr, endptr, typep, valp,
> +			     sizeof_auxv_type);
> +}
> +
> +/* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
> +   Return 0 if *READPTR is already at the end of the buffer.
> +   Return -1 if there is insufficient buffer for a whole entry.
> +   Return 1 if an entry was read into *TYPEP and *VALP.  */

Same comment about the comment.

> +int
> +svr4_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
> +		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)

This last line is not properly aligned on the parenthesis.

Simon

[PATCH v2] Add SVR4 psABI specific parser for AUXV entries

[Kamil Rytarowski]

NetBSD and OpenBSD always use an int to store the type as
defined in the SVR4 psABI specifications rather than long
as assumed by the default parser.

Define svr4_auxv_parse() that shares code with default_auxv_parse().

Remove obsd_auxv_parse() and switch OpenBSD to svr4_auxv_parse().
Remove not fully accurate comment from obsd-tdep.c.

Use svr4_auxv_parse() on NetBSD.

gdb/ChangeLog:

	* auxv.h (svr4_auxv_parse): New.
	* auxv.c (default_auxv_parse): Split into default_auxv_parse
	and generic_auxv_parse.
	(svr4_auxv_parse): Add.
	* obsd-tdep.c: Include "auxv.h".
	(obsd_auxv_parse): Remove.
	(obsd_init_abi): Remove comment.
	(obsd_init_abi): Change set_gdbarch_auxv_parse passed argument
	from `obsd_auxv_parse' to `svr4_auxv_parse'.
	* nbsd-tdep.c: Include "auxv.h".
	(nbsd_init_abi): Call set_gdbarch_auxv_parse.
---
 gdb/ChangeLog   | 14 ++++++++++++
 gdb/auxv.c      | 61 +++++++++++++++++++++++++++++++++++++------------
 gdb/auxv.h      | 17 +++++++++++++-
 gdb/nbsd-tdep.c |  2 ++
 gdb/obsd-tdep.c | 30 ++----------------------
 5 files changed, 80 insertions(+), 44 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 255262a2f27..1bb3c0892f2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,17 @@
+2020-04-07  Kamil Rytarowski  <n54@gmx.com>
+
+	* auxv.h (svr4_auxv_parse): New.
+	* auxv.c (default_auxv_parse): Split into default_auxv_parse
+	and generic_auxv_parse.
+	(svr4_auxv_parse): Add.
+	* obsd-tdep.c: Include "auxv.h".
+	(obsd_auxv_parse): Remove.
+	(obsd_init_abi): Remove comment.
+	(obsd_init_abi): Change set_gdbarch_auxv_parse passed argument
+	from `obsd_auxv_parse' to `svr4_auxv_parse'.
+	* nbsd-tdep.c: Include "auxv.h".
+	(nbsd_init_abi): Call set_gdbarch_auxv_parse.
+
 2020-04-07  Kamil Rytarowski  <n54@gmx.com>

 	* nbsd-tdep.c: Include "objfiles.h".
diff --git a/gdb/auxv.c b/gdb/auxv.c
index c13d7a22eb9..28e8d2f704d 100644
--- a/gdb/auxv.c
+++ b/gdb/auxv.c
@@ -248,34 +248,65 @@ memory_xfer_auxv (struct target_ops *ops,
   return procfs_xfer_auxv (readbuf, writebuf, offset, len, xfered_len);
 }

-/* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
-   Return 0 if *READPTR is already at the end of the buffer.
-   Return -1 if there is insufficient buffer for a whole entry.
-   Return 1 if an entry was read into *TYPEP and *VALP.  */
-int
-default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
-		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
+/* This function compared to other auxv_parse functions: it takes the size of
+   the auxv type field as a parameter.  */
+
+static int
+generic_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
+		    gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp,
+		    int sizeof_auxv_type)
 {
-  const int sizeof_auxv_field = gdbarch_ptr_bit (target_gdbarch ())
-				/ TARGET_CHAR_BIT;
-  const enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
+  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
+  const int sizeof_auxv_val = TYPE_LENGTH (ptr_type);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   gdb_byte *ptr = *readptr;

   if (endptr == ptr)
     return 0;

-  if (endptr - ptr < sizeof_auxv_field * 2)
+  if (endptr - ptr < 2 * sizeof_auxv_val)
     return -1;

-  *typep = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
-  ptr += sizeof_auxv_field;
-  *valp = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
-  ptr += sizeof_auxv_field;
+  *typep = extract_unsigned_integer (ptr, sizeof_auxv_type, byte_order);
+  /* Even if the auxv type takes less space than an auxv value, there is
+     padding after the type such that the value is aligned on a multiple of
+     its size (and this is why we advance by `sizeof_auxv_val` and not
+     `sizeof_auxv_type`.  */
+  ptr += sizeof_auxv_val;
+  *valp = extract_unsigned_integer (ptr, sizeof_auxv_val, byte_order);
+  ptr += sizeof_auxv_val;

   *readptr = ptr;
   return 1;
 }

+/* See auxv.h.  */
+
+int
+default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
+		    gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
+{
+  struct gdbarch *gdbarch = target_gdbarch ();
+  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
+  const int sizeof_auxv_type = TYPE_LENGTH (ptr_type);
+
+  return generic_auxv_parse (gdbarch, readptr, endptr, typep, valp,
+			     sizeof_auxv_type);
+}
+
+/* See auxv.h.  */
+
+int
+svr4_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
+		 gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
+{
+  struct type *int_type = builtin_type (gdbarch)->builtin_int;
+  const int sizeof_auxv_type = TYPE_LENGTH (int_type);
+
+  return generic_auxv_parse (gdbarch, readptr, endptr, typep, valp,
+			     sizeof_auxv_type);
+}
+
 /* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
    Return 0 if *READPTR is already at the end of the buffer.
    Return -1 if there is insufficient buffer for a whole entry.
diff --git a/gdb/auxv.h b/gdb/auxv.h
index a5a932ec80e..c1d11ea2c7b 100644
--- a/gdb/auxv.h
+++ b/gdb/auxv.h
@@ -25,12 +25,27 @@
 /* See "include/elf/common.h" for the definition of valid AT_* values.  */

 /* The default implementation of to_auxv_parse, used by the target
-   stack.  */
+   stack.

+   Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
+   Return 0 if *READPTR is already at the end of the buffer.
+   Return -1 if there is insufficient buffer for a whole entry.
+   Return 1 if an entry was read into *TYPEP and *VALP.  */
 extern int default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
 			       gdb_byte *endptr, CORE_ADDR *typep,
 			       CORE_ADDR *valp);

+/* The SVR4 psABI implementation of to_auxv_parse, that uses an int to
+   store the type rather than long as assumed by the default parser.
+
+   Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
+   Return 0 if *READPTR is already at the end of the buffer.
+   Return -1 if there is insufficient buffer for a whole entry.
+   Return 1 if an entry was read into *TYPEP and *VALP.  */
+extern int svr4_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
+			       gdb_byte *endptr, CORE_ADDR *typep,
+			       CORE_ADDR *valp);
+
 /* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
    Return 0 if *READPTR is already at the end of the buffer.
    Return -1 if there is insufficient buffer for a whole entry.
diff --git a/gdb/nbsd-tdep.c b/gdb/nbsd-tdep.c
index 1d7230feef8..158a43bebaa 100644
--- a/gdb/nbsd-tdep.c
+++ b/gdb/nbsd-tdep.c
@@ -20,6 +20,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

 #include "defs.h"
+#include "auxv.h"
 #include "solib-svr4.h"
 #include "nbsd-tdep.h"
 #include "gdbarch.h"
@@ -362,4 +363,5 @@ nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_gdb_signal_from_target (gdbarch, nbsd_gdb_signal_from_target);
   set_gdbarch_gdb_signal_to_target (gdbarch, nbsd_gdb_signal_to_target);
   set_gdbarch_skip_solib_resolver (gdbarch, nbsd_skip_solib_resolver);
+  set_gdbarch_auxv_parse (gdbarch, svr4_auxv_parse);
 }
diff --git a/gdb/obsd-tdep.c b/gdb/obsd-tdep.c
index 1c1f574ef19..f2c4d298851 100644
--- a/gdb/obsd-tdep.c
+++ b/gdb/obsd-tdep.c
@@ -18,6 +18,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

 #include "defs.h"
+#include "auxv.h"
 #include "frame.h"
 #include "symtab.h"
 #include "objfiles.h"
@@ -289,32 +290,6 @@ obsd_gdb_signal_to_target (struct gdbarch *gdbarch,
   return -1;
 }

-static int
-obsd_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
-		 gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
-{
-  struct type *int_type = builtin_type (gdbarch)->builtin_int;
-  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
-  const int sizeof_auxv_type = TYPE_LENGTH (int_type);
-  const int sizeof_auxv_val = TYPE_LENGTH (ptr_type);
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  gdb_byte *ptr = *readptr;
-
-  if (endptr == ptr)
-    return 0;
-
-  if (endptr - ptr < 2 * sizeof_auxv_val)
-    return -1;
-
-  *typep = extract_unsigned_integer (ptr, sizeof_auxv_type, byte_order);
-  ptr += sizeof_auxv_val;	/* Alignment.  */
-  *valp = extract_unsigned_integer (ptr, sizeof_auxv_val, byte_order);
-  ptr += sizeof_auxv_val;
-
-  *readptr = ptr;
-  return 1;
-}
-
 void
 obsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -323,6 +298,5 @@ obsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_gdb_signal_to_target (gdbarch,
 				    obsd_gdb_signal_to_target);

-  /* Unlike Linux, OpenBSD actually follows the ELF standard.  */
-  set_gdbarch_auxv_parse (gdbarch, obsd_auxv_parse);
+  set_gdbarch_auxv_parse (gdbarch, svr4_auxv_parse);
 }
--
2.25.0

Re: [PATCH] Add SVR4 psABI specific parser for AUXV entries

[Kamil Rytarowski]

On 08.04.2020 06:11, Simon Marchi wrote:
> On 2020-04-07 10:14 p.m., Kamil Rytarowski wrote:
>> NetBSD and OpenBSD always use an int to store the type as
>> defined in the SVR4 psABI specifications rather than long
>> as assumed by the default parser.
>>
>> Define svr4_auxv_parse() that shares code with default_auxv_parse().
>>
>> Remove obsd_auxv_parse() and switch OpenBSD to svr4_auxv_parse().
>> Remove not fully accurate comment from obsd-tdep.c.
>>
>> Use svr4_auxv_parse() on NetBSD.
>>
>> gdb/ChangeLog:
>>
>> 	* auxv.h (svr4_auxv_parse): New.
>> 	* auxv.c (default_auxv_parse): Split into default_auxv_parse
>> 	and generic_auxv_parse.
>> 	(svr4_auxv_parse): Add.
>> 	* obsd-tdep.c: Include "auxv.h".
>> 	(obsd_auxv_parse): Remove.
>> 	(obsd_init_abi): Remove comment.
>> 	(obsd_init_abi): Change set_gdbarch_auxv_parse passed argument
>> 	from `obsd_auxv_parse' to `svr4_auxv_parse'.
>> 	* nbsd-tdep.c: Include "auxv.h".
>> 	(nbsd_init_abi): Call set_gdbarch_auxv_parse.
>
> Thanks, just a few minor comments.
>

Please see v2.

>> diff --git a/gdb/auxv.c b/gdb/auxv.c
>> index c13d7a22eb9..240f32236b3 100644
>> --- a/gdb/auxv.c
>> +++ b/gdb/auxv.c
>> @@ -248,34 +248,64 @@ memory_xfer_auxv (struct target_ops *ops,
>>    return procfs_xfer_auxv (readbuf, writebuf, offset, len, xfered_len);
>>  }
>>
>> -/* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
>> -   Return 0 if *READPTR is already at the end of the buffer.
>> -   Return -1 if there is insufficient buffer for a whole entry.
>> -   Return 1 if an entry was read into *TYPEP and *VALP.  */
>> -int
>> -default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
>> -		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
>> +/* Generic auxv_parse body shared with default_auxv_parse() and
>> +   svr4_auxv_parse().  */
>
> Instead of saying that it's used by this and that function (which is doomed
> to become obsolete very easily anyway), say what is the particularity of this
> function, compared to other auxv_parse functions: it takes the size of the
> auxv type field as a parameter.
>
>> +
>> +static int
>> +generic_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
>> +		    gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp,
>> +		    int sizeof_auxv_type)
>>  {
>> -  const int sizeof_auxv_field = gdbarch_ptr_bit (target_gdbarch ())
>> -				/ TARGET_CHAR_BIT;
>> -  const enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
>> +  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
>> +  const int sizeof_auxv_val = TYPE_LENGTH (ptr_type);
>> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>>    gdb_byte *ptr = *readptr;
>>
>>    if (endptr == ptr)
>>      return 0;
>>
>> -  if (endptr - ptr < sizeof_auxv_field * 2)
>> +  if (endptr - ptr < 2 * sizeof_auxv_val)
>>      return -1;
>>
>> -  *typep = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
>> -  ptr += sizeof_auxv_field;
>> -  *valp = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
>> -  ptr += sizeof_auxv_field;
>> +  *typep = extract_unsigned_integer (ptr, sizeof_auxv_type, byte_order);
>> +  ptr += sizeof_auxv_val;      /* Alignment.  */
>
> Instead of just "Alignment", please add a comment explaining that even if the
> auxv type takes less space than an auxv value, there is padding after the type
> such that the value is aligned on a multiple of its size (and this is why we
> advance by `sizeof_auxv_val` and not `sizeof_auxv_type`.  I think that can
> be surprising for somebody who does not know this fact.
>
>> +  *valp = extract_unsigned_integer (ptr, sizeof_auxv_val, byte_order);
>> +  ptr += sizeof_auxv_val;
>>
>>    *readptr = ptr;
>>    return 1;
>>  }
>>
>> +/* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
>> +   Return 0 if *READPTR is already at the end of the buffer.
>> +   Return -1 if there is insufficient buffer for a whole entry.
>> +   Return 1 if an entry was read into *TYPEP and *VALP.  */
>
> Since this is the implementation of an exported function, replace the comment
> here with:
>
>   /* See auxv.h.  */
>
> and document in the header file.  I think the documentation you have put in the
> header file for svr4_auxv_parse is good, it explains what the function does and
> how it's different from default_auxv_parse.  I don't think it's particularly
> useful to describe all parameters here, since it's a function that implements
> an interface.  The parameters should be documented where the interface is
> defined.
>
>> +int
>> +default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
>> +		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
>
> This last line is not properly aligned on the parenthesis.
>
>> +{
>> +  struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
>> +  const int sizeof_auxv_type = TYPE_LENGTH (ptr_type);
>> +
>> +  return generic_auxv_parse (target_gdbarch (), readptr, endptr, typep, valp,
>> +			     sizeof_auxv_type);
>> +}
>> +
>> +/* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
>> +   Return 0 if *READPTR is already at the end of the buffer.
>> +   Return -1 if there is insufficient buffer for a whole entry.
>> +   Return 1 if an entry was read into *TYPEP and *VALP.  */
>
> Same comment about the comment.
>
>> +int
>> +svr4_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
>> +		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
>
> This last line is not properly aligned on the parenthesis.
>
> Simon
>

Re: [PATCH v2] Add SVR4 psABI specific parser for AUXV entries

[Simon Marchi]

> diff --git a/gdb/auxv.c b/gdb/auxv.c
> index c13d7a22eb9..28e8d2f704d 100644
> --- a/gdb/auxv.c
> +++ b/gdb/auxv.c
> @@ -248,34 +248,65 @@ memory_xfer_auxv (struct target_ops *ops,
>    return procfs_xfer_auxv (readbuf, writebuf, offset, len, xfered_len);
>  }
> 
> -/* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
> -   Return 0 if *READPTR is already at the end of the buffer.
> -   Return -1 if there is insufficient buffer for a whole entry.
> -   Return 1 if an entry was read into *TYPEP and *VALP.  */
> -int
> -default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
> -		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
> +/* This function compared to other auxv_parse functions: it takes the size of
> +   the auxv type field as a parameter.  */
> +
> +static int
> +generic_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
> +		    gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp,
> +		    int sizeof_auxv_type)
>  {
> -  const int sizeof_auxv_field = gdbarch_ptr_bit (target_gdbarch ())
> -				/ TARGET_CHAR_BIT;
> -  const enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
> +  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
> +  const int sizeof_auxv_val = TYPE_LENGTH (ptr_type);
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>    gdb_byte *ptr = *readptr;
> 
>    if (endptr == ptr)
>      return 0;
> 
> -  if (endptr - ptr < sizeof_auxv_field * 2)
> +  if (endptr - ptr < 2 * sizeof_auxv_val)
>      return -1;
> 
> -  *typep = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
> -  ptr += sizeof_auxv_field;
> -  *valp = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
> -  ptr += sizeof_auxv_field;
> +  *typep = extract_unsigned_integer (ptr, sizeof_auxv_type, byte_order);
> +  /* Even if the auxv type takes less space than an auxv value, there is
> +     padding after the type such that the value is aligned on a multiple of
> +     its size (and this is why we advance by `sizeof_auxv_val` and not
> +     `sizeof_auxv_type`.  */

The parenthesis in the comment is not closed.

> diff --git a/gdb/auxv.h b/gdb/auxv.h
> index a5a932ec80e..c1d11ea2c7b 100644
> --- a/gdb/auxv.h
> +++ b/gdb/auxv.h
> @@ -25,12 +25,27 @@
>  /* See "include/elf/common.h" for the definition of valid AT_* values.  */
> 
>  /* The default implementation of to_auxv_parse, used by the target
> -   stack.  */
> +   stack.
> 
> +   Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
> +   Return 0 if *READPTR is already at the end of the buffer.
> +   Return -1 if there is insufficient buffer for a whole entry.
> +   Return 1 if an entry was read into *TYPEP and *VALP.  */
>  extern int default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
>  			       gdb_byte *endptr, CORE_ADDR *typep,
>  			       CORE_ADDR *valp);
> 
> +/* The SVR4 psABI implementation of to_auxv_parse, that uses an int to
> +   store the type rather than long as assumed by the default parser.
> +
> +   Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
> +   Return 0 if *READPTR is already at the end of the buffer.
> +   Return -1 if there is insufficient buffer for a whole entry.
> +   Return 1 if an entry was read into *TYPEP and *VALP.  */
> +extern int svr4_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
> +			       gdb_byte *endptr, CORE_ADDR *typep,
> +			       CORE_ADDR *valp);

The last two lines are not properly aligned on the parenthesis.

Simon

[PATCH v3] Add SVR4 psABI specific parser for AUXV entries

[Kamil Rytarowski]

NetBSD and OpenBSD always use an int to store the type as
defined in the SVR4 psABI specifications rather than long
as assumed by the default parser.

Define svr4_auxv_parse() that shares code with default_auxv_parse().

Remove obsd_auxv_parse() and switch OpenBSD to svr4_auxv_parse().
Remove not fully accurate comment from obsd-tdep.c.

Use svr4_auxv_parse() on NetBSD.

gdb/ChangeLog:

	* auxv.h (svr4_auxv_parse): New.
	* auxv.c (default_auxv_parse): Split into default_auxv_parse
	and generic_auxv_parse.
	(svr4_auxv_parse): Add.
	* obsd-tdep.c: Include "auxv.h".
	(obsd_auxv_parse): Remove.
	(obsd_init_abi): Remove comment.
	(obsd_init_abi): Change set_gdbarch_auxv_parse passed argument
	from `obsd_auxv_parse' to `svr4_auxv_parse'.
	* nbsd-tdep.c: Include "auxv.h".
	(nbsd_init_abi): Call set_gdbarch_auxv_parse.
---
 gdb/ChangeLog   | 14 ++++++++++++
 gdb/auxv.c      | 61 +++++++++++++++++++++++++++++++++++++------------
 gdb/auxv.h      | 17 +++++++++++++-
 gdb/nbsd-tdep.c |  2 ++
 gdb/obsd-tdep.c | 30 ++----------------------
 5 files changed, 80 insertions(+), 44 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 255262a2f27..1bb3c0892f2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,17 @@
+2020-04-07  Kamil Rytarowski  <n54@gmx.com>
+
+	* auxv.h (svr4_auxv_parse): New.
+	* auxv.c (default_auxv_parse): Split into default_auxv_parse
+	and generic_auxv_parse.
+	(svr4_auxv_parse): Add.
+	* obsd-tdep.c: Include "auxv.h".
+	(obsd_auxv_parse): Remove.
+	(obsd_init_abi): Remove comment.
+	(obsd_init_abi): Change set_gdbarch_auxv_parse passed argument
+	from `obsd_auxv_parse' to `svr4_auxv_parse'.
+	* nbsd-tdep.c: Include "auxv.h".
+	(nbsd_init_abi): Call set_gdbarch_auxv_parse.
+
 2020-04-07  Kamil Rytarowski  <n54@gmx.com>

 	* nbsd-tdep.c: Include "objfiles.h".
diff --git a/gdb/auxv.c b/gdb/auxv.c
index c13d7a22eb9..2ffcd73b988 100644
--- a/gdb/auxv.c
+++ b/gdb/auxv.c
@@ -248,34 +248,65 @@ memory_xfer_auxv (struct target_ops *ops,
   return procfs_xfer_auxv (readbuf, writebuf, offset, len, xfered_len);
 }

-/* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
-   Return 0 if *READPTR is already at the end of the buffer.
-   Return -1 if there is insufficient buffer for a whole entry.
-   Return 1 if an entry was read into *TYPEP and *VALP.  */
-int
-default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
-		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
+/* This function compared to other auxv_parse functions: it takes the size of
+   the auxv type field as a parameter.  */
+
+static int
+generic_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
+		    gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp,
+		    int sizeof_auxv_type)
 {
-  const int sizeof_auxv_field = gdbarch_ptr_bit (target_gdbarch ())
-				/ TARGET_CHAR_BIT;
-  const enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
+  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
+  const int sizeof_auxv_val = TYPE_LENGTH (ptr_type);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   gdb_byte *ptr = *readptr;

   if (endptr == ptr)
     return 0;

-  if (endptr - ptr < sizeof_auxv_field * 2)
+  if (endptr - ptr < 2 * sizeof_auxv_val)
     return -1;

-  *typep = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
-  ptr += sizeof_auxv_field;
-  *valp = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
-  ptr += sizeof_auxv_field;
+  *typep = extract_unsigned_integer (ptr, sizeof_auxv_type, byte_order);
+  /* Even if the auxv type takes less space than an auxv value, there is
+     padding after the type such that the value is aligned on a multiple of
+     its size (and this is why we advance by `sizeof_auxv_val` and not
+     `sizeof_auxv_type`).  */
+  ptr += sizeof_auxv_val;
+  *valp = extract_unsigned_integer (ptr, sizeof_auxv_val, byte_order);
+  ptr += sizeof_auxv_val;

   *readptr = ptr;
   return 1;
 }

+/* See auxv.h.  */
+
+int
+default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
+		    gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
+{
+  struct gdbarch *gdbarch = target_gdbarch ();
+  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
+  const int sizeof_auxv_type = TYPE_LENGTH (ptr_type);
+
+  return generic_auxv_parse (gdbarch, readptr, endptr, typep, valp,
+			     sizeof_auxv_type);
+}
+
+/* See auxv.h.  */
+
+int
+svr4_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
+		 gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
+{
+  struct type *int_type = builtin_type (gdbarch)->builtin_int;
+  const int sizeof_auxv_type = TYPE_LENGTH (int_type);
+
+  return generic_auxv_parse (gdbarch, readptr, endptr, typep, valp,
+			     sizeof_auxv_type);
+}
+
 /* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
    Return 0 if *READPTR is already at the end of the buffer.
    Return -1 if there is insufficient buffer for a whole entry.
diff --git a/gdb/auxv.h b/gdb/auxv.h
index a5a932ec80e..9525801e37d 100644
--- a/gdb/auxv.h
+++ b/gdb/auxv.h
@@ -25,12 +25,27 @@
 /* See "include/elf/common.h" for the definition of valid AT_* values.  */

 /* The default implementation of to_auxv_parse, used by the target
-   stack.  */
+   stack.

+   Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
+   Return 0 if *READPTR is already at the end of the buffer.
+   Return -1 if there is insufficient buffer for a whole entry.
+   Return 1 if an entry was read into *TYPEP and *VALP.  */
 extern int default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
 			       gdb_byte *endptr, CORE_ADDR *typep,
 			       CORE_ADDR *valp);

+/* The SVR4 psABI implementation of to_auxv_parse, that uses an int to
+   store the type rather than long as assumed by the default parser.
+
+   Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
+   Return 0 if *READPTR is already at the end of the buffer.
+   Return -1 if there is insufficient buffer for a whole entry.
+   Return 1 if an entry was read into *TYPEP and *VALP.  */
+extern int svr4_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
+			    gdb_byte *endptr, CORE_ADDR *typep,
+			    CORE_ADDR *valp);
+
 /* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
    Return 0 if *READPTR is already at the end of the buffer.
    Return -1 if there is insufficient buffer for a whole entry.
diff --git a/gdb/nbsd-tdep.c b/gdb/nbsd-tdep.c
index 1d7230feef8..158a43bebaa 100644
--- a/gdb/nbsd-tdep.c
+++ b/gdb/nbsd-tdep.c
@@ -20,6 +20,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

 #include "defs.h"
+#include "auxv.h"
 #include "solib-svr4.h"
 #include "nbsd-tdep.h"
 #include "gdbarch.h"
@@ -362,4 +363,5 @@ nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_gdb_signal_from_target (gdbarch, nbsd_gdb_signal_from_target);
   set_gdbarch_gdb_signal_to_target (gdbarch, nbsd_gdb_signal_to_target);
   set_gdbarch_skip_solib_resolver (gdbarch, nbsd_skip_solib_resolver);
+  set_gdbarch_auxv_parse (gdbarch, svr4_auxv_parse);
 }
diff --git a/gdb/obsd-tdep.c b/gdb/obsd-tdep.c
index 1c1f574ef19..f2c4d298851 100644
--- a/gdb/obsd-tdep.c
+++ b/gdb/obsd-tdep.c
@@ -18,6 +18,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

 #include "defs.h"
+#include "auxv.h"
 #include "frame.h"
 #include "symtab.h"
 #include "objfiles.h"
@@ -289,32 +290,6 @@ obsd_gdb_signal_to_target (struct gdbarch *gdbarch,
   return -1;
 }

-static int
-obsd_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
-		 gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
-{
-  struct type *int_type = builtin_type (gdbarch)->builtin_int;
-  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
-  const int sizeof_auxv_type = TYPE_LENGTH (int_type);
-  const int sizeof_auxv_val = TYPE_LENGTH (ptr_type);
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  gdb_byte *ptr = *readptr;
-
-  if (endptr == ptr)
-    return 0;
-
-  if (endptr - ptr < 2 * sizeof_auxv_val)
-    return -1;
-
-  *typep = extract_unsigned_integer (ptr, sizeof_auxv_type, byte_order);
-  ptr += sizeof_auxv_val;	/* Alignment.  */
-  *valp = extract_unsigned_integer (ptr, sizeof_auxv_val, byte_order);
-  ptr += sizeof_auxv_val;
-
-  *readptr = ptr;
-  return 1;
-}
-
 void
 obsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -323,6 +298,5 @@ obsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_gdb_signal_to_target (gdbarch,
 				    obsd_gdb_signal_to_target);

-  /* Unlike Linux, OpenBSD actually follows the ELF standard.  */
-  set_gdbarch_auxv_parse (gdbarch, obsd_auxv_parse);
+  set_gdbarch_auxv_parse (gdbarch, svr4_auxv_parse);
 }
--
2.25.0

Re: [PATCH v2] Add SVR4 psABI specific parser for AUXV entries

[Kamil Rytarowski]

On 09.04.2020 02:04, Simon Marchi wrote:
>> diff --git a/gdb/auxv.c b/gdb/auxv.c
>> index c13d7a22eb9..28e8d2f704d 100644
>> --- a/gdb/auxv.c
>> +++ b/gdb/auxv.c
>> @@ -248,34 +248,65 @@ memory_xfer_auxv (struct target_ops *ops,
>>    return procfs_xfer_auxv (readbuf, writebuf, offset, len, xfered_len);
>>  }
>>
>> -/* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
>> -   Return 0 if *READPTR is already at the end of the buffer.
>> -   Return -1 if there is insufficient buffer for a whole entry.
>> -   Return 1 if an entry was read into *TYPEP and *VALP.  */
>> -int
>> -default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
>> -		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
>> +/* This function compared to other auxv_parse functions: it takes the size of
>> +   the auxv type field as a parameter.  */
>> +
>> +static int
>> +generic_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
>> +		    gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp,
>> +		    int sizeof_auxv_type)
>>  {
>> -  const int sizeof_auxv_field = gdbarch_ptr_bit (target_gdbarch ())
>> -				/ TARGET_CHAR_BIT;
>> -  const enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
>> +  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
>> +  const int sizeof_auxv_val = TYPE_LENGTH (ptr_type);
>> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>>    gdb_byte *ptr = *readptr;
>>
>>    if (endptr == ptr)
>>      return 0;
>>
>> -  if (endptr - ptr < sizeof_auxv_field * 2)
>> +  if (endptr - ptr < 2 * sizeof_auxv_val)
>>      return -1;
>>
>> -  *typep = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
>> -  ptr += sizeof_auxv_field;
>> -  *valp = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
>> -  ptr += sizeof_auxv_field;
>> +  *typep = extract_unsigned_integer (ptr, sizeof_auxv_type, byte_order);
>> +  /* Even if the auxv type takes less space than an auxv value, there is
>> +     padding after the type such that the value is aligned on a multiple of
>> +     its size (and this is why we advance by `sizeof_auxv_val` and not
>> +     `sizeof_auxv_type`.  */
>
> The parenthesis in the comment is not closed.
>

Fixed.

>> diff --git a/gdb/auxv.h b/gdb/auxv.h
>> index a5a932ec80e..c1d11ea2c7b 100644
>> --- a/gdb/auxv.h
>> +++ b/gdb/auxv.h
>> @@ -25,12 +25,27 @@
>>  /* See "include/elf/common.h" for the definition of valid AT_* values.  */
>>
>>  /* The default implementation of to_auxv_parse, used by the target
>> -   stack.  */
>> +   stack.
>>
>> +   Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
>> +   Return 0 if *READPTR is already at the end of the buffer.
>> +   Return -1 if there is insufficient buffer for a whole entry.
>> +   Return 1 if an entry was read into *TYPEP and *VALP.  */
>>  extern int default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
>>  			       gdb_byte *endptr, CORE_ADDR *typep,
>>  			       CORE_ADDR *valp);
>>
>> +/* The SVR4 psABI implementation of to_auxv_parse, that uses an int to
>> +   store the type rather than long as assumed by the default parser.
>> +
>> +   Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
>> +   Return 0 if *READPTR is already at the end of the buffer.
>> +   Return -1 if there is insufficient buffer for a whole entry.
>> +   Return 1 if an entry was read into *TYPEP and *VALP.  */
>> +extern int svr4_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
>> +			       gdb_byte *endptr, CORE_ADDR *typep,
>> +			       CORE_ADDR *valp);
>
> The last two lines are not properly aligned on the parenthesis.
>

Fixed.

> Simon
>

Please see v3.

Re: [PATCH v3] Add SVR4 psABI specific parser for AUXV entries

[Simon Marchi]

On 2020-04-08 8:56 p.m., Kamil Rytarowski wrote:
> NetBSD and OpenBSD always use an int to store the type as
> defined in the SVR4 psABI specifications rather than long
> as assumed by the default parser.
> 
> Define svr4_auxv_parse() that shares code with default_auxv_parse().
> 
> Remove obsd_auxv_parse() and switch OpenBSD to svr4_auxv_parse().
> Remove not fully accurate comment from obsd-tdep.c.
> 
> Use svr4_auxv_parse() on NetBSD.
> 
> gdb/ChangeLog:
> 
> 	* auxv.h (svr4_auxv_parse): New.
> 	* auxv.c (default_auxv_parse): Split into default_auxv_parse
> 	and generic_auxv_parse.
> 	(svr4_auxv_parse): Add.
> 	* obsd-tdep.c: Include "auxv.h".
> 	(obsd_auxv_parse): Remove.
> 	(obsd_init_abi): Remove comment.
> 	(obsd_init_abi): Change set_gdbarch_auxv_parse passed argument
> 	from `obsd_auxv_parse' to `svr4_auxv_parse'.
> 	* nbsd-tdep.c: Include "auxv.h".
> 	(nbsd_init_abi): Call set_gdbarch_auxv_parse.
>

Thanks, this is ok.

Simon