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

  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.
  

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
  

On 08/09/2017 01:23 PM, Jiong Wang wrote:

>  /* 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. 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?

Thanks,
Pedro Alves
  

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.
  

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
  

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.
  

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)
  

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