Re: [PATCH v5 12/16] [gdb/generic] corefile/bug: Use thread-specific gdbarch when dumping register state to core files

Luis Machado via Gdb-patches
2023-09-08T11:10:03Z

Could a global maintainer please go through this change and let me know if it is OK? It touches a generic part of gdb.

Though I don't think it should change the behavior of non-aarch64 targets.

On 9/7/23 16:20, Luis Machado via Gdb-patches wrote:
> When we have a core file generated by gdb (via the gcore command), gdb dumps
> the target description to a note. During loading of that core file, gdb will
> first try to load that saved target description.
>
> This works fine for almost all architectures. But AArch64 has a few
> dynamically-generated target descriptions/gdbarch depending on the vector
> length that was in use at the time the core file was generated.
>
> The target description gdb dumps to the core file note is the one generated
> at the time of attachment/startup. If, for example, the SVE vector length
> changed during execution, this would not reflect on the core file, as gdb
> would still dump the initial target description.
>
> Another issue is that the gdbarch potentially doesn't match the thread's
> real gdbarch, and so things like the register cache may have different formats
> and sizes.
>
> To address this, fetch the thread's architecture before dumping its register
> state. That way we will always use the correct target description/gdbarch.
> ---
> gdb/linux-tdep.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index b5eee5e108c..7d0976932c6 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -2099,12 +2099,28 @@ linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
> stop_signal);
>
> if (signalled_thr != nullptr)
> - linux_corefile_thread (signalled_thr, &thread_args);
> + {
> + /* On some architectures, like AArch64, each thread can have a distinct
> + gdbarch (due to scalable extensions), and using the inferior gdbarch
> + is incorrect.
> +
> + Fetch each thread's gdbarch and pass it down to the lower layers so
> + we can dump the right set of registers. */
> + thread_args.gdbarch = target_thread_architecture (signalled_thr->ptid);
> + linux_corefile_thread (signalled_thr, &thread_args);
> + }
> for (thread_info *thr : current_inferior ()->non_exited_threads ())
> {
> if (thr == signalled_thr)
> continue;
>
> + /* On some architectures, like AArch64, each thread can have a distinct
> + gdbarch (due to scalable extensions), and using the inferior gdbarch
> + is incorrect.
> +
> + Fetch each thread's gdbarch and pass it down to the lower layers so
> + we can dump the right set of registers. */
> + thread_args.gdbarch = target_thread_architecture (thr->ptid);
> linux_corefile_thread (thr, &thread_args);
> }
>