[AArch64][6/6] Core file support for "pauth" feature

[AArch64][6/6] Core file support for "pauth" feature

[Jiong Wang]

[-- Attachment #1: Type: text/plain, Size: 1026 bytes --]

This patch add coredump support for the new "pauth" feature.  A few necessary
low level methods and descriptions are added accordingly.

2017-08-09  Jiong Wang  <jiong.wang@arm.com>
bfd/
	* elf-bfd.h (elfcore_write_aarch_pauth): New function declaration.
	* elf.c (elfcore_grok_aarch_pauth): New function.
	(elfcore_grok_note): Handle NT_ARM_PAUTH.
	(elfcore_write_aarch_pauth): New function.
	(elfcore_write_register_note): Handle ".reg-aarch-pauth" section.

gdb/
	* aarch64-linux-tdep.c: #include "auxv.h".
	#include "elf/common.h"
	(aarch64_collect_pauth_regset): New function.
	(aarch64_supply_pauth_regset): New function.
	(aarch64_linux_pauthregset): New regset.
	(aarch64_linux_iterate_over_regset_sections): Support
	".reg-aarch64-pauth" section.
	(aarch64_linux_core_read_description): New function.
	(aarch64_linux_init_abi): Register core_read_description method.
	* aarch64-linux-tdep.h (AARCH64_LINUX_SIZEOF_PAUTH): New define.
	(HWCAP_APIA): New define.
	(aarch64_linux_pauthregset): New variable declaration.


[-- Attachment #2: gdb-6.patch --]
[-- Type: text/x-patch, Size: 6583 bytes --]

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 92a8e02..d9b61ba 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2539,6 +2539,8 @@ extern char *elfcore_write_s390_gs_bc
   (bfd *, char *, int *, const void *, int);
 extern char *elfcore_write_arm_vfp
   (bfd *, char *, int *, const void *, int);
+extern char *elfcore_write_aarch_pauth
+  (bfd *, char *, int *, const void *, int);
 extern char *elfcore_write_aarch_tls
   (bfd *, char *, int *, const void *, int);
 extern char *elfcore_write_aarch_hw_break
diff --git a/bfd/elf.c b/bfd/elf.c
index b99e297..30772cf 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -9207,6 +9207,12 @@ elfcore_grok_arm_vfp (bfd *abfd, Elf_Internal_Note *note)
 }
 
 static bfd_boolean
+elfcore_grok_aarch_pauth (bfd *abfd, Elf_Internal_Note *note)
+{
+  return elfcore_make_note_pseudosection (abfd, ".reg-aarch-pauth", note);
+}
+
+static bfd_boolean
 elfcore_grok_aarch_tls (bfd *abfd, Elf_Internal_Note *note)
 {
   return elfcore_make_note_pseudosection (abfd, ".reg-aarch-tls", note);
@@ -9699,6 +9705,13 @@ elfcore_grok_note (bfd *abfd, Elf_Internal_Note *note)
       else
 	return TRUE;
 
+    case NT_ARM_PAC_MASK:
+      if (note->namesz == 6
+	  && strcmp (note->namedata, "LINUX") == 0)
+	return elfcore_grok_aarch_pauth (abfd, note);
+      else
+	return TRUE;
+
     case NT_ARM_TLS:
       if (note->namesz == 6
 	  && strcmp (note->namedata, "LINUX") == 0)
@@ -10794,6 +10807,18 @@ elfcore_write_arm_vfp (bfd *abfd,
 }
 
 char *
+elfcore_write_aarch_pauth (bfd *abfd,
+			   char *buf,
+			   int *bufsiz,
+			   const void *aarch_pauth,
+			   int size)
+{
+  char *note_name = "LINUX";
+  return elfcore_write_note (abfd, buf, bufsiz,
+			     note_name, NT_ARM_PAC_MASK, aarch_pauth, size);
+}
+
+char *
 elfcore_write_aarch_tls (bfd *abfd,
 		       char *buf,
 		       int *bufsiz,
@@ -10875,6 +10900,8 @@ elfcore_write_register_note (bfd *abfd,
     return elfcore_write_s390_gs_bc (abfd, buf, bufsiz, data, size);
   if (strcmp (section, ".reg-arm-vfp") == 0)
     return elfcore_write_arm_vfp (abfd, buf, bufsiz, data, size);
+  if (strcmp (section, ".reg-aarch-pauth") == 0)
+    return elfcore_write_aarch_pauth (abfd, buf, bufsiz, data, size);
   if (strcmp (section, ".reg-aarch-tls") == 0)
     return elfcore_write_aarch_tls (abfd, buf, bufsiz, data, size);
   if (strcmp (section, ".reg-aarch-hw-break") == 0)
diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
index ec6125a..56ca1ce 100644
--- a/gdb/aarch64-linux-tdep.c
+++ b/gdb/aarch64-linux-tdep.c
@@ -45,6 +45,8 @@
 
 #include "record-full.h"
 #include "linux-record.h"
+#include "auxv.h"
+#include "elf/common.h"
 
 /* Signal frame handling.
 
@@ -217,6 +219,65 @@ const struct regset aarch64_linux_fpregset =
     regcache_supply_regset, regcache_collect_regset
   };
 
+/* Collect register REGNUM from REGCACHE and store its contents in the buffer
+   PAUTH_REGS which is of SIZE.  REGSET is not used as for PAUTH regset the
+   register number inside GDB is dynamicly allocated so we are not using
+   register map in REGSET.  */
+
+static void
+aarch64_collect_pauth_regset (const struct regset *regset,
+			      const struct regcache *regcache,
+			      int regnum, void *pauth_regs, size_t size)
+{
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  gdb_assert (size == AARCH64_LINUX_SIZEOF_PAUTH);
+  gdb_assert (regnum == -1);
+  gdb_assert (aarch64_tdep_has_pauth_p (tdep));
+
+  int dmask = tdep->regnum.pauth_reg_base;
+  int cmask = tdep->regnum.pauth_reg_base + 1;
+
+  memset (pauth_regs, 0, AARCH64_LINUX_SIZEOF_PAUTH);
+  regcache_raw_collect (regcache, dmask, pauth_regs);
+  regcache_raw_collect (regcache, cmask,
+			(gdb_byte *) pauth_regs + X_REGISTER_SIZE);
+
+  return;
+}
+
+/* Supply register REGNUM from the buffer PAUTH_REGS which is of SIZE to
+   REGCACHE.  REGSET is not used as for PAUTH regset the register number inside
+   GDB is dynamicly allocated so we are not using register map in REGSET.  */
+
+static void
+aarch64_supply_pauth_regset (const struct regset *regset,
+			     struct regcache *regcache,
+			     int regnum, const void *pauth_regs, size_t size)
+{
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  gdb_assert (size == AARCH64_LINUX_SIZEOF_PAUTH);
+  gdb_assert (regnum == -1);
+  gdb_assert (aarch64_tdep_has_pauth_p (tdep));
+
+  int dmask = tdep->regnum.pauth_reg_base;
+  int cmask = tdep->regnum.pauth_reg_base + 1;
+
+  regcache_raw_supply (regcache, dmask, pauth_regs);
+  regcache_raw_supply (regcache, cmask,
+		       (gdb_byte *) pauth_regs + X_REGISTER_SIZE);
+
+  return;
+}
+
+const struct regset aarch64_linux_pauthregset =
+  {
+    NULL, aarch64_supply_pauth_regset, aarch64_collect_pauth_regset
+  };
+
 /* Implement the "regset_from_core_section" gdbarch method.  */
 
 static void
@@ -225,10 +286,16 @@ aarch64_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
 					    void *cb_data,
 					    const struct regcache *regcache)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
   cb (".reg", AARCH64_LINUX_SIZEOF_GREGSET, &aarch64_linux_gregset,
       NULL, cb_data);
   cb (".reg2", AARCH64_LINUX_SIZEOF_FPREGSET, &aarch64_linux_fpregset,
       NULL, cb_data);
+
+  if (aarch64_tdep_has_pauth_p (tdep))
+    cb (".reg-aarch-pauth", AARCH64_LINUX_SIZEOF_PAUTH,
+	&aarch64_linux_pauthregset, NULL, cb_data);
 }
 
 /* Determine target description from core file.  */
@@ -242,7 +309,7 @@ aarch64_linux_core_read_description (struct gdbarch *gdbarch,
   if (target_auxv_search (target, AT_HWCAP, &aarch64_hwcap) != 1)
     return NULL;
 
-  return tdesc_aarch64;
+  return aarch64_hwcap & HWCAP_APIA ? tdesc_aarch64_pauth : tdesc_aarch64;
 }
 
 /* Implementation of `gdbarch_stap_is_single_operand', as defined in
diff --git a/gdb/aarch64-linux-tdep.h b/gdb/aarch64-linux-tdep.h
index d0f9b12..580a8b8 100644
--- a/gdb/aarch64-linux-tdep.h
+++ b/gdb/aarch64-linux-tdep.h
@@ -29,6 +29,14 @@
    are 4 bytes wide each, and the whole structure is padded to 128 bit
    alignment.  */
 #define AARCH64_LINUX_SIZEOF_FPREGSET (33 * V_REGISTER_SIZE)
+#define AARCH64_LINUX_SIZEOF_PAUTH (2 * X_REGISTER_SIZE)
 
 extern const struct regset aarch64_linux_gregset;
 extern const struct regset aarch64_linux_fpregset;
+extern const struct regset aarch64_linux_pauthregset;
+
+#ifndef HWCAP_APIA
+/* AArch64 GNU/Linux HWCAP values.  These should be synced with kernel
+   definitions.  */
+#define HWCAP_APIA (1 << 16)
+#endif

Re: [AArch64][6/6] Core file support for "pauth" feature

[Nick Clifton]

Hi Jiong,

> 2017-08-09  Jiong Wang  <jiong.wang@arm.com>
> bfd/
>      * elf-bfd.h (elfcore_write_aarch_pauth): New function declaration.
>      * elf.c (elfcore_grok_aarch_pauth): New function.
>      (elfcore_grok_note): Handle NT_ARM_PAUTH.
>      (elfcore_write_aarch_pauth): New function.
>      (elfcore_write_register_note): Handle ".reg-aarch-pauth" section.

This part of the patch is approved.

Cheers
  Nick

Re: [AArch64][6/6] Core file support for "pauth" feature

[Yao Qi]

On 17-08-10 12:08:36, Pedro Alves wrote:
> > +#ifndef HWCAP_APIA
> > +/* AArch64 GNU/Linux HWCAP values.  These should be synced with kernel
> > +   definitions.  */
> > +#define HWCAP_APIA (1 << 16)
> > +#endif
> 
> Re. the #ifndef, consider that tdep.h files are included in cross
> debugger builds.  E.g., an x86-hosted gdb cross debugging aarch64.
> Some archs have "namespaced" names like the s390 mips, sparc, etc.
> (e.g., HWCAP_S390_VX) which avoids the case of the names being defined
> on host/target with a different meanings/values, but not all do.
> But even with such names, we always have to provide fallback definitions
> for cross debuggers.  And with that all in mind, and since you're defining
> fallbacks anyway, how about unconditionally defining/using our
> own conflict-resistant versions, like AARCH64_HWCAP_APIA?
> 

I am inclined to use the same macro name as kernel uses.  These macros are
only used in $arch-linux-{tdep,nat}.c, so it is clear that the macros
are about architecture $arch.

-- 
Yao (齐尧)

Re: [AArch64][6/6] Core file support for "pauth" feature

[Pedro Alves]

On 08/10/2017 10:22 PM, Yao Qi wrote:
> On 17-08-10 12:08:36, Pedro Alves wrote:
>>> +#ifndef HWCAP_APIA
>>> +/* AArch64 GNU/Linux HWCAP values.  These should be synced with kernel
>>> +   definitions.  */
>>> +#define HWCAP_APIA (1 << 16)
>>> +#endif
>>
>> Re. the #ifndef, consider that tdep.h files are included in cross
>> debugger builds.  E.g., an x86-hosted gdb cross debugging aarch64.
>> Some archs have "namespaced" names like the s390 mips, sparc, etc.
>> (e.g., HWCAP_S390_VX) which avoids the case of the names being defined
>> on host/target with a different meanings/values, but not all do.
>> But even with such names, we always have to provide fallback definitions
>> for cross debuggers.  And with that all in mind, and since you're defining
>> fallbacks anyway, how about unconditionally defining/using our
>> own conflict-resistant versions, like AARCH64_HWCAP_APIA?
>>
> 
> I am inclined to use the same macro name as kernel uses.  These macros are
> only used in $arch-linux-{tdep,nat}.c, so it is clear that the macros
> are about architecture $arch.

I think there's a misunderstanding.  It's not about clarity -- if HWCAP_APIA
is defined on a !Aarch64 host as some value other than "(1 << 16)", then
this:

> +++ b/gdb/aarch64-linux-tdep.c
>  
> -  return tdesc_aarch64;
> +  return aarch64_hwcap & HWCAP_APIA ? tdesc_aarch64_pauth : tdesc_aarch64;
>  }

will silently compile to use wrong value.

Might never happen in practice, but why write a potential problem,
_particularly since you already have to write the fallback
macro anyway_?  What's the advantage of not doing what I suggested?

It'd be different if the macro was _only_ used in a -nat.c
file, but then I'd object to defining it in the -tdep.h file.

Thanks,
Pedro Alves

Re: [AArch64][6/6] Core file support for "pauth" feature

[Yao Qi]

On 17-08-10 22:32:40, Pedro Alves wrote:
> 
> I think there's a misunderstanding.  It's not about clarity -- if HWCAP_APIA
> is defined on a !Aarch64 host as some value other than "(1 << 16)", then
> this:
> 
> > +++ b/gdb/aarch64-linux-tdep.c
> >  
> > -  return tdesc_aarch64;
> > +  return aarch64_hwcap & HWCAP_APIA ? tdesc_aarch64_pauth : tdesc_aarch64;
> >  }
> 
> will silently compile to use wrong value.
> 
> Might never happen in practice, but why write a potential problem,
> _particularly since you already have to write the fallback
> macro anyway_?  What's the advantage of not doing what I suggested?

