[PATCH] Correct decoding AUXV on NetBSD

[PATCH] Correct decoding AUXV on NetBSD

[Kamil Rytarowski]

The type field is always 32bit. The value field reflects the size of
the register/pointer.

gdb/ChangeLog:

	* auxv.c (default_auxv_parse): Add new variable sizeof_auxv_type
	and use it in extract_unsigned_integer().
---
 gdb/ChangeLog | 5 +++++
 gdb/auxv.c    | 7 ++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 04db5e76734..5a2eb8e3416 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2020-03-14  Kamil Rytarowski  <n54@gmx.com>
+
+	* auxv.c (default_auxv_parse): Add new variable sizeof_auxv_type
+	and use it in extract_unsigned_integer().
+
 2020-03-14  Kamil Rytarowski  <n54@gmx.com>

 	* m68k-bsd-nat.c (fetch_registers): New variable lwp and pass
diff --git a/gdb/auxv.c b/gdb/auxv.c
index c13d7a22eb9..a035034a920 100644
--- a/gdb/auxv.c
+++ b/gdb/auxv.c
@@ -258,6 +258,11 @@ default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
 {
   const int sizeof_auxv_field = gdbarch_ptr_bit (target_gdbarch ())
 				/ TARGET_CHAR_BIT;
+#ifdef __NetBSD__
+  const int sizeof_auxv_type = sizeof(int32_t);
+#else
+  const int sizeof_auxv_type = sizeof_auxv_field;
+#endif
   const enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
   gdb_byte *ptr = *readptr;

@@ -267,7 +272,7 @@ default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
   if (endptr - ptr < sizeof_auxv_field * 2)
     return -1;

-  *typep = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
+  *typep = extract_unsigned_integer (ptr, sizeof_auxv_type, byte_order);
   ptr += sizeof_auxv_field;
   *valp = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
   ptr += sizeof_auxv_field;
--
2.25.0

Re: [PATCH] Correct decoding AUXV on NetBSD

[Tom Tromey]

>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:

Kamil> The type field is always 32bit. The value field reflects the size of
Kamil> the register/pointer.

Kamil> gdb/ChangeLog:

Kamil> 	* auxv.c (default_auxv_parse): Add new variable sizeof_auxv_type
Kamil> 	and use it in extract_unsigned_integer().

Kamil> +#ifdef __NetBSD__
Kamil> +  const int sizeof_auxv_type = sizeof(int32_t);
Kamil> +#else
Kamil> +  const int sizeof_auxv_type = sizeof_auxv_field;
Kamil> +#endif

IIUC, sizeof_auxv_type describes a property of the target.  That means
this approach is incorrect, because it will only work for native
debugging, and will do the wrong thing in other cases.

If default_auxv_parse is incorrect for NetBSD, another way is to
override it in the appropriate gdbarch.  See target_auxv_parse.  OpenBSD
appears to do this, see obsd_auxv_parse.

Tom

[PATCH v2] Add support for "info auxv" on NetBSD

[Kamil Rytarowski]

Register nbsd_auxv_parse() that overloads the default (Linux-style)
AUXV parsing. On NetBSD the type parameter is defined as int32_t
for all architectures.

Register nbsd_init_abi() that sets set_gdbarch_auxv_parse().

Call nbsd_init_abi() from all CPU-specific NetBSD tdep files.

gdb/ChangeLog:

	* nbsd-tdep.c: Include "gdbarch.h".
	* nbsd-tdep.c (nbsd_auxv_parse, nbsd_init_abi): New.
	* alpha-nbsd-tdep.c (alphanbsd_init_abi): Call nbsd_init_abi().
	* amd64-nbsd-tdep.c (amd64nbsd_init_abi): Likewise.
	* arm-nbsd-tdep.c (arm_netbsd_elf_init_abi): Likewise.
	* hppa-nbsd-tdep.c (hppanbsd_init_abi): Likewise.
	* i386-nbsd-tdep.c (i386nbsd_init_abi): Likewise.
	* mips-nbsd-tdep.c (nbsd_init_abi): Likewise.
	* ppc-nbsd-tdep.c (ppcnbsd_init_abi): Likewise.
	* sh-nbsd-tdep.c (shnbsd_init_abi): Likewise.
	* sparc-nbsd-tdep.c (sparc32nbsd_init_abi): Likewise.
	* sparc64-nbsd-tdep.c (sparc64nbsd_init_abi): Likewise.
	* vax-nbsd-tdep.c (vaxnbsd_elf_init_abi): Likewise.
---
 gdb/ChangeLog           | 16 ++++++++++++++++
 gdb/alpha-nbsd-tdep.c   |  2 ++
 gdb/amd64-nbsd-tdep.c   |  1 +
 gdb/arm-nbsd-tdep.c     |  3 +++
 gdb/hppa-nbsd-tdep.c    |  2 ++
 gdb/i386-nbsd-tdep.c    |  2 ++
 gdb/mips-nbsd-tdep.c    |  2 ++
 gdb/nbsd-tdep.c         | 33 +++++++++++++++++++++++++++++++++
 gdb/nbsd-tdep.h         |  2 ++
 gdb/ppc-nbsd-tdep.c     |  2 ++
 gdb/sh-nbsd-tdep.c      |  1 +
 gdb/sparc-nbsd-tdep.c   |  2 ++
 gdb/sparc64-nbsd-tdep.c |  2 ++
 gdb/vax-nbsd-tdep.c     |  2 ++
 14 files changed, 72 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a0d97584189..42b2129d104 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,19 @@
+2020-03-16  Kamil Rytarowski  <n54@gmx.com>
+
+	* nbsd-tdep.c: Include "gdbarch.h".
+	* nbsd-tdep.c (nbsd_auxv_parse, nbsd_init_abi): New.
+	* alpha-nbsd-tdep.c (alphanbsd_init_abi): Call nbsd_init_abi().
+	* amd64-nbsd-tdep.c (amd64nbsd_init_abi): Likewise.
+	* arm-nbsd-tdep.c (arm_netbsd_elf_init_abi): Likewise.
+	* hppa-nbsd-tdep.c (hppanbsd_init_abi): Likewise.
+	* i386-nbsd-tdep.c (i386nbsd_init_abi): Likewise.
+	* mips-nbsd-tdep.c (nbsd_init_abi): Likewise.
+	* ppc-nbsd-tdep.c (ppcnbsd_init_abi): Likewise.
+	* sh-nbsd-tdep.c (shnbsd_init_abi): Likewise.
+	* sparc-nbsd-tdep.c (sparc32nbsd_init_abi): Likewise.
+	* sparc64-nbsd-tdep.c (sparc64nbsd_init_abi): Likewise.
+	* vax-nbsd-tdep.c (vaxnbsd_elf_init_abi): Likewise.
+
 2020-03-14  Tom Tromey  <tom@tromey.com>

 	* c-typeprint.c (cp_type_print_method_args): Print "__restrict__"
diff --git a/gdb/alpha-nbsd-tdep.c b/gdb/alpha-nbsd-tdep.c
index ab9240e35da..58294edb3f6 100644
--- a/gdb/alpha-nbsd-tdep.c
+++ b/gdb/alpha-nbsd-tdep.c
@@ -258,6 +258,8 @@ alphanbsd_init_abi (struct gdbarch_info info,
   /* Hook into the MDEBUG frame unwinder.  */
   alpha_mdebug_init_abi (info, gdbarch);

+  nbsd_init_abi (info, gdbarch);
+
   /* NetBSD/alpha does not provide single step support via ptrace(2); we
      must use software single-stepping.  */
   set_gdbarch_software_single_step (gdbarch, alpha_software_single_step);
diff --git a/gdb/amd64-nbsd-tdep.c b/gdb/amd64-nbsd-tdep.c
index 89d07236b85..59071488ed7 100644
--- a/gdb/amd64-nbsd-tdep.c
+++ b/gdb/amd64-nbsd-tdep.c
@@ -106,6 +106,7 @@ amd64nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)

   amd64_init_abi (info, gdbarch,
 		  amd64_target_description (X86_XSTATE_SSE_MASK, true));
+  nbsd_init_abi (info, gdbarch);

   tdep->jb_pc_offset = 7 * 8;

diff --git a/gdb/arm-nbsd-tdep.c b/gdb/arm-nbsd-tdep.c
index e01df50bc25..a6104f760b3 100644
--- a/gdb/arm-nbsd-tdep.c
+++ b/gdb/arm-nbsd-tdep.c
@@ -150,6 +150,9 @@ arm_netbsd_elf_init_abi (struct gdbarch_info info,
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

   arm_netbsd_init_abi_common (info, gdbarch);
+
+  nbsd_init_abi (info, gdbarch);
+
   if (tdep->fp_model == ARM_FLOAT_AUTO)
     tdep->fp_model = ARM_FLOAT_SOFT_VFP;

diff --git a/gdb/hppa-nbsd-tdep.c b/gdb/hppa-nbsd-tdep.c
index b532ab1d5cc..d601aa96f3f 100644
--- a/gdb/hppa-nbsd-tdep.c
+++ b/gdb/hppa-nbsd-tdep.c
@@ -201,6 +201,8 @@ hppanbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   /* Obviously NetBSD is BSD-based.  */
   hppabsd_init_abi (info, gdbarch);

+  nbsd_init_abi (info, gdbarch);
+
   /* Core file support.  */
   set_gdbarch_iterate_over_regset_sections
     (gdbarch, hppanbsd_iterate_over_regset_sections);
diff --git a/gdb/i386-nbsd-tdep.c b/gdb/i386-nbsd-tdep.c
index 3157451e08f..f350412d9bd 100644
--- a/gdb/i386-nbsd-tdep.c
+++ b/gdb/i386-nbsd-tdep.c
@@ -377,6 +377,8 @@ i386nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   /* Obviously NetBSD is BSD-based.  */
   i386bsd_init_abi (info, gdbarch);

+  nbsd_init_abi (info, gdbarch);
+
   /* NetBSD has a different `struct reg'.  */
   tdep->gregset_reg_offset = i386nbsd_r_reg_offset;
   tdep->gregset_num_regs = ARRAY_SIZE (i386nbsd_r_reg_offset);
diff --git a/gdb/mips-nbsd-tdep.c b/gdb/mips-nbsd-tdep.c
index 38bc7ce636b..6f4d22b24fb 100644
--- a/gdb/mips-nbsd-tdep.c
+++ b/gdb/mips-nbsd-tdep.c
@@ -354,6 +354,8 @@ static void
 mipsnbsd_init_abi (struct gdbarch_info info,
                    struct gdbarch *gdbarch)
 {
+  nbsd_init_abi (info, gdbarch)
+
   set_gdbarch_iterate_over_regset_sections
     (gdbarch, mipsnbsd_iterate_over_regset_sections);

diff --git a/gdb/nbsd-tdep.c b/gdb/nbsd-tdep.c
index 49bb2b706bd..a9207e16625 100644
--- a/gdb/nbsd-tdep.c
+++ b/gdb/nbsd-tdep.c
@@ -22,6 +22,7 @@
 #include "defs.h"
 #include "solib-svr4.h"
 #include "nbsd-tdep.h"
+#include "gdbarch.h"

 /* FIXME: kettenis/20060115: We should really eliminate the next two
    functions completely.  */
@@ -47,3 +48,35 @@ nbsd_pc_in_sigtramp (CORE_ADDR pc, const char *func_name)
   return (func_name != NULL
 	  && startswith (func_name, "__sigtramp"));
 }
+
+static int
+nbsd_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
+nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  set_gdbarch_auxv_parse (gdbarch, nbsd_auxv_parse);
+}
diff --git a/gdb/nbsd-tdep.h b/gdb/nbsd-tdep.h
index c99a8b537b6..9301ff7a69a 100644
--- a/gdb/nbsd-tdep.h
+++ b/gdb/nbsd-tdep.h
@@ -25,4 +25,6 @@ struct link_map_offsets *nbsd_lp64_solib_svr4_fetch_link_map_offsets (void);

 int nbsd_pc_in_sigtramp (CORE_ADDR, const char *);

+void nbsd_init_abi (struct gdbarch_info, struct gdbarch *);
+
 #endif /* NBSD_TDEP_H */
diff --git a/gdb/ppc-nbsd-tdep.c b/gdb/ppc-nbsd-tdep.c
index d75930c9f8d..81492deaccd 100644
--- a/gdb/ppc-nbsd-tdep.c
+++ b/gdb/ppc-nbsd-tdep.c
@@ -173,6 +173,8 @@ static void
 ppcnbsd_init_abi (struct gdbarch_info info,
                   struct gdbarch *gdbarch)
 {
+  nbsd_init_abi (info, gdbarch);
+
   /* For NetBSD, this is an on again, off again thing.  Some systems
      do use the broken struct convention, and some don't.  */
   set_gdbarch_return_value (gdbarch, ppcnbsd_return_value);
diff --git a/gdb/sh-nbsd-tdep.c b/gdb/sh-nbsd-tdep.c
index aa319261684..2b2a7e3fd4a 100644
--- a/gdb/sh-nbsd-tdep.c
+++ b/gdb/sh-nbsd-tdep.c
@@ -63,6 +63,7 @@ shnbsd_init_abi (struct gdbarch_info info,
                   struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  nbsd_init_abi (info, gdbarch);

   tdep->core_gregmap = (struct sh_corefile_regmap *)regmap;
   tdep->sizeof_gregset = 84;
diff --git a/gdb/sparc-nbsd-tdep.c b/gdb/sparc-nbsd-tdep.c
index 7aba6020d27..ab1b557c57c 100644
--- a/gdb/sparc-nbsd-tdep.c
+++ b/gdb/sparc-nbsd-tdep.c
@@ -296,6 +296,8 @@ sparc32nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

+  nbsd_init_abi (info, gdbarch);
+
   /* NetBSD doesn't support the 128-bit `long double' from the psABI.  */
   set_gdbarch_long_double_bit (gdbarch, 64);
   set_gdbarch_long_double_format (gdbarch, floatformats_ieee_double);
diff --git a/gdb/sparc64-nbsd-tdep.c b/gdb/sparc64-nbsd-tdep.c
index cd5bfe89410..dac7fa78b9b 100644
--- a/gdb/sparc64-nbsd-tdep.c
+++ b/gdb/sparc64-nbsd-tdep.c
@@ -249,6 +249,8 @@ sparc64nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

+  nbsd_init_abi (info, gdbarch);
+
   tdep->gregset = &sparc64nbsd_gregset;
   tdep->sizeof_gregset = 160;

diff --git a/gdb/vax-nbsd-tdep.c b/gdb/vax-nbsd-tdep.c
index c2c08cc1603..7630ac5ab94 100644
--- a/gdb/vax-nbsd-tdep.c
+++ b/gdb/vax-nbsd-tdep.c
@@ -29,6 +29,8 @@
 static void
 vaxnbsd_elf_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
+  nbsd_init_abi (info, gdbarch);
+
   /* NetBSD ELF uses SVR4-style shared libraries.  */
   set_solib_svr4_fetch_link_map_offsets
     (gdbarch, svr4_ilp32_fetch_link_map_offsets);
--
2.25.0

Re: [PATCH] Correct decoding AUXV on NetBSD

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 1048 bytes --]

On 16.03.2020 14:25, Tom Tromey wrote:
>>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:
> 
> Kamil> The type field is always 32bit. The value field reflects the size of
> Kamil> the register/pointer.
> 
> Kamil> gdb/ChangeLog:
> 
> Kamil> 	* auxv.c (default_auxv_parse): Add new variable sizeof_auxv_type
> Kamil> 	and use it in extract_unsigned_integer().
> 
> Kamil> +#ifdef __NetBSD__
> Kamil> +  const int sizeof_auxv_type = sizeof(int32_t);
> Kamil> +#else
> Kamil> +  const int sizeof_auxv_type = sizeof_auxv_field;
> Kamil> +#endif
> 
> IIUC, sizeof_auxv_type describes a property of the target.  That means
> this approach is incorrect, because it will only work for native
> debugging, and will do the wrong thing in other cases.
> 
> If default_auxv_parse is incorrect for NetBSD, another way is to
> override it in the appropriate gdbarch.  See target_auxv_parse.  OpenBSD
> appears to do this, see obsd_auxv_parse.
> 
> Tom
> 

Done. Please see "[PATCH v2] Add support for "info auxv" on NetBSD".


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] Add support for "info auxv" on NetBSD

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 10400 bytes --]

