[PATCH] bfd: loongarch: modify NT_PRSTATUS note to 480 bytes for kernel uapi.And gdb need more protect for coredump.

Andrew Burgess aburgess@redhat.com
Sat Oct 25 12:28:36 GMT 2025
lijian1@kylinos.cn writes:

> From: lijian1 <lijian1@kylinos.cn>
>
> Signed-off-by: lijian1 <lijian1@kylinos.cn>
>
> -For loongarch64 from the kernel uapi, the sizeof (struct elf_prstatus) on Linux/LoongArch is 0x1e0(480bytes, include the padding),from kernel 6.6-6.18,keep this value.But the old value for PRSTATUS_SIZE(0x1d8) is 472bytes, this will lead to no .reg section generated.And if there is no reg sections generated, the thread list will not generate normally.So keep the value with kernel in bfd and add protection for gdb.
> ---
>  bfd/elfnn-loongarch.c | 2 +-
>  gdb/thread.c          | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index 53cdb783859..8f06fbbf12f 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -6540,7 +6540,7 @@ loongarch_elf_copy_indirect_symbol (struct bfd_link_info *info,
>    _bfd_elf_link_hash_copy_indirect (info, dir, ind);
>  }
>  
> -#define PRSTATUS_SIZE		    0x1d8
> +#define PRSTATUS_SIZE		    0x1e0
>  #define PRSTATUS_OFFSET_PR_CURSIG   0xc
>  #define PRSTATUS_OFFSET_PR_PID	    0x20
>  #define ELF_GREGSET_T_SIZE	    0x168
> diff --git a/gdb/thread.c b/gdb/thread.c
> index 472f41969cf..92908b0f78e 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -298,6 +298,9 @@ add_thread_silent (process_stratum_target *targ, ptid_t ptid)
>    gdb_assert (targ != nullptr);
>  
>    inferior *inf = find_inferior_ptid (targ, ptid);
> +  
> +  /*if there is no reg section, the inf will be null*/
> +  gdb_assert(inf != nullptr);

Comments should start with a capital letter, and with a period, and have
two spaces after that.  Variable references can be capitalised, so:

  /* If there is not reg section then INF will be NULL.  */

However, this comment indicates that the change you've made here is not
correct.  First, add_thread_silent is a general function, not just used
for core file handling, so talking about reg sections is confusing,
someone could be looking at this function for some other reason, so:

  /* If we are adding a thread from a core file, and there is no '.reg'
     section, then INF will be NULL.  */

Adds more context.  However, this is still wrong.  Asserts are not for
error handling, they are for assertion things which are known to be
true.  A core file is an outside source of data, as such it cannot be
trusted.  A corrupted core file could be missing a '.reg' section, in
fact, Tom just posted a patch:

  https://inbox.sourceware.org/gdb-patches/20251024124208.1875651-1-tdevries@suse.de

to handle a similar case.  As such, you need to identify these cases and
raise an error to tell the user that their core file is corrupted.

The GDB part of this patch isn't OK.

Thanks,
Andrew


>  
>    threads_debug_printf ("add thread to inferior %d, ptid %s, target %s",
>  			inf->num, ptid.to_string ().c_str (),
> -- 
> 2.43.0



More information about the Binutils mailing list