[RFA] Linux/Sparc patch 3

[RFA] Linux/Sparc patch 3

[David S. Miller]

The child_xfer_memory implementation in infptrace.c is
SLOWWWWWWW...  Especially when we have fully functional
PTRACE_{READ,WRITE}DATA under Linux/Sparc.

Another bug fix is that we need to make sure all registers
are read before store takes place, thus we define
CHILD_PREPARE_TO_STORE.  All the BSD'ish Sparc ports set
this as well, for the same reason.

We also define PTRACE_*_TYPE in preparation for 64-bit support.

Tested on sparc-linux-gnu.  No new regressions, and one regression
fixed (gdb.base/huge.exp actually finishes before the timeout).

2002-04-19  David S. Miller  <davem@redhat.com>

	* config/sparc/nm-linux.h (KERNEL_U_SIZE, kernel_u_size): Delete.
	(CHILD_PREPARE_TO_STORE, PTRACE_ARG3_TYPE, PTRACE_XFER_TYPE,
	CHILD_XFER_MEMORY): Define.
	* sparc-linux-nat.c: Include inferior.h and asm/unistd.h
	(ptrace_syscallm child_xfer_memory): New functions.
	* Makefile.in (sparc-linux-nat.o): Update dependencies.
	
--- config/sparc/nm-linux.h.~1~	Sun Feb 24 14:56:07 2002
+++ config/sparc/nm-linux.h	Fri Apr 19 15:46:01 2002
@@ -26,7 +26,20 @@
 
 #define FETCH_INFERIOR_REGISTERS
 
-/* Return sizeof user struct to callers in less machine dependent routines */
+/* Before storing, we need to read all the registers.
 
-#define KERNEL_U_SIZE kernel_u_size()
-extern int kernel_u_size (void);
+   This would be needed even if ptrace provided a way to read/write
+   one register at a time.  The reason is that if you change the value
+   of SP_REGNUM, you have to read in the local/in registers first.
+   These registers are saved in memory on the stack on Sparc, not in
+   the ptrace save area.  */
+
+#define CHILD_PREPARE_TO_STORE() read_register_bytes (0, NULL, REGISTER_BYTES)
+
+#define PTRACE_ARG3_TYPE long
+
+#ifdef __arch64__
+#define PTRACE_XFER_TYPE long
+#endif
+
+#define CHILD_XFER_MEMORY
--- sparc-linux-nat.c.~1~	Sun Feb 24 14:14:33 2002
+++ sparc-linux-nat.c	Fri Apr 19 15:51:54 2002
@@ -21,8 +21,12 @@
 
 #include "defs.h"
 #include "regcache.h"
+#include "inferior.h"
    
 #include <sys/procfs.h>
+#include <sys/ptrace.h>
+
+#include <asm/unistd.h>
 
 /* Prototypes for supply_gregset etc.  */
 #include "gregset.h"
@@ -97,4 +101,41 @@
 
   if (regno == -1 || regno == FPS_REGNUM)
     regcache_collect (FPS_REGNUM, &fpregsetp->pr_fsr);
+}
+
+
+/* Oh well, glibc's Linux ptrace() implementation does not understand
+   the 5-arg calls that we need here.  */
+#define __NR_ptrace_syscall __NR_ptrace
+_syscall5(int,ptrace_syscall,int,req,int,pid,unsigned long,memaddr,int,len,char *,myaddr)
+
+/* PTRACE_{READ,WRITE}DATA works under Sparc GNU/Linux and it is sooooo much
+   faster than what infptrace.c does.  For example, using infptrace.c
+   the test gdb.base/huge.exp can take forever, with this code it is nearly
+   instantaneous.  */
+
+int
+child_xfer_memory (CORE_ADDR memaddr, char *myaddr, int len, int write,
+		   struct mem_attrib *attrib, struct target_ops *target)
+{
+  int proc_id, err;
+
+  if ((proc_id = TIDGET (inferior_ptid)) == 0)
+    proc_id = PIDGET (inferior_ptid);		/* Not a threaded program.  */
+
+  errno = 0;
+
+  if (write)
+    {
+      ptrace_syscall (PTRACE_WRITEDATA, proc_id, memaddr, len, myaddr);
+    }
+  else
+    {
+      ptrace_syscall (PTRACE_READDATA, proc_id, memaddr, len, myaddr);
+    }
+
+  if (errno)
+    return 0;
+
+  return len;
 }
