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

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

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

> On 9/8/22 07:41, Thiago Jung Bauermann via Gdb-patches wrote:
>> +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);

Oops, I hadn't noticed that this patch had a bug (memory leak,
actually): it inadvertently changed the moved code to call
aarch64_create_target_description instead of aarch64_read_description.

In v2 I fixed this, and double-checked that there's no actual change in
the moved code.

>> +  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

Thank you for your review!

-- 
Thiago