PATCH: gdbserver: Clear regcache if buf is NULL

PATCH: gdbserver: Clear regcache if buf is NULL

[H.J. Lu]

If xstate_bv bits are zero, XSAVE extended state is in init state and
gdbserver should treat XMM/YMM registers as 0. This patch clears regcache
if buf is NULL. OK to install?

Thanks.


H.J.
---
2010-02-03  H.J. Lu  <hongjiu.lu@intel.com>

	* regcache.c (supply_register): Clear regcache if buf is NULL.

diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index 2082604..d6cdc46 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -215,7 +215,10 @@ register_data (struct regcache *regcache, int n, int fetch)
 void
 supply_register (struct regcache *regcache, int n, const void *buf)
 {
-  memcpy (register_data (regcache, n, 0), buf, register_size (n));
+  if (buf)
+    memcpy (register_data (regcache, n, 0), buf, register_size (n));
+  else
+    memset (register_data (regcache, n, 0), 0, register_size (n));
 }
 
 void

Re: PATCH: gdbserver: Clear regcache if buf is NULL

[Daniel Jacobowitz]

On Wed, Feb 03, 2010 at 09:44:14AM -0800, H.J. Lu wrote:
> If xstate_bv bits are zero, XSAVE extended state is in init state and
> gdbserver should treat XMM/YMM registers as 0. This patch clears regcache
> if buf is NULL. OK to install?

OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: gdbserver: Clear regcache if buf is NULL

[Pedro Alves]

On Wednesday 03 February 2010 17:44:14, H.J. Lu wrote:
> If xstate_bv bits are zero, XSAVE extended state is in init state and
> gdbserver should treat XMM/YMM registers as 0. This patch clears regcache
> if buf is NULL. OK to install?
> 
> Thanks.
> 
> 
> H.J.
> ---
> 2010-02-03  H.J. Lu  <hongjiu.lu@intel.com>
> 
> 	* regcache.c (supply_register): Clear regcache if buf is NULL.
> 
> diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
> index 2082604..d6cdc46 100644
> --- a/gdb/gdbserver/regcache.c
> +++ b/gdb/gdbserver/regcache.c
> @@ -215,7 +215,10 @@ register_data (struct regcache *regcache, int n, int fetch)
>  void
>  supply_register (struct regcache *regcache, int n, const void *buf)
>  {
> -  memcpy (register_data (regcache, n, 0), buf, register_size (n));
> +  if (buf)
> +    memcpy (register_data (regcache, n, 0), buf, register_size (n));
> +  else
> +    memset (register_data (regcache, n, 0), 0, register_size (n));
>  }

Hi H.J..  I'm looking at where in the current sources this
ended up being used, and not finding it.  Am I right in thinking
that the need for this was obsoleted when you later tought
gdbserver x86-linux about xml descriptions?

I'm teaching gdbserver to tell gdb about unavailable registers
in the context of tracepoint frames, and wondering whether to put
that info in the regcache for general use, or whether I can
make it tracepoint specific.

-- 
Pedro Alves

Re: PATCH: gdbserver: Clear regcache if buf is NULL

[H.J. Lu]

On Tue, Sep 28, 2010 at 1:23 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Wednesday 03 February 2010 17:44:14, H.J. Lu wrote:
>> If xstate_bv bits are zero, XSAVE extended state is in init state and
>> gdbserver should treat XMM/YMM registers as 0. This patch clears regcache
>> if buf is NULL. OK to install?
>>
>> Thanks.
>>
>>
>> H.J.
>> ---
>> 2010-02-03  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>       * regcache.c (supply_register): Clear regcache if buf is NULL.
>>
>> diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
>> index 2082604..d6cdc46 100644
>> --- a/gdb/gdbserver/regcache.c
>> +++ b/gdb/gdbserver/regcache.c
>> @@ -215,7 +215,10 @@ register_data (struct regcache *regcache, int n, int fetch)
>>  void
>>  supply_register (struct regcache *regcache, int n, const void *buf)
>>  {
>> -  memcpy (register_data (regcache, n, 0), buf, register_size (n));
>> +  if (buf)
>> +    memcpy (register_data (regcache, n, 0), buf, register_size (n));
>> +  else
>> +    memset (register_data (regcache, n, 0), 0, register_size (n));
>>  }
>
> Hi H.J..  I'm looking at where in the current sources this
> ended up being used, and not finding it.  Am I right in thinking
> that the need for this was obsoleted when you later tought
> gdbserver x86-linux about xml descriptions?
>

