Re: [PATCH 7/8] gdb/aarch64: Factor out most of the thread_architecture method

[Luis Machado <luis.machado@arm.com>]

On 9/8/22 07:41, Thiago Jung Bauermann via Gdb-patches 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 | 25 +------------------------
>   gdb/aarch64-tdep.c      | 34 ++++++++++++++++++++++++++++++++++
>   gdb/aarch64-tdep.h      |  2 ++
>   3 files changed, 37 insertions(+), 24 deletions(-)
> 
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index 633cbc08796c..ff4c69f6af7d 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -891,36 +891,13 @@ aarch64_linux_nat_target::thread_architecture (ptid_t ptid)
>     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);
>     int vq = aarch64_sve_get_vq (ptid.lwp ());
>   
>     /* If ptrace fails we can't determine vq, but the thread_architecture method
>        always succeeds so all we can do here is assert that vq is valid.  */
>     gdb_assert (vq >= 0);
> -  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 7229b53838e8..18c2b1ec1523 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -3420,6 +3420,40 @@ 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_create_target_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 d8513023c376..3d78515e167f 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -139,4 +139,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