[PATCH] %g0 on sparc should always be available

[PATCH] %g0 on sparc should always be available

[Mark Kettenis]

Many RISC CPUs have a register that's hardwired to 0.  Typically GDB
code uses regcache_raw_supply(..., ..., NULL) to "transfer" the value
of such a register.  However these days, that marks the register as
unavailable.  This diff fixes that for %g0 on SPARC.

Committed as obvious.


2011-05-21  Mark Kettenis  <kettenis@gnu.org>

	* sparc-nat.c (sparc_fetch_inferior_registers): Explicitly supply
	zero as the value for %g0 in the register cache.
	* sparc-tdep.c (sparc32_supply_gregset): Likewise.
	* sparc64-tdep.c (sparc64_supply_gregset): Likewise.

Index: sparc-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc-nat.c,v
retrieving revision 1.43
diff -u -p -r1.43 sparc-nat.c
--- sparc-nat.c	1 Jan 2011 15:33:15 -0000	1.43
+++ sparc-nat.c	21 May 2011 19:08:34 -0000
@@ -159,7 +159,9 @@ sparc_fetch_inferior_registers (struct t
 
   if (regnum == SPARC_G0_REGNUM)
     {
-      regcache_raw_supply (regcache, SPARC_G0_REGNUM, NULL);
+      gdb_byte zero[8] = { 0 };
+
+      regcache_raw_supply (regcache, SPARC_G0_REGNUM, &zero);
       return;
     }
 
Index: sparc-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc-tdep.c,v
retrieving revision 1.219
diff -u -p -r1.219 sparc-tdep.c
--- sparc-tdep.c	18 Mar 2011 18:52:32 -0000	1.219
+++ sparc-tdep.c	21 May 2011 19:08:34 -0000
@@ -1618,6 +1618,7 @@ sparc32_supply_gregset (const struct spa
 			int regnum, const void *gregs)
 {
   const gdb_byte *regs = gregs;
+  gdb_byte zero[4] = { 0 };
   int i;
 
   if (regnum == SPARC32_PSR_REGNUM || regnum == -1)
@@ -1637,7 +1638,7 @@ sparc32_supply_gregset (const struct spa
 			 regs + gregset->r_y_offset);
 
   if (regnum == SPARC_G0_REGNUM || regnum == -1)
-    regcache_raw_supply (regcache, SPARC_G0_REGNUM, NULL);
+    regcache_raw_supply (regcache, SPARC_G0_REGNUM, &zero);
 
   if ((regnum >= SPARC_G1_REGNUM && regnum <= SPARC_O7_REGNUM) || regnum == -1)
     {
Index: sparc64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc64-tdep.c,v
retrieving revision 1.53
diff -u -p -r1.53 sparc64-tdep.c
--- sparc64-tdep.c	18 Mar 2011 18:52:32 -0000	1.53
+++ sparc64-tdep.c	21 May 2011 19:08:35 -0000
@@ -1206,6 +1206,7 @@ sparc64_supply_gregset (const struct spa
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int sparc32 = (gdbarch_ptr_bit (gdbarch) == 32);
   const gdb_byte *regs = gregs;
+  gdb_byte zero[8] = { 0 };
   int i;
 
   if (sparc32)
@@ -1268,7 +1269,7 @@ sparc64_supply_gregset (const struct spa
     }
 
   if (regnum == SPARC_G0_REGNUM || regnum == -1)
-    regcache_raw_supply (regcache, SPARC_G0_REGNUM, NULL);
+    regcache_raw_supply (regcache, SPARC_G0_REGNUM, &zero);
 
   if ((regnum >= SPARC_G1_REGNUM && regnum <= SPARC_O7_REGNUM) || regnum == -1)
     {

Re: [PATCH] %g0 on sparc should always be available

[Pedro Alves]

On Saturday 21 May 2011 20:19:49, Mark Kettenis wrote:
> Many RISC CPUs have a register that's hardwired to 0.  Typically GDB
> code uses regcache_raw_supply(..., ..., NULL) to "transfer" the value
> of such a register.  However these days, that marks the register as
> unavailable.  This diff fixes that for %g0 on SPARC.
> 
> Committed as obvious.

Thanks!  The <unavailable> stuff is going into 7.3 as well.  Want
to put the patch there too?

(I see alpha-tdep.c needs the same treatment for ALPHA_ZERO_REGNUM.)

-- 
Pedro Alves

$zero on alpha should always be available

[Pedro Alves]

On Sunday 22 May 2011 12:17:12, Pedro Alves wrote:
> On Saturday 21 May 2011 20:19:49, Mark Kettenis wrote:
> > Many RISC CPUs have a register that's hardwired to 0.  Typically GDB
> > code uses regcache_raw_supply(..., ..., NULL) to "transfer" the value
> > of such a register.  However these days, that marks the register as
> > unavailable.  This diff fixes that for %g0 on SPARC.
> > 
> > Committed as obvious.
> 
> Thanks!  The <unavailable> stuff is going into 7.3 as well.  Want
> to put the patch there too?
> 
> (I see alpha-tdep.c needs the same treatment for ALPHA_ZERO_REGNUM.)

Here's what I came up with by inspection.  I have no alpha machine to test
this on.  The alpha-nat.c change would need
checking on an alpha running OSF if we wanted to test this...


Pedro Alves

2011-05-23  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	* alpha-tdep.c (alpha_cannot_fetch_register): Don't return true
	for ALPHA_ZERO_REGNUM.
	(alpha_supply_int_regs): Explicitly supply zero as the value for
	ALPHA_ZERO_REGNUM in the register cache.
	* alpha-nat.c (fetch_osf_core_registers): Ditto.

---
 gdb/alpha-nat.c  |    9 +++++++++
 gdb/alpha-tdep.c |    9 ++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

Index: src/gdb/alpha-tdep.c
===================================================================
--- src.orig/gdb/alpha-tdep.c	2011-04-20 16:22:32.000000000 +0100
+++ src/gdb/alpha-tdep.c	2011-05-23 18:24:29.448753681 +0100
@@ -111,8 +111,7 @@ alpha_register_name (struct gdbarch *gdb
 static int
 alpha_cannot_fetch_register (struct gdbarch *gdbarch, int regno)
 {
-  return (regno == ALPHA_ZERO_REGNUM
-          || strlen (alpha_register_name (gdbarch, regno)) == 0);
+  return (strlen (alpha_register_name (gdbarch, regno)) == 0);
 }
 
 static int
@@ -1424,7 +1423,11 @@ alpha_supply_int_regs (struct regcache *
       regcache_raw_supply (regcache, i, regs + i * 8);
 
   if (regno == ALPHA_ZERO_REGNUM || regno == -1)
-    regcache_raw_supply (regcache, ALPHA_ZERO_REGNUM, NULL);
+    {
+      const gdb_byte zero[8] = { 0 };
+
+      regcache_raw_supply (regcache, ALPHA_ZERO_REGNUM, zero);
+    }
 
   if (regno == ALPHA_PC_REGNUM || regno == -1)
     regcache_raw_supply (regcache, ALPHA_PC_REGNUM, pc);
Index: src/gdb/alpha-nat.c
===================================================================
--- src.orig/gdb/alpha-nat.c	2011-01-13 15:07:16.000000000 +0000
+++ src/gdb/alpha-nat.c	2011-05-23 18:28:14.428753680 +0100
@@ -101,6 +101,15 @@ fetch_osf_core_registers (struct regcach
 	  regcache_raw_supply (regcache, regno, NULL);
 	  continue;
 	}
+
+      if (regno == ALPHA_ZERO_REGNUM)
+	{
+	  const gdb_byte zero[8] = { 0 };
+
+	  regcache_raw_supply (regcache, regno, zero);
+	  continue;
+	}
+
       addr = 8 * core_reg_mapping[regno];
       if (addr < 0 || addr >= core_reg_size)
 	{

Re: $zero on alpha should always be available

[Joel Brobecker]

> Here's what I came up with by inspection.  I have no alpha machine to test
> this on.  The alpha-nat.c change would need
> checking on an alpha running OSF if we wanted to test this...

Thanks, Pedro.  I will try to test this tomorrow. I think Tru64 is
one of the systems where expect or tcl no longer work for me, so
I might not be able to run the testsuite. But worse case scenario,
I can try running it against AdaCore's testsuite.

> 2011-05-23  Pedro Alves  <pedro@codesourcery.com>
> 
> 	gdb/
> 	* alpha-tdep.c (alpha_cannot_fetch_register): Don't return true
> 	for ALPHA_ZERO_REGNUM.
> 	(alpha_supply_int_regs): Explicitly supply zero as the value for
> 	ALPHA_ZERO_REGNUM in the register cache.
> 	* alpha-nat.c (fetch_osf_core_registers): Ditto.

-- 
Joel

Re: $zero on alpha should always be available

[Pedro Alves]

On Tuesday 24 May 2011 05:44:18, Joel Brobecker wrote:
> > Here's what I came up with by inspection.  I have no alpha machine to test
> > this on.  The alpha-nat.c change would need
> > checking on an alpha running OSF if we wanted to test this...
> 
> Thanks, Pedro.  I will try to test this tomorrow. I think Tru64 is
> one of the systems where expect or tcl no longer work for me, so
> I might not be able to run the testsuite. But worse case scenario,
> I can try running it against AdaCore's testsuite.

Thanks Joel.  Even if you can't run any testsuite,
confirming that nothing broke in an obvious way, and
that $zero appears as 0 to 'p $zero' or "info all-registers",
and not as <unavailable> with a running program would be a good
test already.  The alpha-nat.c change is trying to make 
the same fix for loading alpha-OSF cores.

> 
> > 2011-05-23  Pedro Alves  <pedro@codesourcery.com>
> > 
> > 	gdb/
> > 	* alpha-tdep.c (alpha_cannot_fetch_register): Don't return true
> > 	for ALPHA_ZERO_REGNUM.
> > 	(alpha_supply_int_regs): Explicitly supply zero as the value for
> > 	ALPHA_ZERO_REGNUM in the register cache.
> > 	* alpha-nat.c (fetch_osf_core_registers): Ditto.
> 
> 

-- 
Pedro Alves

Re: $zero on alpha should always be available

[Joel Brobecker]

Hi Pedro,

> Thanks Joel.  Even if you can't run any testsuite,
> confirming that nothing broke in an obvious way, and
> that $zero appears as 0 to 'p $zero' or "info all-registers",
> and not as <unavailable> with a running program would be a good
> test already.  The alpha-nat.c change is trying to make 
> the same fix for loading alpha-OSF cores.

It turned out to be a little more effort than usual to test the change,
because we no longer do builds on Tru64, and one of the machines
I used to use broke down.

That being said, I was able to test the debugger, both with AdaCore's
testsuite as well as the manual testing that suggested, and it all
works marvelously :-).

Thanks for taking care of that...

-- 
Joel

Re: $zero on alpha should always be available

[Pedro Alves]

On Wednesday 25 May 2011 21:51:39, Joel Brobecker wrote:
> Hi Pedro,
> 
> > Thanks Joel.  Even if you can't run any testsuite,
> > confirming that nothing broke in an obvious way, and
> > that $zero appears as 0 to 'p $zero' or "info all-registers",
> > and not as <unavailable> with a running program would be a good
> > test already.  The alpha-nat.c change is trying to make 
> > the same fix for loading alpha-OSF cores.
> 
> It turned out to be a little more effort than usual to test the change,
> because we no longer do builds on Tru64, and one of the machines
> I used to use broke down.

Sorry about the extra work.

> That being said, I was able to test the debugger, both with AdaCore's
> testsuite as well as the manual testing that suggested, and it all
> works marvelously :-).
> 
> Thanks for taking care of that...

Thanks!  I've applied it to mainline and 7.3 (given it was a
regression).

-- 
Pedro Alves