[PATCH] Fix regcache_restore() handling of unavailable regs.

[PATCH] Fix regcache_restore() handling of unavailable regs.

[David Miller]

Across inferior dummy calls, regcache entries which were "unavailable"
were restored incorrectly after such dummy calls.  The problem is
incorrect interpretation of cooked_read()'s return value in
regcache_restore().

Ok to commit?

BTW, regcache_restore() seems to not be used outside of regcache.c
any more.  We should probably mark it static and remove the extern
from regcache.h

gdb/

	* regcache.c (regcache_restore): Do not write unavailable regs.

diff --git a/gdb/regcache.c b/gdb/regcache.c
index 37092f8..3fb6811 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -351,9 +351,10 @@ regcache_restore (struct regcache *dst,
     {
       if (gdbarch_register_reggroup_p (gdbarch, regnum, restore_reggroup))
 	{
-	  int valid = cooked_read (cooked_read_context, regnum, buf);
+	  enum register_status status;
 
-	  if (valid)
+	  status = cooked_read (cooked_read_context, regnum, buf);
+	  if (status == REG_VALID)
 	    regcache_cooked_write (dst, regnum, buf);
 	}
     }

Re: [PATCH] Fix regcache_restore() handling of unavailable regs.

[Pedro Alves]

On Tuesday 11 October 2011 07:40:27, David Miller wrote:
> 
> Across inferior dummy calls, regcache entries which were "unavailable"
> were restored incorrectly after such dummy calls.  The problem is
> incorrect interpretation of cooked_read()'s return value in
> regcache_restore().
> 
> Ok to commit?

Okay.

I don't think the problem is fully solved though.  If it happens
that the same set of registers are unavailable in src/dst,
then things work.  If if not, nothing is marking the dst
register as unavailable.  And that's tricky, due to pseudo
registers, and partially available registers.  I think this will
need a bit more surgery to fix properly.  Until then, please
apply your patch.  I'll add a TODO to think about this.

OOC, what register are you seeing as unavailable?  Is this a
case of ptrace not exposing all the machine's registers?
A context where the kernel hasn't saved all registers (a signal
frame, or something like that).  Something else?

Thanks.

-- 
Pedro Alves

Re: [PATCH] Fix regcache_restore() handling of unavailable regs.

[David Miller]

From: Pedro Alves <pedro@codesourcery.com>
Date: Tue, 11 Oct 2011 11:44:49 +0100

> OOC, what register are you seeing as unavailable?

%wim and %tbr on sparc 32-bit

> Is this a case of ptrace not exposing all the machine's registers?

Yes, ptrace() does not provide these registers.  They really aren't
relevant for analyzing userland process execution at all.

They are, however, useful when debugging an operating system over
a serial line with GDB, and the Sparc infrastructure for that does
properly provide these registers.

Re: [PATCH] Fix regcache_restore() handling of unavailable regs.

[Pedro Alves]

On Tuesday 11 October 2011 07:40:27, David Miller wrote:
> BTW, regcache_restore() seems to not be used outside of regcache.c
> any more.  We should probably mark it static and remove the extern
> from regcache.h

Forgot to reply to this.  Yes, go ahead if you'd like.  Consider it
preapproved.

-- 
Pedro Alves

Re: [PATCH] Fix regcache_restore() handling of unavailable regs.

[David Miller]

From: Pedro Alves <pedro@codesourcery.com>
Date: Tue, 11 Oct 2011 11:50:24 +0100

> On Tuesday 11 October 2011 07:40:27, David Miller wrote:
>> BTW, regcache_restore() seems to not be used outside of regcache.c
>> any more.  We should probably mark it static and remove the extern
>> from regcache.h
> 
> Forgot to reply to this.  Yes, go ahead if you'd like.  Consider it
> preapproved.

Will do, thanks for reviewing.