[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
More information about the Binutils mailing list
Sat Oct 25 12:28:36 GMT 2025
- Previous message (by thread): [PATCH] bfd: loongarch: modify NT_PRSTATUS note to 480 bytes for kernel uapi.And gdb need more protect for coredump.
- Next message (by thread): [PATCH v2 0/1] bfd/loongarch: set PRSTATUS_SIZE=0x1e0 to match kernel's struct elf_prstatus size
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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
- Previous message (by thread): [PATCH] bfd: loongarch: modify NT_PRSTATUS note to 480 bytes for kernel uapi.And gdb need more protect for coredump.
- Next message (by thread): [PATCH v2 0/1] bfd/loongarch: set PRSTATUS_SIZE=0x1e0 to match kernel's struct elf_prstatus size
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
More information about the Binutils mailing list