--- Makefile.in.~1~	Mon Apr  8 20:06:13 2002
+++ Makefile.in	Fri Apr 19 15:52:11 2002
@@ -2046,7 +2046,7 @@
 	$(symtab_h) $(gdb_string_h) $(source_h) $(completer_h) $(linespec_h) \
 	$(ui_out_h)
 
-sparc-linux-nat.o: sparc-linux-nat.c $(defs_h) $(regcache_h)
+sparc-linux-nat.o: sparc-linux-nat.c $(defs_h) $(regcache_h) $(inferior_h)
 
 sparc-nat.o: sparc-nat.c $(bfd_h) $(defs_h) $(inferior_h) $(gdbcore_h) \
 	$(target_h) $(regcache_h)

Re: [RFA] Linux/Sparc patch 3

[Michael Snyder]

"David S. Miller" wrote:
> 
> The child_xfer_memory implementation in infptrace.c is
> SLOWWWWWWW...  Especially when we have fully functional
> PTRACE_{READ,WRITE}DATA under Linux/Sparc.

Interesting ... could we benefit from making this
change on other (perhaps all) linux architectures?
If so, might it be possible to make the change in
a single place?

> 
> Another bug fix is that we need to make sure all registers
> are read before store takes place, thus we define
> CHILD_PREPARE_TO_STORE.  All the BSD'ish Sparc ports set
> this as well, for the same reason.

Seems perfectly reasonable.  Is this also something
that should be done for all linuxen?


> We also define PTRACE_*_TYPE in preparation for 64-bit support.

This seems like it could benefit from using multi-arch...


> Tested on sparc-linux-gnu.  No new regressions, and one regression
> fixed (gdb.base/huge.exp actually finishes before the timeout).
> 
> 2002-04-19  David S. Miller  <davem@redhat.com>
> 
>         * config/sparc/nm-linux.h (KERNEL_U_SIZE, kernel_u_size): Delete.
>         (CHILD_PREPARE_TO_STORE, PTRACE_ARG3_TYPE, PTRACE_XFER_TYPE,
>         CHILD_XFER_MEMORY): Define.
>         * sparc-linux-nat.c: Include inferior.h and asm/unistd.h
>         (ptrace_syscallm child_xfer_memory): New functions.
>         * Makefile.in (sparc-linux-nat.o): Update dependencies.
> 
> --- config/sparc/nm-linux.h.~1~ Sun Feb 24 14:56:07 2002
> +++ config/sparc/nm-linux.h     Fri Apr 19 15:46:01 2002
> @@ -26,7 +26,20 @@
> 
>  #define FETCH_INFERIOR_REGISTERS
> 
> -/* Return sizeof user struct to callers in less machine dependent routines */
> +/* Before storing, we need to read all the registers.
> 
> -#define KERNEL_U_SIZE kernel_u_size()
> -extern int kernel_u_size (void);
> +   This would be needed even if ptrace provided a way to read/write
> +   one register at a time.  The reason is that if you change the value
> +   of SP_REGNUM, you have to read in the local/in registers first.
> +   These registers are saved in memory on the stack on Sparc, not in
> +   the ptrace save area.  */
> +
> +#define CHILD_PREPARE_TO_STORE() read_register_bytes (0, NULL, REGISTER_BYTES)
> +
> +#define PTRACE_ARG3_TYPE long
> +
> +#ifdef __arch64__
> +#define PTRACE_XFER_TYPE long
> +#endif
> +
> +#define CHILD_XFER_MEMORY
> --- sparc-linux-nat.c.~1~       Sun Feb 24 14:14:33 2002
> +++ sparc-linux-nat.c   Fri Apr 19 15:51:54 2002
> @@ -21,8 +21,12 @@
> 
>  #include "defs.h"
>  #include "regcache.h"
> +#include "inferior.h"
> 
>  #include <sys/procfs.h>
> +#include <sys/ptrace.h>
> +
> +#include <asm/unistd.h>
> 
>  /* Prototypes for supply_gregset etc.  */
>  #include "gregset.h"
> @@ -97,4 +101,41 @@
> 
>    if (regno == -1 || regno == FPS_REGNUM)
>      regcache_collect (FPS_REGNUM, &fpregsetp->pr_fsr);
> +}
> +
> +
> +/* Oh well, glibc's Linux ptrace() implementation does not understand
> +   the 5-arg calls that we need here.  */
> +#define __NR_ptrace_syscall __NR_ptrace
> +_syscall5(int,ptrace_syscall,int,req,int,pid,unsigned long,memaddr,int,len,char *,myaddr)
> +
> +/* PTRACE_{READ,WRITE}DATA works under Sparc GNU/Linux and it is sooooo much
> +   faster than what infptrace.c does.  For example, using infptrace.c
> +   the test gdb.base/huge.exp can take forever, with this code it is nearly
> +   instantaneous.  */
> +
> +int
> +child_xfer_memory (CORE_ADDR memaddr, char *myaddr, int len, int write,
> +                  struct mem_attrib *attrib, struct target_ops *target)
> +{
> +  int proc_id, err;
> +
> +  if ((proc_id = TIDGET (inferior_ptid)) == 0)
> +    proc_id = PIDGET (inferior_ptid);          /* Not a threaded program.  */
> +
> +  errno = 0;
> +
> +  if (write)
> +    {
> +      ptrace_syscall (PTRACE_WRITEDATA, proc_id, memaddr, len, myaddr);
> +    }
> +  else
> +    {
> +      ptrace_syscall (PTRACE_READDATA, proc_id, memaddr, len, myaddr);
> +    }
> +
> +  if (errno)
> +    return 0;
> +
> +  return len;
>  }
> --- Makefile.in.~1~     Mon Apr  8 20:06:13 2002
> +++ Makefile.in Fri Apr 19 15:52:11 2002
> @@ -2046,7 +2046,7 @@
>         $(symtab_h) $(gdb_string_h) $(source_h) $(completer_h) $(linespec_h) \
>         $(ui_out_h)
> 
> -sparc-linux-nat.o: sparc-linux-nat.c $(defs_h) $(regcache_h)
> +sparc-linux-nat.o: sparc-linux-nat.c $(defs_h) $(regcache_h) $(inferior_h)
> 
>  sparc-nat.o: sparc-nat.c $(bfd_h) $(defs_h) $(inferior_h) $(gdbcore_h) \
>         $(target_h) $(regcache_h)

