Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit core file

[Mark Kettenis <mark.kettenis@xs4all.nl>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2964 bytes --]

> Date: Mon, 12 Apr 2010 11:50:47 -0700
> From: "H.J. Lu" <hjl.tools@gmail.com>
> 
> On Mon, Apr 12, 2010 at 11:23 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >> Date: Mon, 12 Apr 2010 06:22:25 -0700
> >> From: "H.J. Lu" <hongjiu.lu@intel.com>
> >>
> >> On Sun, Apr 11, 2010 at 01:52:50PM -0700, H.J. Lu wrote:
> >> > Hi,
> >> >
> >> > Thanks for Mark's pointer. Solution is very simple. We just need to
> >> > make sure that we call the right fill_gregset for 32bit executable
> >> > on both Linux/x86-64 and Linux/i386.  OK to install?
> >> >
> >> > Thanks.
> >> >
> >> >
> >>
> >> Small update to use tdep->gregset_reg_offset instead of
> >> i386_linux_gregset_reg_offset.  OK to install?
> >
> > No.  Like I explained in an earlier mail, we're not supposed to end up
> > in fetch_gregset() in the first place.
> >
> 
> fetch_gregset is always defined and used to fill .reg section in
> coredump on Linux/x86. i386-tdep.c has

No, that's not how it's supposed to be.  And unless there is a bug
somewhere, this is not what happens for the 64x64 and 32x32 cases.

> const struct regset *
> i386_regset_from_core_section (struct gdbarch *gdbarch,
>                                const char *sect_name, size_t sect_size)
> {
>   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> 
>   if (strcmp (sect_name, ".reg") == 0 && sect_size == tdep->sizeof_gregset)
>     {
>       if (tdep->gregset == NULL)
>         tdep->gregset = regset_alloc (gdbarch, i386_supply_gregset,
>                                       i386_collect_gregset);
>       return tdep->gregset;
>     }
> 
> For Linux,  tdep->sizeof_gregset != the size of .reg section.

Then there is your bug.

> In fact, they have nothing to do with each other. sizeof_gregset
> includes SSE and AVX registers, which have offset == -1 since they
> aren't general-purpose registers. I don't believe we should set
> tdep->gregset since general-purpose registers is a special case for
> Linux/x86. They are handled differently. I don't want to change it.

You're wrong here.  This code used to work just fine.  And I believe
it still works fine for the 32x32 and 64x64 cases.

I see that in i386-linux-tdep.c:i386_linux_regset_sections[] specifies
the size of the regset as 144 and sets tdep->sizeof_gregset to 17 * 4
= 68.  That can't be right.  Given that
amd64-linux-tdep.c:amd64_linux_regset_sections[] specifies the size as
144 as well, I'm betting 68 is the right valaue for i386.  I think
somebody got confused here, since the size of the NT_PRSTATUS note in
32-bit core dumps happens to be 144, but the actual size of the space
reserved for storing the registers in there is smaller.

Anyway, there is a somewhat fundamental flaw in
linux-nat.c:linux_nat_do_thread_registers() in that it always passes
the size of 64-bit version of gregset_t in the
gdbarch_regset_from_core_section() call.  That's wrong, and probably
the ultimate reason why the 64x32 gcore case isn't working properly.