Re: [rfc/rft] ppc gdbserver: autodetect AltiVec and SPE

[Daniel Jacobowitz <drow at false dot org>]

On Thu, Apr 17, 2008 at 01:28:01PM -0400, Daniel Jacobowitz wrote:
> This doesn't seem to be true.
> 
> gdbserver: Unknown register fpscr requested
> 
> There's no code to use PPC_PTRACE_GETREGS, so we do in fact reach
> here.  I'm retesting with
> 
>   /* Some kernels do not allow us to store fpscr.  */
>   if (!(ppc_hwcap & PPC_FEATURE_HAS_SPE) && regno == find_regno ("fpscr"))
>     return 2;

Unfortunately that didn't fix it.

FAIL -> PASS: default/gdb.sum:gdb.threads/linux-dp.exp: continue to
breakpoint: about to create philosopher: 2
PASS -> FAIL: te500v2/gdb.sum:gdb.base/store.exp: upvar double l;
print old r, expecting -2
PASS -> FAIL: te500v2/gdb.sum:gdb.base/store.exp: var double l; print
old r, expecting -2
PASS -> FAIL: te500v2/gdb.sum:gdb.base/store.exp: var doublest l;
print old r, expecting -2
PASS -> FAIL: te500v2/gdb.sum:gdb.threads/linux-dp.exp: continue to
breakpoint: about to create philosopher: 4
PASS -> FAIL: te600/gdb.sum:gdb.threads/linux-dp.exp: continue to
breakpoint: about to create philosopher: 1
PASS -> FAIL: te600/gdb.sum:gdb.threads/linux-dp.exp: continue to
breakpoint: about to create philosopher: 2

The linux-dp.exp failures are noise; I have a patch.  The store.exp
failures mean that SPE registers aren't working.

It looks like we've fallen afoul of the code which fetches the
first register set, then if it is not available does not try any
further register sets.  So when we try PTRACE_GETVRREGS and fail
we never try PTRACE_GETEVRREGS.

This code's bitten me before.  It's even worse now, because if which
ptrace operations succeed depends on the inferior's mode, the values
may change during multi.  So I've pulled it out.  The attached patch,
combined with yours, produces no regressions on any of (SPE, AltiVec,
classic PowerPC).

Want to check them both in?

-- 
Daniel Jacobowitz
CodeSourcery

2008-04-17  Daniel Jacobowitz  <dan@codesourcery.com>

	* linux-ppc-low.c (ppc_cannot_store_register): Skip for SPE.
	* linux-low.c (disabled_regsets, num_regsets): New.
	(use_regsets_p): Delete.
	(linux_wait_for_process): Clear disabled_regsets.
	(regsets_fetch_inferior_registers): Check and set it.
	(regsets_store_inferior_registers): Likewise.
	(linux_fetch_registers, linux_store_registers): Do not use
	use_regsets_p.
	(initialize_low): Allocate disabled_regsets.

2008-04-17  Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.threads/linux-dp.exp: Continue after unrecognized lines.

