[v2,5/6] gdb/aarch64: Factor out most of the thread_architecture method

  The same logic will be used by a subsequent commit when remotely debugging
an aarch64-linux target.

The code isn't changed, just moved around.
---
 gdb/aarch64-linux-nat.c | 28 ++--------------------------
 gdb/aarch64-tdep.c      | 35 +++++++++++++++++++++++++++++++++++
 gdb/aarch64-tdep.h      |  2 ++
 3 files changed, 39 insertions(+), 26 deletions(-)
  

On 11/26/22 02:04, Thiago Jung Bauermann wrote:
> The same logic will be used by a subsequent commit when remotely debugging
> an aarch64-linux target.
> 
> The code isn't changed, just moved around.
> ---
>   gdb/aarch64-linux-nat.c | 28 ++--------------------------
>   gdb/aarch64-tdep.c      | 35 +++++++++++++++++++++++++++++++++++
>   gdb/aarch64-tdep.h      |  2 ++
>   3 files changed, 39 insertions(+), 26 deletions(-)
> 
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index caefcb364852..ca230ea4fdb0 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -884,33 +884,9 @@ aarch64_linux_nat_target::thread_architecture (ptid_t ptid)
>     /* Find the current gdbarch the same way as process_stratum_target.  */
>     inferior *inf = find_inferior_ptid (this, ptid);
>     gdb_assert (inf != NULL);
> -
> -  /* If this is a 32-bit architecture, then this is ARM, not AArch64.
> -     There's no SVE vectors here, so just return the inferior
> -     architecture.  */
> -  if (gdbarch_bfd_arch_info (inf->gdbarch)->bits_per_word == 32)
> -    return inf->gdbarch;
> -
> -  /* Only return it if the current vector length matches the one in the tdep.  */
> -  aarch64_gdbarch_tdep *tdep
> -    = gdbarch_tdep<aarch64_gdbarch_tdep> (inf->gdbarch);
>     uint64_t vq = aarch64_sve_get_vq (ptid.lwp ());
> -  if (vq == tdep->vq)
> -    return inf->gdbarch;
> -
> -  /* We reach here if the vector length for the thread is different from its
> -     value at process start.  Lookup gdbarch via info (potentially creating a
> -     new one) by using a target description that corresponds to the new vq value
> -     and the current architecture features.  */
> -
> -  const struct target_desc *tdesc = gdbarch_target_desc (inf->gdbarch);
> -  aarch64_features features = aarch64_features_from_target_desc (tdesc);
> -  features.vq = vq;
> -
> -  struct gdbarch_info info;
> -  info.bfd_arch_info = bfd_lookup_arch (bfd_arch_aarch64, bfd_mach_aarch64);
> -  info.target_desc = aarch64_read_description (features);
> -  return gdbarch_find_by_info (info);
> +
> +  return aarch64_update_gdbarch (inf->gdbarch, vq);
>   }
>   
>   /* Implement the "supports_memory_tagging" target_ops method.  */
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 07330356fdcb..ffc128d91f60 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -3486,6 +3486,41 @@ aarch64_cannot_store_register (struct gdbarch *gdbarch, int regnum)
>   	  || regnum == AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base));
>   }
>   
> +/* Helper function for the "thread_architecture" target_ops method.
> +
> +   Returns a new gdbarch that is equivalent to the given gdbarch, but with SVE
> +   registers reflecting the given vq value.  */
> +
> +struct gdbarch *
> +aarch64_update_gdbarch (struct gdbarch *gdbarch, uint64_t vq)
> +{
> +  /* If this is a 32-bit architecture, then this is ARM, not AArch64.
> +     There's no SVE vectors here, so just return the inferior
> +     architecture.  */
> +  if (gdbarch_bfd_arch_info (gdbarch)->bits_per_word == 32)
> +    return gdbarch;
> +
> +  /* Only return it if the current vector length matches the one in the
> +     tdep.  */
> +  aarch64_gdbarch_tdep *tdep = gdbarch_tdep<aarch64_gdbarch_tdep> (gdbarch);
> +  if (vq == tdep->vq)
> +    return gdbarch;
> +
> +  /* We reach here if the vector length for the thread is different from its
> +     value at process start.  Lookup gdbarch via info (potentially creating a
> +     new one) by using a target description that corresponds to the new vq value
> +     and the current architecture features.  */
> +
> +  const struct target_desc *tdesc = gdbarch_target_desc (gdbarch);
> +  aarch64_features features = aarch64_features_from_target_desc (tdesc);
> +  features.vq = vq;
> +
> +  struct gdbarch_info info;
> +  info.bfd_arch_info = bfd_lookup_arch (bfd_arch_aarch64, bfd_mach_aarch64);
> +  info.target_desc = aarch64_read_description (features);
> +  return gdbarch_find_by_info (info);
> +}
> +
>   /* Implement the stack_frame_destroyed_p gdbarch method.  */
>   
>   static int
> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> index 55ccf2e777d2..80b9b3281a2d 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -143,4 +143,6 @@ void aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
>   
>   bool aarch64_displaced_step_hw_singlestep (struct gdbarch *gdbarch);
>   
> +struct gdbarch *aarch64_update_gdbarch (struct gdbarch *gdbarch, uint64_t vq);
> +
>   #endif /* aarch64-tdep.h */