i387-fp.c has

  if ((x86_xcr0 & I386_XSTATE_SSE))
    {
      int xmm0_regnum = find_regno ("xmm0");

      if ((clear_bv & I386_XSTATE_SSE))
        p = NULL;
      else
        p = (char *) buf;

      for (i = 0; i < num_xmm_registers; i++)
        {
          if (p)
            p = ((char *) &fp->xmm_space[0]) + i * 16;
          supply_register (regcache, i + xmm0_regnum, p);
        }
    }



-- 
H.J.

Re: PATCH: gdbserver: Clear regcache if buf is NULL

[Pedro Alves]

On Friday 01 October 2010 02:51:07, H.J. Lu wrote:
> On Tue, Sep 28, 2010 at 1:23 PM, Pedro Alves <pedro@codesourcery.com> wrote:

> > Hi H.J..  I'm looking at where in the current sources this
> > ended up being used, and not finding it.  Am I right in thinking
> > that the need for this was obsoleted when you later tought
> > gdbserver x86-linux about xml descriptions?
> >
> 
> i387-fp.c has
> 
>   if ((x86_xcr0 & I386_XSTATE_SSE))
>     {
>       int xmm0_regnum = find_regno ("xmm0");
> 
>       if ((clear_bv & I386_XSTATE_SSE))
>         p = NULL;
>       else
>         p = (char *) buf;
> 
>       for (i = 0; i < num_xmm_registers; i++)
>         {
>           if (p)
>             p = ((char *) &fp->xmm_space[0]) + i * 16;
>           supply_register (regcache, i + xmm0_regnum, p);
>         }
>     }
> 

Thanks, and sorry as for not coming back to this promptly.
Hmm, I don't think I have a way to trigger that.
So is the registers' value really zero, or should gdb
ideally present them as not available (and zero is just a
hack)?

-- 
Pedro Alves

Re: PATCH: gdbserver: Clear regcache if buf is NULL

[H.J. Lu]

On Sat, Oct 16, 2010 at 1:00 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Friday 01 October 2010 02:51:07, H.J. Lu wrote:
>> On Tue, Sep 28, 2010 at 1:23 PM, Pedro Alves <pedro@codesourcery.com> wrote:
>
>> > Hi H.J..  I'm looking at where in the current sources this
>> > ended up being used, and not finding it.  Am I right in thinking
>> > that the need for this was obsoleted when you later tought
>> > gdbserver x86-linux about xml descriptions?
>> >
>>
>> i387-fp.c has
>>
>>   if ((x86_xcr0 & I386_XSTATE_SSE))
>>     {
>>       int xmm0_regnum = find_regno ("xmm0");
>>
>>       if ((clear_bv & I386_XSTATE_SSE))
>>         p = NULL;
>>       else
>>         p = (char *) buf;
>>
>>       for (i = 0; i < num_xmm_registers; i++)
>>         {
>>           if (p)
>>             p = ((char *) &fp->xmm_space[0]) + i * 16;
>>           supply_register (regcache, i + xmm0_regnum, p);
>>         }
>>     }
>>
>
> Thanks, and sorry as for not coming back to this promptly.
> Hmm, I don't think I have a way to trigger that.
> So is the registers' value really zero, or should gdb
> ideally present them as not available (and zero is just a
> hack)?
>

You will see it only on machines with XSAVE and enabled in OS.
Please see

13.6 XSAVE/XRSTOR AND PROCESSOR EXTENDED STATE
MANAGEMENT

in

Intel® 64 and IA-32 Architectures Software Developer’s Manual
Volume 3A: System Programming Guide, Part 1

Basically values in vector registers may not be valid, depending
on vector state.


-- 
H.J.

Re: PATCH: gdbserver: Clear regcache if buf is NULL

[Pedro Alves]

On Saturday 16 October 2010 21:16:37, H.J. Lu wrote:
> >
> > Thanks, and sorry as for not coming back to this promptly.
> > Hmm, I don't think I have a way to trigger that.
> > So is the registers' value really zero, or should gdb
> > ideally present them as not available (and zero is just a
> > hack)?
> >
> 
> You will see it only on machines with XSAVE and enabled in OS.
> Please see
> 
> 13.6 XSAVE/XRSTOR AND PROCESSOR EXTENDED STATE
> MANAGEMENT
> 
> in
> 
> Intel® 64 and IA-32 Architectures Software Developer’s Manual
> Volume 3A: System Programming Guide, Part 1
> 
> Basically values in vector registers may not be valid, depending
> on vector state.

Okay, thanks.  The changes I'm working on would make gdb print
for such registers:

 (gdb) info registers
 ...
 foo-register             *value not available*
 ...

