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

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

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);
>      }
>