LGTM.

Approved-by: Luis Machado <luis.machado@arm.com>
  

>  /* Implement the "supports_memory_tagging" target_ops method.  */
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 07330356fdcb..ffc128d91f60 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -3486,6 +3486,41 @@ aarch64_cannot_store_register (struct gdbarch *gdbarch, int regnum)
>  	  || regnum == AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base));
>  }
>  
> +/* Helper function for the "thread_architecture" target_ops method.
> +
> +   Returns a new gdbarch that is equivalent to the given gdbarch, but with SVE
> +   registers reflecting the given vq value.  */
> +
> +struct gdbarch *
> +aarch64_update_gdbarch (struct gdbarch *gdbarch, uint64_t vq)

Document this using:

/* See aarch64-tdep.h.  */

... and move the doc to the header file.

Otherwise:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon
  

Luis Machado <luis.machado@arm.com> writes:

> On 11/26/22 02:04, Thiago Jung Bauermann wrote:
>> The same logic will be used by a subsequent commit when remotely debugging
>> an aarch64-linux target.
>> The code isn't changed, just moved around.
>> ---
>>   gdb/aarch64-linux-nat.c | 28 ++--------------------------
>>   gdb/aarch64-tdep.c      | 35 +++++++++++++++++++++++++++++++++++
>>   gdb/aarch64-tdep.h      |  2 ++
>>   3 files changed, 39 insertions(+), 26 deletions(-)
>
> LGTM.
>
> Approved-by: Luis Machado <luis.machado@arm.com>

Thanks! Added the Approved-by tag to the patch description for v3.
  

Simon Marchi <simark@simark.ca> writes:

>>  /* Implement the "supports_memory_tagging" target_ops method.  */
>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>> index 07330356fdcb..ffc128d91f60 100644
>> --- a/gdb/aarch64-tdep.c
>> +++ b/gdb/aarch64-tdep.c
>> @@ -3486,6 +3486,41 @@ aarch64_cannot_store_register (struct gdbarch *gdbarch, int regnum)
>>  	  || regnum == AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base));
>>  }
>>  
>> +/* Helper function for the "thread_architecture" target_ops method.
>> +
>> +   Returns a new gdbarch that is equivalent to the given gdbarch, but with SVE
>> +   registers reflecting the given vq value.  */
>> +
>> +struct gdbarch *
>> +aarch64_update_gdbarch (struct gdbarch *gdbarch, uint64_t vq)
>
> Document this using:
>
> /* See aarch64-tdep.h.  */
>
> ... and move the doc to the header file.

I did that for v3.

> Otherwise:
>
> Approved-By: Simon Marchi <simon.marchi@efficios.com>

Thanks! Added the Approved-by tag to the patch description for v3.