Ping?

I would like to reuse nbsd_init_abi() for more code from my pile of patches.

On 16.03.2020 19:17, Kamil Rytarowski wrote:
> Register nbsd_auxv_parse() that overloads the default (Linux-style)
> AUXV parsing. On NetBSD the type parameter is defined as int32_t
> for all architectures.
> 
> Register nbsd_init_abi() that sets set_gdbarch_auxv_parse().
> 
> Call nbsd_init_abi() from all CPU-specific NetBSD tdep files.
> 
> gdb/ChangeLog:
> 
> 	* nbsd-tdep.c: Include "gdbarch.h".
> 	* nbsd-tdep.c (nbsd_auxv_parse, nbsd_init_abi): New.
> 	* alpha-nbsd-tdep.c (alphanbsd_init_abi): Call nbsd_init_abi().
> 	* amd64-nbsd-tdep.c (amd64nbsd_init_abi): Likewise.
> 	* arm-nbsd-tdep.c (arm_netbsd_elf_init_abi): Likewise.
> 	* hppa-nbsd-tdep.c (hppanbsd_init_abi): Likewise.
> 	* i386-nbsd-tdep.c (i386nbsd_init_abi): Likewise.
> 	* mips-nbsd-tdep.c (nbsd_init_abi): Likewise.
> 	* ppc-nbsd-tdep.c (ppcnbsd_init_abi): Likewise.
> 	* sh-nbsd-tdep.c (shnbsd_init_abi): Likewise.
> 	* sparc-nbsd-tdep.c (sparc32nbsd_init_abi): Likewise.
> 	* sparc64-nbsd-tdep.c (sparc64nbsd_init_abi): Likewise.
> 	* vax-nbsd-tdep.c (vaxnbsd_elf_init_abi): Likewise.
> ---
>  gdb/ChangeLog           | 16 ++++++++++++++++
>  gdb/alpha-nbsd-tdep.c   |  2 ++
>  gdb/amd64-nbsd-tdep.c   |  1 +
>  gdb/arm-nbsd-tdep.c     |  3 +++
>  gdb/hppa-nbsd-tdep.c    |  2 ++
>  gdb/i386-nbsd-tdep.c    |  2 ++
>  gdb/mips-nbsd-tdep.c    |  2 ++
>  gdb/nbsd-tdep.c         | 33 +++++++++++++++++++++++++++++++++
>  gdb/nbsd-tdep.h         |  2 ++
>  gdb/ppc-nbsd-tdep.c     |  2 ++
>  gdb/sh-nbsd-tdep.c      |  1 +
>  gdb/sparc-nbsd-tdep.c   |  2 ++
>  gdb/sparc64-nbsd-tdep.c |  2 ++
>  gdb/vax-nbsd-tdep.c     |  2 ++
>  14 files changed, 72 insertions(+)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index a0d97584189..42b2129d104 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,19 @@
> +2020-03-16  Kamil Rytarowski  <n54@gmx.com>
> +
> +	* nbsd-tdep.c: Include "gdbarch.h".
> +	* nbsd-tdep.c (nbsd_auxv_parse, nbsd_init_abi): New.
> +	* alpha-nbsd-tdep.c (alphanbsd_init_abi): Call nbsd_init_abi().
> +	* amd64-nbsd-tdep.c (amd64nbsd_init_abi): Likewise.
> +	* arm-nbsd-tdep.c (arm_netbsd_elf_init_abi): Likewise.
> +	* hppa-nbsd-tdep.c (hppanbsd_init_abi): Likewise.
> +	* i386-nbsd-tdep.c (i386nbsd_init_abi): Likewise.
> +	* mips-nbsd-tdep.c (nbsd_init_abi): Likewise.
> +	* ppc-nbsd-tdep.c (ppcnbsd_init_abi): Likewise.
> +	* sh-nbsd-tdep.c (shnbsd_init_abi): Likewise.
> +	* sparc-nbsd-tdep.c (sparc32nbsd_init_abi): Likewise.
> +	* sparc64-nbsd-tdep.c (sparc64nbsd_init_abi): Likewise.
> +	* vax-nbsd-tdep.c (vaxnbsd_elf_init_abi): Likewise.
> +
>  2020-03-14  Tom Tromey  <tom@tromey.com>
> 
>  	* c-typeprint.c (cp_type_print_method_args): Print "__restrict__"
> diff --git a/gdb/alpha-nbsd-tdep.c b/gdb/alpha-nbsd-tdep.c
> index ab9240e35da..58294edb3f6 100644
> --- a/gdb/alpha-nbsd-tdep.c
> +++ b/gdb/alpha-nbsd-tdep.c
> @@ -258,6 +258,8 @@ alphanbsd_init_abi (struct gdbarch_info info,
>    /* Hook into the MDEBUG frame unwinder.  */
>    alpha_mdebug_init_abi (info, gdbarch);
> 
> +  nbsd_init_abi (info, gdbarch);
> +
>    /* NetBSD/alpha does not provide single step support via ptrace(2); we
>       must use software single-stepping.  */
>    set_gdbarch_software_single_step (gdbarch, alpha_software_single_step);
> diff --git a/gdb/amd64-nbsd-tdep.c b/gdb/amd64-nbsd-tdep.c
> index 89d07236b85..59071488ed7 100644
> --- a/gdb/amd64-nbsd-tdep.c
> +++ b/gdb/amd64-nbsd-tdep.c
> @@ -106,6 +106,7 @@ amd64nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> 
>    amd64_init_abi (info, gdbarch,
>  		  amd64_target_description (X86_XSTATE_SSE_MASK, true));
> +  nbsd_init_abi (info, gdbarch);
> 
>    tdep->jb_pc_offset = 7 * 8;
> 
> diff --git a/gdb/arm-nbsd-tdep.c b/gdb/arm-nbsd-tdep.c
> index e01df50bc25..a6104f760b3 100644
> --- a/gdb/arm-nbsd-tdep.c
> +++ b/gdb/arm-nbsd-tdep.c
> @@ -150,6 +150,9 @@ arm_netbsd_elf_init_abi (struct gdbarch_info info,
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> 
>    arm_netbsd_init_abi_common (info, gdbarch);
> +
> +  nbsd_init_abi (info, gdbarch);
> +
>    if (tdep->fp_model == ARM_FLOAT_AUTO)
>      tdep->fp_model = ARM_FLOAT_SOFT_VFP;
> 
> diff --git a/gdb/hppa-nbsd-tdep.c b/gdb/hppa-nbsd-tdep.c
> index b532ab1d5cc..d601aa96f3f 100644
> --- a/gdb/hppa-nbsd-tdep.c
> +++ b/gdb/hppa-nbsd-tdep.c
> @@ -201,6 +201,8 @@ hppanbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>    /* Obviously NetBSD is BSD-based.  */
>    hppabsd_init_abi (info, gdbarch);
> 
> +  nbsd_init_abi (info, gdbarch);
> +
>    /* Core file support.  */
>    set_gdbarch_iterate_over_regset_sections
>      (gdbarch, hppanbsd_iterate_over_regset_sections);
> diff --git a/gdb/i386-nbsd-tdep.c b/gdb/i386-nbsd-tdep.c
> index 3157451e08f..f350412d9bd 100644
> --- a/gdb/i386-nbsd-tdep.c
> +++ b/gdb/i386-nbsd-tdep.c
> @@ -377,6 +377,8 @@ i386nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>    /* Obviously NetBSD is BSD-based.  */
>    i386bsd_init_abi (info, gdbarch);
> 
> +  nbsd_init_abi (info, gdbarch);
> +
>    /* NetBSD has a different `struct reg'.  */
>    tdep->gregset_reg_offset = i386nbsd_r_reg_offset;
>    tdep->gregset_num_regs = ARRAY_SIZE (i386nbsd_r_reg_offset);
> diff --git a/gdb/mips-nbsd-tdep.c b/gdb/mips-nbsd-tdep.c
> index 38bc7ce636b..6f4d22b24fb 100644
> --- a/gdb/mips-nbsd-tdep.c
> +++ b/gdb/mips-nbsd-tdep.c
> @@ -354,6 +354,8 @@ static void
>  mipsnbsd_init_abi (struct gdbarch_info info,
>                     struct gdbarch *gdbarch)
>  {
> +  nbsd_init_abi (info, gdbarch)
> +
>    set_gdbarch_iterate_over_regset_sections
>      (gdbarch, mipsnbsd_iterate_over_regset_sections);
> 
> diff --git a/gdb/nbsd-tdep.c b/gdb/nbsd-tdep.c
> index 49bb2b706bd..a9207e16625 100644
> --- a/gdb/nbsd-tdep.c
> +++ b/gdb/nbsd-tdep.c
> @@ -22,6 +22,7 @@
>  #include "defs.h"
>  #include "solib-svr4.h"
>  #include "nbsd-tdep.h"
> +#include "gdbarch.h"
> 
>  /* FIXME: kettenis/20060115: We should really eliminate the next two
>     functions completely.  */
> @@ -47,3 +48,35 @@ nbsd_pc_in_sigtramp (CORE_ADDR pc, const char *func_name)
>    return (func_name != NULL
>  	  && startswith (func_name, "__sigtramp"));
>  }
> +
> +static int
> +nbsd_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
> +nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> +{
> +  set_gdbarch_auxv_parse (gdbarch, nbsd_auxv_parse);
> +}
> diff --git a/gdb/nbsd-tdep.h b/gdb/nbsd-tdep.h
> index c99a8b537b6..9301ff7a69a 100644
> --- a/gdb/nbsd-tdep.h
> +++ b/gdb/nbsd-tdep.h
> @@ -25,4 +25,6 @@ struct link_map_offsets *nbsd_lp64_solib_svr4_fetch_link_map_offsets (void);
> 
>  int nbsd_pc_in_sigtramp (CORE_ADDR, const char *);
> 
> +void nbsd_init_abi (struct gdbarch_info, struct gdbarch *);
> +
>  #endif /* NBSD_TDEP_H */
> diff --git a/gdb/ppc-nbsd-tdep.c b/gdb/ppc-nbsd-tdep.c
> index d75930c9f8d..81492deaccd 100644
> --- a/gdb/ppc-nbsd-tdep.c
> +++ b/gdb/ppc-nbsd-tdep.c
> @@ -173,6 +173,8 @@ static void
>  ppcnbsd_init_abi (struct gdbarch_info info,
>                    struct gdbarch *gdbarch)
>  {
> +  nbsd_init_abi (info, gdbarch);
> +
>    /* For NetBSD, this is an on again, off again thing.  Some systems
>       do use the broken struct convention, and some don't.  */
>    set_gdbarch_return_value (gdbarch, ppcnbsd_return_value);
> diff --git a/gdb/sh-nbsd-tdep.c b/gdb/sh-nbsd-tdep.c
> index aa319261684..2b2a7e3fd4a 100644
> --- a/gdb/sh-nbsd-tdep.c
> +++ b/gdb/sh-nbsd-tdep.c
> @@ -63,6 +63,7 @@ shnbsd_init_abi (struct gdbarch_info info,
>                    struct gdbarch *gdbarch)
>  {
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +  nbsd_init_abi (info, gdbarch);
> 
>    tdep->core_gregmap = (struct sh_corefile_regmap *)regmap;
>    tdep->sizeof_gregset = 84;
> diff --git a/gdb/sparc-nbsd-tdep.c b/gdb/sparc-nbsd-tdep.c
> index 7aba6020d27..ab1b557c57c 100644
> --- a/gdb/sparc-nbsd-tdep.c
> +++ b/gdb/sparc-nbsd-tdep.c
> @@ -296,6 +296,8 @@ sparc32nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> 
> +  nbsd_init_abi (info, gdbarch);
> +
>    /* NetBSD doesn't support the 128-bit `long double' from the psABI.  */
>    set_gdbarch_long_double_bit (gdbarch, 64);
>    set_gdbarch_long_double_format (gdbarch, floatformats_ieee_double);
> diff --git a/gdb/sparc64-nbsd-tdep.c b/gdb/sparc64-nbsd-tdep.c
> index cd5bfe89410..dac7fa78b9b 100644
> --- a/gdb/sparc64-nbsd-tdep.c
> +++ b/gdb/sparc64-nbsd-tdep.c
> @@ -249,6 +249,8 @@ sparc64nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> 
> +  nbsd_init_abi (info, gdbarch);
> +
>    tdep->gregset = &sparc64nbsd_gregset;
>    tdep->sizeof_gregset = 160;
> 
> diff --git a/gdb/vax-nbsd-tdep.c b/gdb/vax-nbsd-tdep.c
> index c2c08cc1603..7630ac5ab94 100644
> --- a/gdb/vax-nbsd-tdep.c
> +++ b/gdb/vax-nbsd-tdep.c
> @@ -29,6 +29,8 @@
>  static void
>  vaxnbsd_elf_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
> +  nbsd_init_abi (info, gdbarch);
> +
>    /* NetBSD ELF uses SVR4-style shared libraries.  */
>    set_solib_svr4_fetch_link_map_offsets
>      (gdbarch, svr4_ilp32_fetch_link_map_offsets);
> --
> 2.25.0
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] Add support for "info auxv" on NetBSD

[Tom Tromey]

>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:

Kamil> Register nbsd_auxv_parse() that overloads the default (Linux-style)
Kamil> AUXV parsing. On NetBSD the type parameter is defined as int32_t
Kamil> for all architectures.

Thanks for the patch.

Kamil> +
Kamil> +static int
Kamil> +nbsd_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
Kamil> +		 gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)

In gdb a new function should always have an intro comment.  This one
could just explain which gdbarch callback it implements.

Kamil> +{
Kamil> +  struct type *int_type = builtin_type (gdbarch)->builtin_int;

Presumably you want builtin_in32 here?  builtin_int is
architecture-dependent.  Or you could just hard-code the size, with a
suitable comment.

Kamil> +
Kamil> +void
Kamil> +nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)

The intro comment here should probably say "See nbsd-tdep.h."...

Kamil> +++ b/gdb/nbsd-tdep.h
Kamil> @@ -25,4 +25,6 @@ struct link_map_offsets *nbsd_lp64_solib_svr4_fetch_link_map_offsets (void);

Kamil>  int nbsd_pc_in_sigtramp (CORE_ADDR, const char *);

Kamil> +void nbsd_init_abi (struct gdbarch_info, struct gdbarch *);

... and then there should be a descriptive comment above this line.

thanks,
Tom

Re: [PATCH v2] Add support for "info auxv" on NetBSD

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 1785 bytes --]

On 20.03.2020 16:43, Tom Tromey wrote:
>>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:
> 
> Kamil> Register nbsd_auxv_parse() that overloads the default (Linux-style)
> Kamil> AUXV parsing. On NetBSD the type parameter is defined as int32_t
> Kamil> for all architectures.
> 
> Thanks for the patch.
> 
> Kamil> +
> Kamil> +static int
> Kamil> +nbsd_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
> Kamil> +		 gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
> 
> In gdb a new function should always have an intro comment.  This one
> could just explain which gdbarch callback it implements.
> 

Done.

> Kamil> +{
> Kamil> +  struct type *int_type = builtin_type (gdbarch)->builtin_int;
> 
> Presumably you want builtin_in32 here?  builtin_int is
> architecture-dependent.  Or you could just hard-code the size, with a
> suitable comment.
> 

int is de facto alias for int32 on NetBSD.

It cannot be shorter (there are assumptions that it is at least 32-bit)
and a lot of things will break for making it longer. So it will likely
stay that way forever.

I prefer to keep it as 'int' to match the preexisting OpenBSD code.

> Kamil> +
> Kamil> +void
> Kamil> +nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> 
> The intro comment here should probably say "See nbsd-tdep.h."...
> 

Done.

> Kamil> +++ b/gdb/nbsd-tdep.h
> Kamil> @@ -25,4 +25,6 @@ struct link_map_offsets *nbsd_lp64_solib_svr4_fetch_link_map_offsets (void);
> 
> Kamil>  int nbsd_pc_in_sigtramp (CORE_ADDR, const char *);
> 
> Kamil> +void nbsd_init_abi (struct gdbarch_info, struct gdbarch *);
> 
> ... and then there should be a descriptive comment above this line.
> 

Done.

> thanks,
> Tom
> 

Please see v3.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3] Add support for "info auxv" on NetBSD

[Kamil Rytarowski]

Register nbsd_auxv_parse() that overloads the default (Linux-style)
AUXV parsing. On NetBSD the type parameter is defined as int32_t
for all architectures.

Register nbsd_init_abi() that sets set_gdbarch_auxv_parse().

Call nbsd_init_abi() from all CPU-specific NetBSD tdep files.

gdb/ChangeLog:

	* nbsd-tdep.c: Include "gdbarch.h".
	* nbsd-tdep.c (nbsd_auxv_parse, nbsd_init_abi): New.
	* alpha-nbsd-tdep.c (alphanbsd_init_abi): Call nbsd_init_abi().
	* amd64-nbsd-tdep.c (amd64nbsd_init_abi): Likewise.
	* arm-nbsd-tdep.c (arm_netbsd_elf_init_abi): Likewise.
	* hppa-nbsd-tdep.c (hppanbsd_init_abi): Likewise.
	* i386-nbsd-tdep.c (i386nbsd_init_abi): Likewise.
	* mips-nbsd-tdep.c (nbsd_init_abi): Likewise.
	* ppc-nbsd-tdep.c (ppcnbsd_init_abi): Likewise.
	* sh-nbsd-tdep.c (shnbsd_init_abi): Likewise.
	* sparc-nbsd-tdep.c (sparc32nbsd_init_abi): Likewise.
	* sparc64-nbsd-tdep.c (sparc64nbsd_init_abi): Likewise.
	* vax-nbsd-tdep.c (vaxnbsd_elf_init_abi): Likewise.
---
 gdb/ChangeLog           | 16 ++++++++++++++++
 gdb/alpha-nbsd-tdep.c   |  2 ++
 gdb/amd64-nbsd-tdep.c   |  1 +
 gdb/arm-nbsd-tdep.c     |  3 +++
 gdb/hppa-nbsd-tdep.c    |  2 ++
 gdb/i386-nbsd-tdep.c    |  2 ++
 gdb/mips-nbsd-tdep.c    |  2 ++
 gdb/nbsd-tdep.c         | 38 ++++++++++++++++++++++++++++++++++++++
 gdb/nbsd-tdep.h         |  4 ++++
 gdb/ppc-nbsd-tdep.c     |  2 ++
 gdb/sh-nbsd-tdep.c      |  1 +
 gdb/sparc-nbsd-tdep.c   |  2 ++
 gdb/sparc64-nbsd-tdep.c |  2 ++
 gdb/vax-nbsd-tdep.c     |  2 ++
 14 files changed, 79 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 650b74bae4a..572403001ce 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,19 @@
+2020-03-20  Kamil Rytarowski  <n54@gmx.com>
+
+	* nbsd-tdep.c: Include "gdbarch.h".
+	* nbsd-tdep.c (nbsd_auxv_parse, nbsd_init_abi): New.
+	* alpha-nbsd-tdep.c (alphanbsd_init_abi): Call nbsd_init_abi().
+	* amd64-nbsd-tdep.c (amd64nbsd_init_abi): Likewise.
+	* arm-nbsd-tdep.c (arm_netbsd_elf_init_abi): Likewise.
+	* hppa-nbsd-tdep.c (hppanbsd_init_abi): Likewise.
+	* i386-nbsd-tdep.c (i386nbsd_init_abi): Likewise.
+	* mips-nbsd-tdep.c (nbsd_init_abi): Likewise.
+	* ppc-nbsd-tdep.c (ppcnbsd_init_abi): Likewise.
+	* sh-nbsd-tdep.c (shnbsd_init_abi): Likewise.
+	* sparc-nbsd-tdep.c (sparc32nbsd_init_abi): Likewise.
+	* sparc64-nbsd-tdep.c (sparc64nbsd_init_abi): Likewise.
+	* vax-nbsd-tdep.c (vaxnbsd_elf_init_abi): Likewise.
+
 2020-03-20  Kamil Rytarowski  <n54@gmx.com>

 	* amd64-bsd-nat.c (gdb_ptrace): Change return type from `int' to
diff --git a/gdb/alpha-nbsd-tdep.c b/gdb/alpha-nbsd-tdep.c
index ab9240e35da..58294edb3f6 100644
--- a/gdb/alpha-nbsd-tdep.c
+++ b/gdb/alpha-nbsd-tdep.c
@@ -258,6 +258,8 @@ alphanbsd_init_abi (struct gdbarch_info info,
   /* Hook into the MDEBUG frame unwinder.  */
   alpha_mdebug_init_abi (info, gdbarch);

+  nbsd_init_abi (info, gdbarch);
+
   /* NetBSD/alpha does not provide single step support via ptrace(2); we
      must use software single-stepping.  */
   set_gdbarch_software_single_step (gdbarch, alpha_software_single_step);
diff --git a/gdb/amd64-nbsd-tdep.c b/gdb/amd64-nbsd-tdep.c
index 89d07236b85..59071488ed7 100644
--- a/gdb/amd64-nbsd-tdep.c
+++ b/gdb/amd64-nbsd-tdep.c
@@ -106,6 +106,7 @@ amd64nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)

   amd64_init_abi (info, gdbarch,
 		  amd64_target_description (X86_XSTATE_SSE_MASK, true));
+  nbsd_init_abi (info, gdbarch);

   tdep->jb_pc_offset = 7 * 8;

diff --git a/gdb/arm-nbsd-tdep.c b/gdb/arm-nbsd-tdep.c
index e01df50bc25..a6104f760b3 100644
--- a/gdb/arm-nbsd-tdep.c
+++ b/gdb/arm-nbsd-tdep.c
@@ -150,6 +150,9 @@ arm_netbsd_elf_init_abi (struct gdbarch_info info,
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

   arm_netbsd_init_abi_common (info, gdbarch);
+
+  nbsd_init_abi (info, gdbarch);
+
   if (tdep->fp_model == ARM_FLOAT_AUTO)
     tdep->fp_model = ARM_FLOAT_SOFT_VFP;

diff --git a/gdb/hppa-nbsd-tdep.c b/gdb/hppa-nbsd-tdep.c
index b532ab1d5cc..d601aa96f3f 100644
--- a/gdb/hppa-nbsd-tdep.c
+++ b/gdb/hppa-nbsd-tdep.c
@@ -201,6 +201,8 @@ hppanbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   /* Obviously NetBSD is BSD-based.  */
   hppabsd_init_abi (info, gdbarch);

+  nbsd_init_abi (info, gdbarch);
+
   /* Core file support.  */
   set_gdbarch_iterate_over_regset_sections
     (gdbarch, hppanbsd_iterate_over_regset_sections);
diff --git a/gdb/i386-nbsd-tdep.c b/gdb/i386-nbsd-tdep.c
index 3157451e08f..f350412d9bd 100644
--- a/gdb/i386-nbsd-tdep.c
+++ b/gdb/i386-nbsd-tdep.c
@@ -377,6 +377,8 @@ i386nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   /* Obviously NetBSD is BSD-based.  */
   i386bsd_init_abi (info, gdbarch);

+  nbsd_init_abi (info, gdbarch);
+
   /* NetBSD has a different `struct reg'.  */
   tdep->gregset_reg_offset = i386nbsd_r_reg_offset;
   tdep->gregset_num_regs = ARRAY_SIZE (i386nbsd_r_reg_offset);
diff --git a/gdb/mips-nbsd-tdep.c b/gdb/mips-nbsd-tdep.c
index 38bc7ce636b..6f4d22b24fb 100644
--- a/gdb/mips-nbsd-tdep.c
+++ b/gdb/mips-nbsd-tdep.c
@@ -354,6 +354,8 @@ static void
 mipsnbsd_init_abi (struct gdbarch_info info,
                    struct gdbarch *gdbarch)
 {
+  nbsd_init_abi (info, gdbarch)
+
   set_gdbarch_iterate_over_regset_sections
     (gdbarch, mipsnbsd_iterate_over_regset_sections);

diff --git a/gdb/nbsd-tdep.c b/gdb/nbsd-tdep.c
index 49bb2b706bd..5d5ea26622e 100644
--- a/gdb/nbsd-tdep.c
+++ b/gdb/nbsd-tdep.c
@@ -22,6 +22,7 @@
 #include "defs.h"
 #include "solib-svr4.h"
 #include "nbsd-tdep.h"
+#include "gdbarch.h"

 /* FIXME: kettenis/20060115: We should really eliminate the next two
    functions completely.  */
@@ -47,3 +48,40 @@ nbsd_pc_in_sigtramp (CORE_ADDR pc, const char *func_name)
   return (func_name != NULL
 	  && startswith (func_name, "__sigtramp"));
 }
+
+/* NetBSD-specific parser for AUXV data with. NetBSD follows the ELF
+   specification, contrary to some other ELF Operating Systems.  */
+
+static int
+nbsd_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;
+}
+
+/* See nbsd-tdep.h.  */
+
+void
+nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  set_gdbarch_auxv_parse (gdbarch, nbsd_auxv_parse);
+}
diff --git a/gdb/nbsd-tdep.h b/gdb/nbsd-tdep.h
index c99a8b537b6..4b06c13f87b 100644
--- a/gdb/nbsd-tdep.h
+++ b/gdb/nbsd-tdep.h
@@ -25,4 +25,8 @@ struct link_map_offsets *nbsd_lp64_solib_svr4_fetch_link_map_offsets (void);

 int nbsd_pc_in_sigtramp (CORE_ADDR, const char *);