rather than:

 (gdb) info registers
 ...
 foo-register             0x0    0
 ...

Trying to print the register's value will show something
like "$1 = <unavailable>", and using it's value in expression
evaluation to determine a computed expression value, an error
will be reported.

Does that sound good?  Or do you think keep using 0 would
be better in this case?

-- 
Pedro Alves

Re: PATCH: gdbserver: Clear regcache if buf is NULL

[H.J. Lu]

On Sat, Oct 16, 2010 at 1:49 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Saturday 16 October 2010 21:16:37, H.J. Lu wrote:
>> >
>> > Thanks, and sorry as for not coming back to this promptly.
>> > Hmm, I don't think I have a way to trigger that.
>> > So is the registers' value really zero, or should gdb
>> > ideally present them as not available (and zero is just a
>> > hack)?
>> >
>>
>> You will see it only on machines with XSAVE and enabled in OS.
>> Please see
>>
>> 13.6 XSAVE/XRSTOR AND PROCESSOR EXTENDED STATE
>> MANAGEMENT
>>
>> in
>>
>> Intel® 64 and IA-32 Architectures Software Developer’s Manual
>> Volume 3A: System Programming Guide, Part 1
>>
>> Basically values in vector registers may not be valid, depending
>> on vector state.
>
> Okay, thanks.  The changes I'm working on would make gdb print
> for such registers:
>
>  (gdb) info registers
>  ...
>  foo-register             *value not available*
>  ...
>
> rather than:
>
>  (gdb) info registers
>  ...
>  foo-register             0x0    0
>  ...
>
> Trying to print the register's value will show something
> like "$1 = <unavailable>", and using it's value in expression
> evaluation to determine a computed expression value, an error
> will be reported.
>
> Does that sound good?  Or do you think keep using 0 would
> be better in this case?
>

Values in vector registers are invalid, not unavailable. OS
initializes them to zero when they are set the firs time in
a program. I prefer *value not valid".

GDB may update vector registers before they are set by program.
GDB sets proper bits in XSAVE area to tell OS/hardware that vector
registers now have valid values.


-- 
H.J.

Re: PATCH: gdbserver: Clear regcache if buf is NULL

[Pedro Alves]

On Saturday 16 October 2010 23:02:27, H.J. Lu wrote:
> > Does that sound good?  Or do you think keep using 0 would
> > be better in this case?
> >
> 
> Values in vector registers are invalid, not unavailable. OS
> initializes them to zero when they are set the firs time in
> a program. I prefer *value not valid".

Ah, yes, agreed.  Okay, that's a different state from the
"unavailable" in traceframe's (which really means the register
exists, and its value also exists, but hasn't been collected
by the tracepoint actions, so we don't know what it was).
I'll adjust my patches to keep using 0 for now in cases like
the vector case we're discussing then.  Thanks!

> GDB may update vector registers before they are set by program.
> GDB sets proper bits in XSAVE area to tell OS/hardware that vector
> registers now have valid values.

-- 
Pedro Alves

Re: PATCH: gdbserver: Clear regcache if buf is NULL

[Mark Kettenis]

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

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Sat, 16 Oct 2010 21:49:36 +0100
> 
> On Saturday 16 October 2010 21:16:37, H.J. Lu wrote:
> > >
> > > Thanks, and sorry as for not coming back to this promptly.
> > > Hmm, I don't think I have a way to trigger that.
> > > So is the registers' value really zero, or should gdb
> > > ideally present them as not available (and zero is just a
> > > hack)?
> > >
> > 
> > You will see it only on machines with XSAVE and enabled in OS.
> > Please see
> > 
> > 13.6 XSAVE/XRSTOR AND PROCESSOR EXTENDED STATE
> > MANAGEMENT
> > 
> > in
> > 
> > Intel® 64 and IA-32 Architectures Software Developer’s Manual
> > Volume 3A: System Programming Guide, Part 1
> > 
> > Basically values in vector registers may not be valid, depending
> > on vector state.
> 
> Okay, thanks.  The changes I'm working on would make gdb print
> for such registers:
> 
>  (gdb) info registers
>  ...
>  foo-register             *value not available*
>  ...
> 
> rather than:
> 
>  (gdb) info registers
>  ...
>  foo-register             0x0    0
>  ...
> 
> Trying to print the register's value will show something
> like "$1 = <unavailable>", and using it's value in expression
> evaluation to determine a computed expression value, an error
> will be reported.
> 
> Does that sound good?  Or do you think keep using 0 would
> be better in this case?

I think that sounds good.