Re: [PATCH v5 13/16] [gdb/generic] corefile/bug: Fixup (gcore) core file target description reading order

[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 as well.

This has the potential to cause some issues for some targets relying on the gdb xml core file note.

On 9/7/23 16:20, Luis Machado via Gdb-patches wrote:
> Due to the nature of the AArch64 SVE/SME extensions in GDB, each thread
> can potentially have distinct target descriptions/gdbarches.
> 
> When loading a gcore-generated core file, at the moment GDB gives priority
> to the target description dumped to NT_GDB_TDESC.  Though technically correct
> for most target, it doesn't work correctly for AArch64 with SVE or SME
> support.
> 
> The correct approach for AArch64/Linux is to rely on the
> gdbarch_core_read_description hook, so it can figure out the proper target
> description for a given thread based on the various available register notes.
> 
> I think this should work for other architectures as well. If not, we may
> need to adjust things so all architectures get the information that they
> need for discovering the target description of the core file.
> 
> Regression-tested on aarch64-linux Ubuntu 22.04/20.04.
> ---
>  gdb/corelow.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index 439270f5559..ae1641fe5d2 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -1229,6 +1229,21 @@ core_target::thread_alive (ptid_t ptid)
>  const struct target_desc *
>  core_target::read_description ()
>  {
> +  /* If the architecture provides a corefile target description hook, use
> +     it now.  Even if the core file contains a target description in a note
> +     section, it is not useful for targets that can potentially have distinct
> +     descriptions for each thread.  One example is AArch64's SVE/SME
> +     extensions that allow per-thread vector length changes, resulting in
> +     registers with different sizes.  */
> +  if (m_core_gdbarch && gdbarch_core_read_description_p (m_core_gdbarch))
> +    {
> +      const struct target_desc *result;
> +
> +      result = gdbarch_core_read_description (m_core_gdbarch, this, core_bfd);
> +      if (result != nullptr)
> +	return result;
> +    }
> +
>    /* If the core file contains a target description note then we will use
>       that in preference to anything else.  */
>    bfd_size_type tdesc_note_size = 0;
> @@ -1252,15 +1267,6 @@ core_target::read_description ()
>  	}
>      }
>  
> -  if (m_core_gdbarch && gdbarch_core_read_description_p (m_core_gdbarch))
> -    {
> -      const struct target_desc *result;
> -
> -      result = gdbarch_core_read_description (m_core_gdbarch, this, core_bfd);
> -      if (result != NULL)
> -	return result;
> -    }
> -
>    return this->beneath ()->read_description ();
>  }
>