Re: [PATCH v3 4/8] gdbserver/linux-aarch64: When thread stops, update its target description

Luis Machado via Gdb-patches
2023-02-06T09:08:36Z

On 2/4/23 15:21, Thiago Jung Bauermann wrote:
>
> Luis Machado <luis.machado@arm.com> writes:
>
>> On 2/2/23 03:47, Simon Marchi wrote:
>>> On 2/1/23 21:54, Thiago Jung Bauermann wrote:
>>>> In any case, it wouldn't be possible to make get_thread_target_desc just
>>>> return thread_info->tdesc because at least the way these patches are
>>>> currently written, when the inferior starts or a new thread of the
>>>> inferior is spawned thread_info->tdesc is nullptr. gdbserver will only
>>>> call get_thread_tdesc after the first stop (in get_thread_regcache, in
>>>> the process of obtaining the pc register), so we will need to cope with
>>>> that situation.
>>> Ok. Would it work if a new thread initially inherited the tdesc from
>>> its process?
>>>
>>
>> It should be fine because the first time we fetch a process target
>> description, it is eventually obtained from the first and only thread.
>> So the SVE vector length should be correct.
>>
>> Any subsequent attempts to use the process' target description (the
>> first one we obtained), after further stops, may end up using an
>> incorrect description.
>>
>> I think this is handled correctly by the target architecture target
>> hook though. But there are other places where this is potentially
>> incorrect.
>>
>> For example...
>>
>> - When using gcore to dump a core file, GDB only dumps a single target
>> description. While this might be correct for a target with a fixed
>> target description or a AArch64 target that doesn't support SVE, it
>> likely won't be correctly for one AArch64 target supporting SVE if its
>> threads changed vector length mid-execution. Either we emit target
>> description notes by thread, or we don't emit a target description
>> note for those cases.
>>
>> - When loading the above/older gcore core files back, GDB will use a
>> potentially incorrect target description. If we decide to emit
>> per-thread target descriptions, it should be fine. Otherwise we may
>> need to have a "thread architecture" hook for core files as well.
>>
>> - The remote has no concept of a thread architecture (Thiago is
>> addressing this with this patch series).
>>
>> - AArch64 frames may have slightly different vg values, which means
>> their gdbarches are different as well.
>>
>> Given the differences between two gdbarches are small, we mostly get
>> away with it. But if there are further differences (different hooks,
>> for example), I fear we may run into a situation where we use an
>> incorrect gdbarch to call a particular hook.
>
> Indeed, good points! Thank you for bringing them up. I can address core
> file dumping/loading after this series.

I have an upcoming patch for SME that should address this for core files, but it still needs some testing.

>
> Regarding frames with different vg values, it's important to be aware of
> this discrepancy but IMHO it makes sense to work on it when it becomes
> a problem...
>

Indeed. It will be a problem for SME and streaming mode, but I have another upcoming patch to hopefully address this as well.