Re: [testsuite patch] for: [PATCH] [PR corefiles/17808] i386: Fix internal error when prstatus in core file is too big

[Mark Kettenis <mark dot kettenis at xs4all dot nl>]

> Date: Fri, 09 Jan 2015 16:27:12 +0000
> From: Pedro Alves <palves@redhat.com>
> 
> > Any other comments?
> 
> Do we need to do the same in other places?  This grep seems to suggest yes:
> 
> $ grep assert * | grep sizeof | grep regset
> amd64obsd-tdep.c:  gdb_assert (len >= tdep->sizeof_gregset + I387_SIZEOF_FXSAVE);
> amd64-tdep.c:  gdb_assert (len == tdep->sizeof_fpregset);
> amd64-tdep.c:  gdb_assert (len == tdep->sizeof_fpregset);
> i386obsd-tdep.c:  gdb_assert (len >= tdep->sizeof_gregset + I387_SIZEOF_FSAVE);
> i386-tdep.c:  gdb_assert (len == tdep->sizeof_gregset);
> i386-tdep.c:  gdb_assert (len == tdep->sizeof_gregset);
> i386-tdep.c:  gdb_assert (len == tdep->sizeof_fpregset);
> i386-tdep.c:  gdb_assert (len == tdep->sizeof_fpregset);
> mips-linux-tdep.c:  gdb_assert (len == sizeof (mips_elf_gregset_t));
> mips-linux-tdep.c:  gdb_assert (len == sizeof (mips_elf_gregset_t));
> mips-linux-tdep.c:  gdb_assert (len == sizeof (mips_elf_fpregset_t));
> mips-linux-tdep.c:  gdb_assert (len == sizeof (mips_elf_fpregset_t));
> mips-linux-tdep.c:  gdb_assert (len == sizeof (mips64_elf_gregset_t));
> mips-linux-tdep.c:  gdb_assert (len == sizeof (mips64_elf_gregset_t));
> mips-linux-tdep.c:  gdb_assert (len == sizeof (mips64_elf_fpregset_t));
> mips-linux-tdep.c:  gdb_assert (len == sizeof (mips64_elf_fpregset_t));
> mn10300-linux-tdep.c:  gdb_assert (len == sizeof (mn10300_elf_gregset_t));
> mn10300-linux-tdep.c:  gdb_assert (len == sizeof (mn10300_elf_fpregset_t));
> mn10300-linux-tdep.c:  gdb_assert (len == sizeof (mn10300_elf_gregset_t));
> 
> On 01/08/2015 04:16 PM, Andreas Arnez wrote:
> > Note that this behavior deviates from the default policy: In general, if
> > some future kernel adds new registers to a register set, then a GDB
> > unaware of this extension would read the known subset and just ignore
> > the unknown bytes.
> 
> That's a good point.
> 
> get_core_register_section checks the section size already:
> 
> get_core_register_section (struct regcache *regcache,
> 			   const struct regset *regset,
> 			   const char *name,
> 			   int min_size,
> 			   int which,
> 			   const char *human_name,
> 			   int required)
> {
> ...
>   size = bfd_section_size (core_bfd, section);
>   if (size < min_size)
>     {
>       warning (_("Section `%s' in core file too small."), section_name);
>       return;
>     }
> ...
> 
> Should we remove all those asserts, and make it the
> job of get_core_register_section to warn if the section
> size is bigger than expected?  We may need to pass
> the "expected" section size to the callback, in addition
> to the "minimum" size though.

The code is designed to allow these sections to grow such that the OS
kernel can add more registers without breaking GDB.