I want the macro name sync'ed with kernel.  However I understand your
point.  Let me see if I can ask kernel people change it to HWCAP_ARM64_APIA,
or something else.  Kernel patches are not merged yet, as far I as
know.

-- 
Yao (齐尧)

Re: [AArch64][6/6] Core file support for "pauth" feature

[Yao Qi]

On 17-08-10 22:04:02, Yao Qi wrote:
> 
> I want the macro name sync'ed with kernel.  However I understand your
> point.  Let me see if I can ask kernel people change it to HWCAP_ARM64_APIA,
> or something else.  Kernel patches are not merged yet, as far I as
> know.

Kernel people don't want to change the naming scheme for new hwcaps.
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-August/525204.html
How about we unconditionally define HWCAP_ARM64_APIA in aarch64-linux-tdep.c?
WDYT?

#define HWCAP_ARM64_APIA (1 << 16)

-- 
Yao (齐尧)

Re: [AArch64][6/6] Core file support for "pauth" feature

[Pedro Alves]

On 08/11/2017 04:38 PM, Yao Qi wrote:
> On 17-08-10 22:04:02, Yao Qi wrote:

> How about we unconditionally define HWCAP_ARM64_APIA in aarch64-linux-tdep.c?
> WDYT?
> 
> #define HWCAP_ARM64_APIA (1 << 16)
> 

Agreed, because that's essentially what I was suggesting.  :-)

The difference was that I was suggesting 'AARCH64_ + $org_name', to
follow the scheme of all other GDB-invented Aarch64-related macro symbols.
AARCH64_HWCAP_APIA seems more clearly to me a gdb-replacement name,
than HWCAP_ARM64_APIA.  I could totally see someone confusing the latter
for a real kernel-defined name, since it follows the scheme of
other ports.  But that's obviously a minor thing.

Thanks,
Pedro Alves