+/* NetBSD specific set of ABI-related routines.  */
+
+void nbsd_init_abi (struct gdbarch_info, struct gdbarch *);
+
 #endif /* NBSD_TDEP_H */
diff --git a/gdb/ppc-nbsd-tdep.c b/gdb/ppc-nbsd-tdep.c
index d75930c9f8d..81492deaccd 100644
--- a/gdb/ppc-nbsd-tdep.c
+++ b/gdb/ppc-nbsd-tdep.c
@@ -173,6 +173,8 @@ static void
 ppcnbsd_init_abi (struct gdbarch_info info,
                   struct gdbarch *gdbarch)
 {
+  nbsd_init_abi (info, gdbarch);
+
   /* For NetBSD, this is an on again, off again thing.  Some systems
      do use the broken struct convention, and some don't.  */
   set_gdbarch_return_value (gdbarch, ppcnbsd_return_value);
diff --git a/gdb/sh-nbsd-tdep.c b/gdb/sh-nbsd-tdep.c
index aa319261684..2b2a7e3fd4a 100644
--- a/gdb/sh-nbsd-tdep.c
+++ b/gdb/sh-nbsd-tdep.c
@@ -63,6 +63,7 @@ shnbsd_init_abi (struct gdbarch_info info,
                   struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  nbsd_init_abi (info, gdbarch);

   tdep->core_gregmap = (struct sh_corefile_regmap *)regmap;
   tdep->sizeof_gregset = 84;
diff --git a/gdb/sparc-nbsd-tdep.c b/gdb/sparc-nbsd-tdep.c
index 7aba6020d27..ab1b557c57c 100644
--- a/gdb/sparc-nbsd-tdep.c
+++ b/gdb/sparc-nbsd-tdep.c
@@ -296,6 +296,8 @@ sparc32nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

+  nbsd_init_abi (info, gdbarch);
+
   /* NetBSD doesn't support the 128-bit `long double' from the psABI.  */
   set_gdbarch_long_double_bit (gdbarch, 64);
   set_gdbarch_long_double_format (gdbarch, floatformats_ieee_double);
diff --git a/gdb/sparc64-nbsd-tdep.c b/gdb/sparc64-nbsd-tdep.c
index cd5bfe89410..dac7fa78b9b 100644
--- a/gdb/sparc64-nbsd-tdep.c
+++ b/gdb/sparc64-nbsd-tdep.c
@@ -249,6 +249,8 @@ sparc64nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

+  nbsd_init_abi (info, gdbarch);
+
   tdep->gregset = &sparc64nbsd_gregset;
   tdep->sizeof_gregset = 160;

diff --git a/gdb/vax-nbsd-tdep.c b/gdb/vax-nbsd-tdep.c
index c2c08cc1603..7630ac5ab94 100644
--- a/gdb/vax-nbsd-tdep.c
+++ b/gdb/vax-nbsd-tdep.c
@@ -29,6 +29,8 @@
 static void
 vaxnbsd_elf_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
+  nbsd_init_abi (info, gdbarch);
+
   /* NetBSD ELF uses SVR4-style shared libraries.  */
   set_solib_svr4_fetch_link_map_offsets
     (gdbarch, svr4_ilp32_fetch_link_map_offsets);
--
2.25.0

Re: [PATCH v3] Add support for "info auxv" on NetBSD

[Kamil Rytarowski]

Ping?

On 20.03.2020 18:27, Kamil Rytarowski wrote:
> Register nbsd_auxv_parse() that overloads the default (Linux-style)
> AUXV parsing. On NetBSD the type parameter is defined as int32_t
> for all architectures.
>
> Register nbsd_init_abi() that sets set_gdbarch_auxv_parse().
>
> Call nbsd_init_abi() from all CPU-specific NetBSD tdep files.
>
> gdb/ChangeLog:
>
> 	* nbsd-tdep.c: Include "gdbarch.h".
> 	* nbsd-tdep.c (nbsd_auxv_parse, nbsd_init_abi): New.
> 	* alpha-nbsd-tdep.c (alphanbsd_init_abi): Call nbsd_init_abi().
> 	* amd64-nbsd-tdep.c (amd64nbsd_init_abi): Likewise.
> 	* arm-nbsd-tdep.c (arm_netbsd_elf_init_abi): Likewise.
> 	* hppa-nbsd-tdep.c (hppanbsd_init_abi): Likewise.
> 	* i386-nbsd-tdep.c (i386nbsd_init_abi): Likewise.
> 	* mips-nbsd-tdep.c (nbsd_init_abi): Likewise.
> 	* ppc-nbsd-tdep.c (ppcnbsd_init_abi): Likewise.
> 	* sh-nbsd-tdep.c (shnbsd_init_abi): Likewise.
> 	* sparc-nbsd-tdep.c (sparc32nbsd_init_abi): Likewise.
> 	* sparc64-nbsd-tdep.c (sparc64nbsd_init_abi): Likewise.
> 	* vax-nbsd-tdep.c (vaxnbsd_elf_init_abi): Likewise.
> ---
>  gdb/ChangeLog           | 16 ++++++++++++++++
>  gdb/alpha-nbsd-tdep.c   |  2 ++
>  gdb/amd64-nbsd-tdep.c   |  1 +
>  gdb/arm-nbsd-tdep.c     |  3 +++
>  gdb/hppa-nbsd-tdep.c    |  2 ++
>  gdb/i386-nbsd-tdep.c    |  2 ++
>  gdb/mips-nbsd-tdep.c    |  2 ++
>  gdb/nbsd-tdep.c         | 38 ++++++++++++++++++++++++++++++++++++++
>  gdb/nbsd-tdep.h         |  4 ++++
>  gdb/ppc-nbsd-tdep.c     |  2 ++
>  gdb/sh-nbsd-tdep.c      |  1 +
>  gdb/sparc-nbsd-tdep.c   |  2 ++
>  gdb/sparc64-nbsd-tdep.c |  2 ++
>  gdb/vax-nbsd-tdep.c     |  2 ++
>  14 files changed, 79 insertions(+)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 650b74bae4a..572403001ce 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,19 @@
> +2020-03-20  Kamil Rytarowski  <n54@gmx.com>
> +
> +	* nbsd-tdep.c: Include "gdbarch.h".
> +	* nbsd-tdep.c (nbsd_auxv_parse, nbsd_init_abi): New.
> +	* alpha-nbsd-tdep.c (alphanbsd_init_abi): Call nbsd_init_abi().
> +	* amd64-nbsd-tdep.c (amd64nbsd_init_abi): Likewise.
> +	* arm-nbsd-tdep.c (arm_netbsd_elf_init_abi): Likewise.
> +	* hppa-nbsd-tdep.c (hppanbsd_init_abi): Likewise.
> +	* i386-nbsd-tdep.c (i386nbsd_init_abi): Likewise.
> +	* mips-nbsd-tdep.c (nbsd_init_abi): Likewise.
> +	* ppc-nbsd-tdep.c (ppcnbsd_init_abi): Likewise.
> +	* sh-nbsd-tdep.c (shnbsd_init_abi): Likewise.
> +	* sparc-nbsd-tdep.c (sparc32nbsd_init_abi): Likewise.
> +	* sparc64-nbsd-tdep.c (sparc64nbsd_init_abi): Likewise.
> +	* vax-nbsd-tdep.c (vaxnbsd_elf_init_abi): Likewise.
> +
>  2020-03-20  Kamil Rytarowski  <n54@gmx.com>
>
>  	* amd64-bsd-nat.c (gdb_ptrace): Change return type from `int' to
> diff --git a/gdb/alpha-nbsd-tdep.c b/gdb/alpha-nbsd-tdep.c
> index ab9240e35da..58294edb3f6 100644
> --- a/gdb/alpha-nbsd-tdep.c
> +++ b/gdb/alpha-nbsd-tdep.c
> @@ -258,6 +258,8 @@ alphanbsd_init_abi (struct gdbarch_info info,
>    /* Hook into the MDEBUG frame unwinder.  */
>    alpha_mdebug_init_abi (info, gdbarch);
>
> +  nbsd_init_abi (info, gdbarch);
> +
>    /* NetBSD/alpha does not provide single step support via ptrace(2); we
>       must use software single-stepping.  */
>    set_gdbarch_software_single_step (gdbarch, alpha_software_single_step);
> diff --git a/gdb/amd64-nbsd-tdep.c b/gdb/amd64-nbsd-tdep.c
> index 89d07236b85..59071488ed7 100644
> --- a/gdb/amd64-nbsd-tdep.c
> +++ b/gdb/amd64-nbsd-tdep.c
> @@ -106,6 +106,7 @@ amd64nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>
>    amd64_init_abi (info, gdbarch,
>  		  amd64_target_description (X86_XSTATE_SSE_MASK, true));
> +  nbsd_init_abi (info, gdbarch);
>
>    tdep->jb_pc_offset = 7 * 8;
>
> diff --git a/gdb/arm-nbsd-tdep.c b/gdb/arm-nbsd-tdep.c
> index e01df50bc25..a6104f760b3 100644
> --- a/gdb/arm-nbsd-tdep.c
> +++ b/gdb/arm-nbsd-tdep.c
> @@ -150,6 +150,9 @@ arm_netbsd_elf_init_abi (struct gdbarch_info info,
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>
>    arm_netbsd_init_abi_common (info, gdbarch);
> +
> +  nbsd_init_abi (info, gdbarch);
> +
>    if (tdep->fp_model == ARM_FLOAT_AUTO)
>      tdep->fp_model = ARM_FLOAT_SOFT_VFP;
>
> diff --git a/gdb/hppa-nbsd-tdep.c b/gdb/hppa-nbsd-tdep.c
> index b532ab1d5cc..d601aa96f3f 100644
> --- a/gdb/hppa-nbsd-tdep.c
> +++ b/gdb/hppa-nbsd-tdep.c
> @@ -201,6 +201,8 @@ hppanbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>    /* Obviously NetBSD is BSD-based.  */
>    hppabsd_init_abi (info, gdbarch);
>
> +  nbsd_init_abi (info, gdbarch);
> +
>    /* Core file support.  */
>    set_gdbarch_iterate_over_regset_sections
>      (gdbarch, hppanbsd_iterate_over_regset_sections);
> diff --git a/gdb/i386-nbsd-tdep.c b/gdb/i386-nbsd-tdep.c
> index 3157451e08f..f350412d9bd 100644
> --- a/gdb/i386-nbsd-tdep.c
> +++ b/gdb/i386-nbsd-tdep.c
> @@ -377,6 +377,8 @@ i386nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>    /* Obviously NetBSD is BSD-based.  */
>    i386bsd_init_abi (info, gdbarch);
>
> +  nbsd_init_abi (info, gdbarch);
> +
>    /* NetBSD has a different `struct reg'.  */
>    tdep->gregset_reg_offset = i386nbsd_r_reg_offset;
>    tdep->gregset_num_regs = ARRAY_SIZE (i386nbsd_r_reg_offset);
> diff --git a/gdb/mips-nbsd-tdep.c b/gdb/mips-nbsd-tdep.c
> index 38bc7ce636b..6f4d22b24fb 100644
> --- a/gdb/mips-nbsd-tdep.c
> +++ b/gdb/mips-nbsd-tdep.c
> @@ -354,6 +354,8 @@ static void
>  mipsnbsd_init_abi (struct gdbarch_info info,
>                     struct gdbarch *gdbarch)
>  {
> +  nbsd_init_abi (info, gdbarch)
> +
>    set_gdbarch_iterate_over_regset_sections
>      (gdbarch, mipsnbsd_iterate_over_regset_sections);
>
> diff --git a/gdb/nbsd-tdep.c b/gdb/nbsd-tdep.c
> index 49bb2b706bd..5d5ea26622e 100644
> --- a/gdb/nbsd-tdep.c
> +++ b/gdb/nbsd-tdep.c
> @@ -22,6 +22,7 @@
>  #include "defs.h"
>  #include "solib-svr4.h"
>  #include "nbsd-tdep.h"
> +#include "gdbarch.h"
>
>  /* FIXME: kettenis/20060115: We should really eliminate the next two
>     functions completely.  */
> @@ -47,3 +48,40 @@ nbsd_pc_in_sigtramp (CORE_ADDR pc, const char *func_name)
>    return (func_name != NULL
>  	  && startswith (func_name, "__sigtramp"));
>  }
> +
> +/* NetBSD-specific parser for AUXV data with. NetBSD follows the ELF
> +   specification, contrary to some other ELF Operating Systems.  */
> +
> +static int
> +nbsd_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;
> +}
> +
> +/* See nbsd-tdep.h.  */
> +
> +void
> +nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> +{
> +  set_gdbarch_auxv_parse (gdbarch, nbsd_auxv_parse);
> +}
> diff --git a/gdb/nbsd-tdep.h b/gdb/nbsd-tdep.h
> index c99a8b537b6..4b06c13f87b 100644
> --- a/gdb/nbsd-tdep.h
> +++ b/gdb/nbsd-tdep.h
> @@ -25,4 +25,8 @@ struct link_map_offsets *nbsd_lp64_solib_svr4_fetch_link_map_offsets (void);
>
>  int nbsd_pc_in_sigtramp (CORE_ADDR, const char *);
>
> +/* NetBSD specific set of ABI-related routines.  */
> +
> +void nbsd_init_abi (struct gdbarch_info, struct gdbarch *);
> +
>  #endif /* NBSD_TDEP_H */
> diff --git a/gdb/ppc-nbsd-tdep.c b/gdb/ppc-nbsd-tdep.c
> index d75930c9f8d..81492deaccd 100644
> --- a/gdb/ppc-nbsd-tdep.c
> +++ b/gdb/ppc-nbsd-tdep.c
> @@ -173,6 +173,8 @@ static void
>  ppcnbsd_init_abi (struct gdbarch_info info,
>                    struct gdbarch *gdbarch)
>  {
> +  nbsd_init_abi (info, gdbarch);
> +
>    /* For NetBSD, this is an on again, off again thing.  Some systems
>       do use the broken struct convention, and some don't.  */
>    set_gdbarch_return_value (gdbarch, ppcnbsd_return_value);
> diff --git a/gdb/sh-nbsd-tdep.c b/gdb/sh-nbsd-tdep.c
> index aa319261684..2b2a7e3fd4a 100644
> --- a/gdb/sh-nbsd-tdep.c
> +++ b/gdb/sh-nbsd-tdep.c
> @@ -63,6 +63,7 @@ shnbsd_init_abi (struct gdbarch_info info,
>                    struct gdbarch *gdbarch)
>  {
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +  nbsd_init_abi (info, gdbarch);
>
>    tdep->core_gregmap = (struct sh_corefile_regmap *)regmap;
>    tdep->sizeof_gregset = 84;
> diff --git a/gdb/sparc-nbsd-tdep.c b/gdb/sparc-nbsd-tdep.c
> index 7aba6020d27..ab1b557c57c 100644
> --- a/gdb/sparc-nbsd-tdep.c
> +++ b/gdb/sparc-nbsd-tdep.c
> @@ -296,6 +296,8 @@ sparc32nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>
> +  nbsd_init_abi (info, gdbarch);
> +
>    /* NetBSD doesn't support the 128-bit `long double' from the psABI.  */
>    set_gdbarch_long_double_bit (gdbarch, 64);
>    set_gdbarch_long_double_format (gdbarch, floatformats_ieee_double);
> diff --git a/gdb/sparc64-nbsd-tdep.c b/gdb/sparc64-nbsd-tdep.c
> index cd5bfe89410..dac7fa78b9b 100644
> --- a/gdb/sparc64-nbsd-tdep.c
> +++ b/gdb/sparc64-nbsd-tdep.c
> @@ -249,6 +249,8 @@ sparc64nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>
> +  nbsd_init_abi (info, gdbarch);
> +
>    tdep->gregset = &sparc64nbsd_gregset;
>    tdep->sizeof_gregset = 160;
>
> diff --git a/gdb/vax-nbsd-tdep.c b/gdb/vax-nbsd-tdep.c
> index c2c08cc1603..7630ac5ab94 100644
> --- a/gdb/vax-nbsd-tdep.c
> +++ b/gdb/vax-nbsd-tdep.c
> @@ -29,6 +29,8 @@
>  static void
>  vaxnbsd_elf_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
> +  nbsd_init_abi (info, gdbarch);
> +
>    /* NetBSD ELF uses SVR4-style shared libraries.  */
>    set_solib_svr4_fetch_link_map_offsets
>      (gdbarch, svr4_ilp32_fetch_link_map_offsets);
> --
> 2.25.0
>

Re: [PATCH v3] Add support for "info auxv" on NetBSD

[John Baldwin]

On 3/26/20 4:26 PM, Kamil Rytarowski wrote:
> Ping?
> 
> On 20.03.2020 18:27, Kamil Rytarowski wrote:
>> Register nbsd_auxv_parse() that overloads the default (Linux-style)
>> AUXV parsing. On NetBSD the type parameter is defined as int32_t
>> for all architectures.

I would tone down some of the rhetoric.  FreeBSD uses the default AUXV
parsing, and I think Solaris does as well, so describing it as Linux-only
isn't very accurate.

(Similarly, I think the earlier reviews I saw around ptrace() claimed that
NetBSD was the only OS to use LWPs with ptrace() in the log messages in
effect which isn't really true as both Solaris and FreeBSD use LWPs
happily with ptrace(), just using a different convention.)

>>  }
>> +
>> +/* NetBSD-specific parser for AUXV data with. NetBSD follows the ELF
>> +   specification, contrary to some other ELF Operating Systems.  */

I would also tone this down a bit, and at least reference the correct
specification.  The ELF spec doesn't define the layout of auxv_t.  The
per-architecture psABI documents do.  Also, just saying that you follow
the spec doesn't help.  I would suggest something like:

/* NetBSD-specific parser for AUXV entries.  NetBSD always uses an int
   to store the type as defined in the SVR4 psABI specifications rather
   than long as assumed by the default parser.  */

-- 
John Baldwin

Re: [PATCH v3] Add support for "info auxv" on NetBSD

[Kamil Rytarowski]

On 27.03.2020 17:31, John Baldwin wrote:
> On 3/26/20 4:26 PM, Kamil Rytarowski wrote:
>> Ping?
>>
>> On 20.03.2020 18:27, Kamil Rytarowski wrote:
>>> Register nbsd_auxv_parse() that overloads the default (Linux-style)
>>> AUXV parsing. On NetBSD the type parameter is defined as int32_t
>>> for all architectures.
>
> I would tone down some of the rhetoric.  FreeBSD uses the default AUXV
> parsing, and I think Solaris does as well, so describing it as Linux-only
> isn't very accurate.
>

It is phrases as Linux-style, not Linux-only.

Linux-style is considered as the default one and other OSs have to tune it.

> (Similarly, I think the earlier reviews I saw around ptrace() claimed that
> NetBSD was the only OS to use LWPs with ptrace() in the log messages in
> effect which isn't really true as both Solaris and FreeBSD use LWPs
> happily with ptrace(), just using a different convention.)
>

The pair of (PID+LWP) for getters and setters of registers (among
certain other ptrace operations) is NetBSD style ptrace(2) API design.
This leads to the point that NetBSD is currently the only OS where
get_ptrace_pid() is not compatible (at leat in the current form).

>>>  }
>>> +
>>> +/* NetBSD-specific parser for AUXV data with. NetBSD follows the ELF
>>> +   specification, contrary to some other ELF Operating Systems.  */
>
> I would also tone this down a bit, and at least reference the correct
> specification.  The ELF spec doesn't define the layout of auxv_t.  The
> per-architecture psABI documents do.  Also, just saying that you follow
> the spec doesn't help.  I would suggest something like:
>
> /* NetBSD-specific parser for AUXV entries.  NetBSD always uses an int
>    to store the type as defined in the SVR4 psABI specifications rather
>    than long as assumed by the default parser.  */
>

This is toned down compared to obsd-tdet.c, that says:

/* Unlike Linux, OpenBSD actually follows the ELF standard.  */

OK, is this patch fine after rephrasing the above texts?

Re: [PATCH v3] Add support for "info auxv" on NetBSD

[John Baldwin]

On 3/27/20 10:04 AM, Kamil Rytarowski wrote:
> On 27.03.2020 17:31, John Baldwin wrote:
>> On 3/26/20 4:26 PM, Kamil Rytarowski wrote:
>>> Ping?
>>>
>>> On 20.03.2020 18:27, Kamil Rytarowski wrote:
>>>> Register nbsd_auxv_parse() that overloads the default (Linux-style)
>>>> AUXV parsing. On NetBSD the type parameter is defined as int32_t
>>>> for all architectures.
>>
>> I would tone down some of the rhetoric.  FreeBSD uses the default AUXV
>> parsing, and I think Solaris does as well, so describing it as Linux-only
>> isn't very accurate.
>>
> 
> It is phrases as Linux-style, not Linux-only.

I would still perhaps drop the paranthetical part as it reads that way
I think even with -style vs -only.

>> (Similarly, I think the earlier reviews I saw around ptrace() claimed that
>> NetBSD was the only OS to use LWPs with ptrace() in the log messages in
>> effect which isn't really true as both Solaris and FreeBSD use LWPs
>> happily with ptrace(), just using a different convention.)
>>
> 
> The pair of (PID+LWP) for getters and setters of registers (among
> certain other ptrace operations) is NetBSD style ptrace(2) API design.
> This leads to the point that NetBSD is currently the only OS where
> get_ptrace_pid() is not compatible (at leat in the current form).

It might be a bit of how it was said, the actual text:

<quote>
    Unlike most other Operating Systems, NetBSD tracks both pid and lwp.
    The process id on NetBSD is stored always in the pid field of ptid.
</quote>

If by "track" you mean "the kernel keeps track of LWPs as distinct
from processes" (which is how I read "track"), then I think it is
inaccurate.


>> I would also tone this down a bit, and at least reference the correct
>> specification.  The ELF spec doesn't define the layout of auxv_t.  The
>> per-architecture psABI documents do.  Also, just saying that you follow
>> the spec doesn't help.  I would suggest something like:
>>
>> /* NetBSD-specific parser for AUXV entries.  NetBSD always uses an int
>>    to store the type as defined in the SVR4 psABI specifications rather
>>    than long as assumed by the default parser.  */
>>
> 
> This is toned down compared to obsd-tdet.c, that says:
> 
> /* Unlike Linux, OpenBSD actually follows the ELF standard.  */

That may be, but that probably isn't who I would choose as a model to
follow.

> OK, is this patch fine after rephrasing the above texts?

It looks fine to me, but it probably needs an approver such as Tom to ok it.
(I can approve FreeBSD-related things, but not really other bits.)

-- 
John Baldwin

Re: [PATCH v3] Add support for "info auxv" on NetBSD

[Kamil Rytarowski]

On 27.03.2020 20:22, John Baldwin wrote:
> On 3/27/20 10:04 AM, Kamil Rytarowski wrote:
>> On 27.03.2020 17:31, John Baldwin wrote:
>>> On 3/26/20 4:26 PM, Kamil Rytarowski wrote:
>>>> Ping?
>>>>
>>>> On 20.03.2020 18:27, Kamil Rytarowski wrote:
>>>>> Register nbsd_auxv_parse() that overloads the default (Linux-style)
>>>>> AUXV parsing. On NetBSD the type parameter is defined as int32_t
>>>>> for all architectures.
>>>
>>> I would tone down some of the rhetoric.  FreeBSD uses the default AUXV
>>> parsing, and I think Solaris does as well, so describing it as Linux-only
>>> isn't very accurate.
>>>
>>
>> It is phrases as Linux-style, not Linux-only.
>
> I would still perhaps drop the paranthetical part as it reads that way
> I think even with -style vs -only.
>
>>> (Similarly, I think the earlier reviews I saw around ptrace() claimed that
>>> NetBSD was the only OS to use LWPs with ptrace() in the log messages in
>>> effect which isn't really true as both Solaris and FreeBSD use LWPs
>>> happily with ptrace(), just using a different convention.)
>>>
>>
>> The pair of (PID+LWP) for getters and setters of registers (among
>> certain other ptrace operations) is NetBSD style ptrace(2) API design.
>> This leads to the point that NetBSD is currently the only OS where
>> get_ptrace_pid() is not compatible (at leat in the current form).
>
> It might be a bit of how it was said, the actual text:
>
> <quote>
>     Unlike most other Operating Systems, NetBSD tracks both pid and lwp.
>     The process id on NetBSD is stored always in the pid field of ptid.
> </quote>
>
> If by "track" you mean "the kernel keeps track of LWPs as distinct
> from processes" (which is how I read "track"), then I think it is
> inaccurate.
>
>
>>> I would also tone this down a bit, and at least reference the correct
>>> specification.  The ELF spec doesn't define the layout of auxv_t.  The
>>> per-architecture psABI documents do.  Also, just saying that you follow
>>> the spec doesn't help.  I would suggest something like:
>>>
>>> /* NetBSD-specific parser for AUXV entries.  NetBSD always uses an int
>>>    to store the type as defined in the SVR4 psABI specifications rather
>>>    than long as assumed by the default parser.  */
>>>
>>
>> This is toned down compared to obsd-tdet.c, that says:
>>
>> /* Unlike Linux, OpenBSD actually follows the ELF standard.  */
>
> That may be, but that probably isn't who I would choose as a model to
> follow.
>
>> OK, is this patch fine after rephrasing the above texts?
>
> It looks fine to me, but it probably needs an approver such as Tom to ok it.
> (I can approve FreeBSD-related things, but not really other bits.)
>

OK. I will rephrase the texts as I understand that the wording is sensitive.

Re: [PATCH v3] Add support for "info auxv" on NetBSD

[Simon Marchi]

On 2020-03-20 1:27 p.m., Kamil Rytarowski wrote:
> @@ -47,3 +48,40 @@ nbsd_pc_in_sigtramp (CORE_ADDR pc, const char *func_name)
>    return (func_name != NULL
>  	  && startswith (func_name, "__sigtramp"));
>  }
> +
> +/* NetBSD-specific parser for AUXV data with. NetBSD follows the ELF
> +   specification, contrary to some other ELF Operating Systems.  */
> +
> +static int
> +nbsd_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;

From this code, I understand that on AMD64/NetBSD, an auxv entry looks like
(each character is a byte):

T T T T P P P P V V V V V V V V

Where T is the type value, P is padding and V is value.  Is that right?

> +
> +  *readptr = ptr;
> +  return 1;
> +}

Instead of defining this function that is very similar to default_auxv_parse, I would
strongly suggest making a parametrized version of default_auxv_parse (and make
default_auxv_parse use it).

Simon