[v2] Add SVR4 psABI specific parser for AUXV entries

Message ID 20200408145832.24243-1-n54@gmx.com
State Superseded
Headers show
Series
  • [v2] Add SVR4 psABI specific parser for AUXV entries
Related show

Commit Message

Kamil Rytarowski April 8, 2020, 2:58 p.m.
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(-)

--
2.25.0

Comments

Simon Marchi April 9, 2020, 12:04 a.m. | #1
> 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
Kamil Rytarowski April 9, 2020, 12:57 a.m. | #2
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.

Patch

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);
 }