diff -u gdb-head/gdb/gdbserver/linux-ppc-low.c src/gdb/gdbserver/linux-ppc-low.c
--- gdb-head/gdb/gdbserver/linux-ppc-low.c	2008-03-26 20:06:45.447120208 +0100
+++ src/gdb/gdbserver/linux-ppc-low.c	17 Apr 2008 17:27:32 -0000
@@ -116,7 +116,7 @@
 {
 #ifndef __powerpc64__
   /* Some kernels do not allow us to store fpscr.  */
-  if (regno == find_regno ("fpscr"))
+  if (!(ppc_hwcap & PPC_FEATURE_HAS_SPE) && regno == find_regno ("fpscr"))
     return 2;
 #endif
 
only in patch2:
unchanged:
--- src/gdb/gdbserver/linux-low.c	28 Feb 2008 05:54:09 -0000	1.76
+++ src/gdb/gdbserver/linux-low.c	17 Apr 2008 18:16:18 -0000
@@ -131,7 +131,8 @@ struct pending_signals
 #define PTRACE_XFER_TYPE long
 
 #ifdef HAVE_LINUX_REGSETS
-static int use_regsets_p = 1;
+static char *disabled_regsets;
+static int num_regsets;
 #endif
 
 #define pid_of(proc) ((proc)->head.id)
@@ -631,6 +632,9 @@ retry:
   if (new_inferior)
     {
       the_low_target.arch_setup ();
+#ifdef HAVE_LINUX_REGSETS
+      memset (disabled_regsets, 0, num_regsets);
+#endif
       new_inferior = 0;
     }
 
@@ -1496,7 +1500,7 @@ regsets_fetch_inferior_registers ()
       void *buf;
       int res;
 
-      if (regset->size == 0)
+      if (regset->size == 0 || disabled_regsets[regset - target_regsets])
 	{
 	  regset ++;
 	  continue;
@@ -1508,18 +1512,10 @@ regsets_fetch_inferior_registers ()
 	{
 	  if (errno == EIO)
 	    {
-	      /* If we get EIO on the first regset, do not try regsets again.
-		 If we get EIO on a later regset, disable that regset.  */
-	      if (regset == target_regsets)
-		{
-		  use_regsets_p = 0;
-		  return -1;
-		}
-	      else
-		{
-		  regset->size = 0;
-		  continue;
-		}
+	      /* If we get EIO on a regset, do not try it again for
+		 this process.  */
+	      disabled_regsets[regset - target_regsets] = 1;
+	      continue;
 	    }
 	  else
 	    {
@@ -1553,7 +1549,7 @@ regsets_store_inferior_registers ()
       void *buf;
       int res;
 
-      if (regset->size == 0)
+      if (regset->size == 0 || disabled_regsets[regset - target_regsets])
 	{
 	  regset ++;
 	  continue;
@@ -1579,18 +1575,10 @@ regsets_store_inferior_registers ()
 	{
 	  if (errno == EIO)
 	    {
-	      /* If we get EIO on the first regset, do not try regsets again.
-		 If we get EIO on a later regset, disable that regset.  */
-	      if (regset == target_regsets)
-		{
-		  use_regsets_p = 0;
-		  return -1;
-		}
-	      else
-		{
-		  regset->size = 0;
-		  continue;
-		}
+	      /* If we get EIO on a regset, do not try it again for
+		 this process.  */
+	      disabled_regsets[regset - target_regsets] = 1;
+	      continue;
 	    }
 	  else
 	    {
@@ -1616,11 +1604,8 @@ void
 linux_fetch_registers (int regno)
 {
 #ifdef HAVE_LINUX_REGSETS
-  if (use_regsets_p)
-    {
-      if (regsets_fetch_inferior_registers () == 0)
-	return;
-    }
+  if (regsets_fetch_inferior_registers () == 0)
+    return;
 #endif
 #ifdef HAVE_LINUX_USRREGS
   usr_fetch_inferior_registers (regno);
@@ -1631,11 +1616,8 @@ void
 linux_store_registers (int regno)
 {
 #ifdef HAVE_LINUX_REGSETS
-  if (use_regsets_p)
-    {
-      if (regsets_store_inferior_registers () == 0)
-	return;
-    }
+  if (regsets_store_inferior_registers () == 0)
+    return;
 #endif
 #ifdef HAVE_LINUX_USRREGS
   usr_store_inferior_registers (regno);
@@ -2084,4 +2066,9 @@ initialize_low (void)
 		       the_low_target.breakpoint_len);
   linux_init_signals ();
   linux_test_for_tracefork ();
+#ifdef HAVE_LINUX_REGSETS
+  for (num_regsets = 0; target_regsets[num_regsets].size >= 0; num_regsets++)
+    ;
+  disabled_regsets = malloc (num_regsets);
+#endif
 }
only in patch2:
unchanged:
--- src/gdb/testsuite/gdb.threads/linux-dp.exp	1 Jan 2008 22:53:22 -0000	1.19
+++ src/gdb/testsuite/gdb.threads/linux-dp.exp	17 Apr 2008 18:28:42 -0000
@@ -80,6 +80,7 @@ for {set i 0} {$i < 5} {incr i} {
 	}
 	-re "^\[^\n\]*\n" {
 	    verbose -log "skipping line" 2
+	    exp_continue -continue_timer
 	}
 	-re "^$gdb_prompt $" {
 	}
@@ -162,6 +163,7 @@ for {set i 0} {$i < 5} {incr i} {
 	}
 	-re "^\[^\n\]*\n" {
 	    verbose -log "skipping line" 2
+	    exp_continue -continue_timer
 	}
 	-re "^$gdb_prompt $" {
 	    if { [llength $threads_before] != 0 } {