Re: [RFA] Linux/Sparc patch 3

[David S. Miller]

   From: Michael Snyder <msnyder@redhat.com>
   Date: Fri, 19 Apr 2002 18:53:54 -0700

   "David S. Miller" wrote:
   > The child_xfer_memory implementation in infptrace.c is
   > SLOWWWWWWW...  Especially when we have fully functional
   > PTRACE_{READ,WRITE}DATA under Linux/Sparc.
   
   Interesting ... could we benefit from making this
   change on other (perhaps all) linux architectures?
   If so, might it be possible to make the change in
   a single place?
   
Only Linux/Sparc supports PTRACE_{READ,WRITE}DATA, nyah nyah :-)

   > Another bug fix is that we need to make sure all registers
   > are read before store takes place, thus we define
   > CHILD_PREPARE_TO_STORE.  All the BSD'ish Sparc ports set
   > this as well, for the same reason.
   
   Seems perfectly reasonable.  Is this also something
   that should be done for all linuxen?
   
No, it is a Sparc specific issue.  Can I ask you to read my changes
when reviewing them?  I describe the situation precisely in the
comment above this define, and I make it clear that this is a Sparc
specific issue.  If my comment doesn't make this clear, show me how to
make it so. :-)
   
   > We also define PTRACE_*_TYPE in preparation for 64-bit support.
   
   This seems like it could benefit from using multi-arch...

Please wait until the facility to do this is really there.  See my
other emails about OS specific gdbarch init calls 

These values only matter for native ptrace calls anyways, I don't see
why you'd bother multi-arching it.
   
Franks a lot,
David S. Miller
davem@redhat.com

Re: [RFA] Linux/Sparc patch 3

[Michael Snyder]

"David S. Miller" wrote:
> 
>    From: Michael Snyder <msnyder@redhat.com>
>    Date: Fri, 19 Apr 2002 18:53:54 -0700
> 
>    "David S. Miller" wrote:

>    > Another bug fix is that we need to make sure all registers
>    > are read before store takes place, thus we define
>    > CHILD_PREPARE_TO_STORE.  All the BSD'ish Sparc ports set
>    > this as well, for the same reason.
> 
>    Seems perfectly reasonable.  Is this also something
>    that should be done for all linuxen?
> 
> No, it is a Sparc specific issue.  Can I ask you to read my changes
> when reviewing them?  I describe the situation precisely in the
> comment above this define, and I make it clear that this is a Sparc
> specific issue.  If my comment doesn't make this clear, show me how to
> make it so. :-)

David,

There's nothing wrong with your comments or your coding skills, 
but you need to sit down and read a book on attitude (specifically, 
not copping one when you're asking for someone's time and energy).
If I'd read this message earlier, I wouldn't have given you the
several hours time that I already have today.

No one here owes you anything, David.  This is a volunteer project.

Sincerely, 
Michael Snyder