Re: [PATCH v5 06/16] [gdbserver/aarch64] refactor: Adjust expedited registers dynamically

Luis Machado via Gdb-patches
2023-09-08T16:01:44Z

On 9/8/23 16:35, Simon Marchi wrote:
> On 9/7/23 11:20, Luis Machado via Gdb-patches wrote:
>> Instead of using static arrays, build the list of expedited registers
>> dynamically using a std::vector.
>>
>> This refactor shouldn't cause any user-visible changes.
>>
>> Regression-tested for aarch64-linux Ubuntu 22.04/20.04.
>> ---
>> gdbserver/linux-aarch64-tdesc.cc | 21 ++++++++++++++-------
>> 1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/gdbserver/linux-aarch64-tdesc.cc b/gdbserver/linux-aarch64-tdesc.cc
>> index 633134955e5..3c60e1a4db0 100644
>> --- a/gdbserver/linux-aarch64-tdesc.cc
>> +++ b/gdbserver/linux-aarch64-tdesc.cc
>> @@ -30,6 +30,8 @@
>> /* All possible aarch64 target descriptors. */
>> static std::unordered_map<aarch64_features, target_desc *> tdesc_aarch64_map;
>>
>> +static std::vector<const char *> expedited_registers;
>> +
>> /* Create the aarch64 target description. */
>>
>> const target_desc *
>> @@ -44,15 +46,20 @@ aarch64_linux_read_description (const aarch64_features &features)
>> if (tdesc == NULL)
>> {
>> tdesc = aarch64_create_target_description (features);
>> + expedited_registers.clear ();
>> +
>> + /* Configure the expedited registers. By default we include x29, sp and
>> + pc. */
>> + expedited_registers.push_back ("x29");
>> + expedited_registers.push_back ("sp");
>> + expedited_registers.push_back ("pc");
>> +
>> + if (features.vq > 0)
>> + expedited_registers.push_back ("vg");
>>
>> - static const char *expedite_regs_aarch64[] = { "x29", "sp", "pc", NULL };
>> - static const char *expedite_regs_aarch64_sve[] = { "x29", "sp", "pc",
>> - "vg", NULL };
>> + expedited_registers.push_back (nullptr);
>>
>> - if (features.vq == 0)
>> - init_target_desc (tdesc, expedite_regs_aarch64);
>> - else
>> - init_target_desc (tdesc, expedite_regs_aarch64_sve);
>> + init_target_desc (tdesc, (const char **) expedited_registers.data ());
>
> It seems weird to have a single std::vector instance. What happens if
> multiple target descriptions are created? Won't the second mess up the
> data of the first one?

Yeah. Well spotted. I think I'll need to have a map-based entry for the expedited registers, because init_target_desc
just copies the pointer instead of the contents.

Let me think about that.

Thanks,
Luis