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 20:11:04 +0000
> From: Pedro Alves <palves@redhat.com>
> 
> On 01/09/2015 07:35 PM, Mark Kettenis wrote:
> >> Date: Fri, 09 Jan 2015 17:19:14 +0000
> >> From: Pedro Alves <palves@redhat.com>
> >>
> >> On 01/09/2015 04:59 PM, Mark Kettenis wrote:
> >>>> 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.
> >>
> >> Not sure what you're disagreeing with.  My comment is in that direction
> >> too (And Andreas' comment I'm quoting).  That is, get_core_register_section
> >> would warn, but still continue processing the section.
> >>
> >> The current code clearly does not work that way, given the assertions.
> > 
> > It shouldn't warn if the sections is bigger that "expected", because
> > in some cases the "expected" size is really the minimum supported
> > size, where later versions of the OS added extra information.  At
> > least not unconditionally.
> 
> I think we're saying the same thing, but what I'm calling "expected",
> you're calling "maximum".  As in, consider the case where GDB
> about a regset section that is supposed to have size A.  GDB is taught
> about this, with "minimum" == A, and "expected/maximum" == A.  Later at
> some point, a new variant of the machine appears with more registers, and
> the regset is extended, to size B.  A GDB that only knows about A encounters
> a core dump with B, and thus issues a warning (which suggests that either
> more info is available that gdb doesn't grok, or the core is broken), but still
> presents the A registers to the user.  Later, someone teaches GDB about B
> registers, and at that point, "minimum" stays A, but "expected/maximum" is
> set to B.  At some point, if the regset is extended further to C, a GDB
> that knows about A and B warns when it sees C.  And on and on.  I think
> we've already seen something like that with the x86 xsave regset?

Yes, the x86 "FPU" register set certainly is an example I had in mind.
It all started when SSE was introduced.

There are also some BSD's where during the transition from a.out to
ELF the floating-point registers were seperated out into their own
section.  In that case the section actually shrunk and the minmum size
was adjusted.