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

Mark Kettenis
2015-01-09T16:59:40Z

> 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.