Re: AVX and unavailable registers, fix system-gcore.exp

["H.J. Lu" <hjl.tools@gmail.com>]

On Thu, Jan 26, 2012 at 11:07 AM, Pedro Alves <palves@redhat.com> wrote:
> I now have access to a machine with AVX, and I'm seeing a few core related
> failures on native x86_64 Fedora 16.  GDBserver doesn't show the failures because
> it treats this issue already like the patch below proposes.
>
> This was discussed a while ago, in the "RFC: partially available registers"
> thread:
>
>  http://old.nabble.com/RFC%3A-partially-available-registers-td32055573i20.html
>
> The failures I'm seeing look like, e.g.:
>
> FAIL: gdb.arch/system-gcore.exp: corefile restored all registers
>
> because before the core, the program showed:
>
> ...
> ymm0           *value not available*
> ...
>
> and when we load back the core we see:
>
> ...
> ymm0           {v8_float = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, v4_double = {0x0, 0x0, 0x0, 0x0}, v32_int8 = {0x0 <repeats 32 time
> s>}, v16_int16 = {0x0 <repeats 16 times>}, v8_int32 = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, v4_int64 = {0x0, 0x0, 0x0, 0x0}, v2_int
> 128 = {0x00000000000000000000000000000000, 0x00000000000000000000000000000000}}
> ...
>
>
> I think we should make live debugging present the still-not-touched x87
> states as zero, as described in the comment in the patch:
>
> +  /* With the delayed xsave mechanism, in between the program
> +     starting, and the program accessing the vector registers for the
> +     first time, the register's values are invalid.  The kernel
> +     initializes register states to zero when they are set the first
> +     time in a program.  This means that from the user-space programs'
> +     perspective, it's the same as if the registers have always been
> +     zero from the start of the program.  Therefore, the debugger
> +     should provide the same illusion to the user.
>
> This is already handled this way with gdbserver, hence we don't see
> these failures there (the gcore tests now work against gdbserver).
> The kernel is also putting the registers touched as zero
> in the core, otherwise we'd see *unavailable* when we loaded the
> core file back.
>
>
> I have a doubt in the xsave-in-corefile support bits.  There's code in place to
> handle a NULL regs (as in no xsave contents to work with), so I'm handling it
> as presently:
>
> +
> +     Note however, the case when REGS is NULL is a different case.
> +     That case means we do not have access to the x87 states, so we
> +     should mark the registers as unavailable (by supplying NULL).  */
> +
>
> but I can't figure out how would we ever get a NULL REGS there.  Is there a
> convoluted path I missed?  amd64-linux-tdep.c unconditionally installs
> amd64_linux_regset_sections as gdbarch_core_regset_sections
> callback, and this includes the .reg-xstate section.
> However, corelow.c:get_core_register_section bails early if
> a section is not found in the core, never reaching regset->supply_regset
> with a NULL `contents'.
>
> Mark, maybe you have a clue?
>
> --
> Pedro Alves
>
> 2012-01-26  Pedro Alves  <palves@redhat.com>
>
>        * i387-tdep.c (i387_supply_xsave): If we have an xsave buffer, and
>        the register state is clear, supply explicit zero, instead of
>        marking the register unavailable.

This fixes 7.3/7.4 regression:

http://www.sourceware.org/bugzilla/show_bug.cgi?id=13766

> ---
>
>  gdb/i387-tdep.c |   87 ++++++++++++++++++++++++++++++++++---------------------
>  1 files changed, 54 insertions(+), 33 deletions(-)
>
> diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
> index 450b8f2..b3adb74 100644
> --- a/gdb/i387-tdep.c
> +++ b/gdb/i387-tdep.c
> @@ -725,6 +725,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
>   const gdb_byte *regs = xsave;
>   int i;
>   unsigned int clear_bv;
> +  const gdb_byte zero[MAX_REGISTER_SIZE] = { 0 };

Shouldn't it be static since it is never modified?

>   const gdb_byte *p;
>   enum
>     {
> @@ -764,6 +765,19 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
>   else
>     clear_bv = I386_XSTATE_AVX_MASK;
>
> +  /* With the delayed xsave mechanism, in between the program
> +     starting, and the program accessing the vector registers for the
> +     first time, the register's values are invalid.  The kernel
> +     initializes register states to zero when they are set the first
> +     time in a program.  This means that from the user-space programs'
> +     perspective, it's the same as if the registers have always been
> +     zero from the start of the program.  Therefore, the debugger
> +     should provide the same illusion to the user.
> +
> +     Note however, the case when REGS is NULL is a different case.
> +     That case means we do not have access to the x87 states, so we
> +     should mark the registers as unavailable (by supplying NULL).  */
> +
>   switch (regclass)
>     {
>     case none:
> @@ -771,26 +785,26 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
>
>     case avxh:
>       if ((clear_bv & I386_XSTATE_AVX))
> -       p = NULL;
> +       regcache_raw_supply (regcache, regnum, regs == NULL ? NULL : zero);
>       else
> -       p = XSAVE_AVXH_ADDR (tdep, regs, regnum);
> -      regcache_raw_supply (regcache, regnum, p);
> +       regcache_raw_supply (regcache, regnum,
> +                            XSAVE_AVXH_ADDR (tdep, regs, regnum));
>       return;
>
>     case sse:
>       if ((clear_bv & I386_XSTATE_SSE))
> -       p = NULL;
> +       regcache_raw_supply (regcache, regnum, regs == NULL ? NULL : zero);
>       else
> -       p = FXSAVE_ADDR (tdep, regs, regnum);
> -      regcache_raw_supply (regcache, regnum, p);
> +       regcache_raw_supply (regcache, regnum,
> +                            FXSAVE_ADDR (tdep, regs, regnum));
>       return;
>
>     case x87:
>       if ((clear_bv & I386_XSTATE_X87))
> -       p = NULL;
> +       regcache_raw_supply (regcache, regnum, regs == NULL ? NULL : zero);
>       else
> -       p = FXSAVE_ADDR (tdep, regs, regnum);
> -      regcache_raw_supply (regcache, regnum, p);
> +       regcache_raw_supply (regcache, regnum,
> +                            FXSAVE_ADDR (tdep, regs, regnum));
>       return;
>
>     case all:
> @@ -798,16 +812,19 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
>       if ((tdep->xcr0 & I386_XSTATE_AVX))
>        {
>          if ((clear_bv & I386_XSTATE_AVX))
> -           p = NULL;
> +           {
> +             for (i = I387_YMM0H_REGNUM (tdep);
> +                  i < I387_YMMENDH_REGNUM (tdep);
> +                  i++)
> +               regcache_raw_supply (regcache, i, regs == NULL ? NULL : zero);
> +           }
>          else
> -           p = regs;
> -
> -         for (i = I387_YMM0H_REGNUM (tdep);
> -              i < I387_YMMENDH_REGNUM (tdep); i++)
>            {
> -             if (p != NULL)
> -               p = XSAVE_AVXH_ADDR (tdep, regs, i);
> -             regcache_raw_supply (regcache, i, p);
> +             for (i = I387_YMM0H_REGNUM (tdep);
> +                  i < I387_YMMENDH_REGNUM (tdep);
> +                  i++)
> +               regcache_raw_supply (regcache, i,
> +                                    XSAVE_AVXH_ADDR (tdep, regs, i));
>            }
>        }
>
> @@ -815,16 +832,18 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
>       if ((tdep->xcr0 & I386_XSTATE_SSE))
>        {
>          if ((clear_bv & I386_XSTATE_SSE))
> -           p = NULL;
> +           {
> +             for (i = I387_XMM0_REGNUM (tdep);
> +                  i < I387_MXCSR_REGNUM (tdep);
> +                  i++)
> +               regcache_raw_supply (regcache, i, regs == NULL ? NULL : zero);
> +           }
>          else
> -           p = regs;
> -
> -         for (i = I387_XMM0_REGNUM (tdep);
> -              i < I387_MXCSR_REGNUM (tdep); i++)
>            {
> -             if (p != NULL)
> -               p = FXSAVE_ADDR (tdep, regs, i);
> -             regcache_raw_supply (regcache, i, p);
> +             for (i = I387_XMM0_REGNUM (tdep);
> +                  i < I387_MXCSR_REGNUM (tdep); i++)
> +               regcache_raw_supply (regcache, i,
> +                                    FXSAVE_ADDR (tdep, regs, i));
>            }
>        }
>
> @@ -832,16 +851,18 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
>       if ((tdep->xcr0 & I386_XSTATE_X87))
>        {
>          if ((clear_bv & I386_XSTATE_X87))
> -           p = NULL;
> +           {
> +             for (i = I387_ST0_REGNUM (tdep);
> +                  i < I387_FCTRL_REGNUM (tdep);
> +                  i++)
> +               regcache_raw_supply (regcache, i, regs != NULL ? zero : NULL);
> +           }
>          else
> -           p = regs;
> -
> -         for (i = I387_ST0_REGNUM (tdep);
> -              i < I387_FCTRL_REGNUM (tdep); i++)
>            {
> -             if (p != NULL)
> -               p = FXSAVE_ADDR (tdep, regs, i);
> -             regcache_raw_supply (regcache, i, p);
> +             for (i = I387_ST0_REGNUM (tdep);
> +                  i < I387_FCTRL_REGNUM (tdep);
> +                  i++)
> +               regcache_raw_supply (regcache, i, FXSAVE_ADDR (tdep, regs, i));
>            }
>        }
>       break;



-- 
H.J.