[commit] Prefer xfer_partial for memory xfers

[commit] Prefer xfer_partial for memory xfers

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 449 bytes --]

Hello,

This modifies the memory read/write functions so that they strongly 
prefer the inferior's to_xfer_partial method over the older 
to_xfer_memory method.

With this in place the need to implement xfer_memory is eliminated - I 
deleted the function from inf-ptrace.c - inferiors (namely GNU/Linux) 
can more easily overide the inherited xfer method.

committed,
Andrew

(and I thought getting GNU/Linux using inferior inheritance was be easy)

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 5691 bytes --]

2004-09-30  Andrew Cagney  <cagney@gnu.org>

	* inf-ptrace.c (inf_ptrace_target): Do not set to_xfer_memory.
	(inf_ptrace_xfer_memory): Delete.
	* target.c (target_xfer_partial_p, xfer_using_stratum): New.
	(target_read_memory, target_write_memory): Use xfer_using_stratum
	when target_xfer_partial_p.
	(debug_target): Move to start of file.
	(target_read_memory_partial, target_write_memory_partial): Use
	to_xfer_partial when target_xfer_partial_p.

Index: inf-ptrace.c
===================================================================
RCS file: /cvs/src/src/gdb/inf-ptrace.c,v
retrieving revision 1.7
diff -p -u -r1.7 inf-ptrace.c
--- inf-ptrace.c	30 Sep 2004 15:47:30 -0000	1.7
+++ inf-ptrace.c	30 Sep 2004 16:17:38 -0000
@@ -742,7 +742,6 @@ inf_ptrace_target (void)
   t->to_resume = inf_ptrace_resume;
   t->to_wait = inf_ptrace_wait;
   t->to_prepare_to_store = inf_ptrace_prepare_to_store;
-  t->to_xfer_memory = inf_ptrace_xfer_memory;
   t->to_xfer_partial = inf_ptrace_xfer_partial;
   t->to_files_info = inf_ptrace_files_info;
   t->to_kill = inf_ptrace_kill_inferior;
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.83
diff -p -u -r1.83 target.c
--- target.c	30 Sep 2004 14:16:20 -0000	1.83
+++ target.c	30 Sep 2004 16:17:38 -0000
@@ -840,6 +840,89 @@ target_section_by_addr (struct target_op
   return NULL;
 }
 
+/* Return non-zero when the target vector has supplied an xfer_partial
+   method and it, rather than xfer_memory, should be used.  */
+static int
+target_xfer_partial_p (void)
+{
+  return (target_stack != NULL
+	  && target_stack->to_xfer_partial != default_xfer_partial);
+}
+
+/* Attempt a transfer all LEN bytes starting at OFFSET between the
+   inferior's KIND:ANNEX space and GDB's READBUF/WRITEBUF buffer.  If
+   the transfer succeeds, return zero, otherwize the host ERRNO is
+   returned.
+
+   The inferior is formed from several layers.  In the case of
+   corefiles, inf-corefile is layered above inf-exec and a request for
+   text (corefiles do not include text pages) will be first sent to
+   the core-stratum, fail, and then sent to the object-file where it
+   will succeed.
+
+   NOTE: cagney/2004-09-30:
+
+   The old code tried to use four separate mechanisms for mapping an
+   object:offset:len tuple onto an inferior and its address space: the
+   target stack; the inferior's TO_SECTIONS; solib's SO_LIST;
+   overlays.
+
+   This is stupid.
+
+   The code below is instead using a single mechanism (currently
+   strata).  If that mechanism proves insufficient then re-factor it
+   implementing another singluar mechanism (for instance, a generic
+   object:annex onto inferior:object:annex say).  */
+
+static int
+xfer_using_stratum (enum target_object object, const char *annex,
+		    CORE_ADDR memaddr, int len, void *readbuf,
+		    const void *writebuf)
+{
+  LONGEST xfered;
+  struct target_ops *target;
+
+  /* Always successful.  */
+  if (len == 0)
+    return 0;
+  /* Never successful.  */
+  if (target_stack == NULL)
+    return EIO;
+
+  target = target_stack;
+  while (1)
+    {
+      xfered = target->to_xfer_partial (target, object, annex,
+					readbuf, writebuf, memaddr, len);
+      if (xfered > 0)
+	{
+	  /* The partial xfer succeeded, update the counts, check that
+	     the xfer hasn't finished and if it hasn't set things up
+	     for the next round.  */
+	  len -= xfered;
+	  if (len <= 0)
+	    return 0;
+	  target = target_stack;
+	}
+      else if (xfered < 0)
+	{
+	  /* Something totally screwed up, abandon the attempt to
+	     xfer.  */
+	  if (errno)
+	    return errno;
+	  else
+	    return EIO;
+	}
+      else
+	{
+	  /* This "stratum" didn't work, try the next one down.  */
+	  target = target->beneath;
+	  if (target == NULL)
+	    return EIO;
+	}
+    }
+}
+
 /* Read LEN bytes of target memory at address MEMADDR, placing the results in
    GDB's memory at MYADDR.  Returns either 0 for success or an errno value
    if any error occurs.
@@ -853,13 +936,21 @@ target_section_by_addr (struct target_op
 int
 target_read_memory (CORE_ADDR memaddr, char *myaddr, int len)
 {
-  return target_xfer_memory (memaddr, myaddr, len, 0);
+  if (target_xfer_partial_p ())
+    return xfer_using_stratum (TARGET_OBJECT_MEMORY, NULL,
+			       memaddr, len, myaddr, NULL);
+  else
+    return target_xfer_memory (memaddr, myaddr, len, 0);
 }
 
 int
 target_write_memory (CORE_ADDR memaddr, char *myaddr, int len)
 {
-  return target_xfer_memory (memaddr, myaddr, len, 1);
+  if (target_xfer_partial_p ())
+    return xfer_using_stratum (TARGET_OBJECT_MEMORY, NULL,
+			       memaddr, len, NULL, myaddr);
+  else
+    return target_xfer_memory (memaddr, myaddr, len, 1);
 }
 
 static int trust_readonly = 0;
@@ -1065,13 +1156,23 @@ target_xfer_memory_partial (CORE_ADDR me
 int
 target_read_memory_partial (CORE_ADDR memaddr, char *buf, int len, int *err)
 {
-  return target_xfer_memory_partial (memaddr, buf, len, 0, err);
+  if (target_xfer_partial_p ())
+    return target_stack->to_xfer_partial (target_stack,
+					  TARGET_OBJECT_MEMORY, NULL,
+					  buf, NULL, memaddr, len);
+  else
+    return target_xfer_memory_partial (memaddr, buf, len, 0, err);
 }
 
 int
 target_write_memory_partial (CORE_ADDR memaddr, char *buf, int len, int *err)
 {
-  return target_xfer_memory_partial (memaddr, buf, len, 1, err);
+  if (target_xfer_partial_p ())
+    return target_stack->to_xfer_partial (target_stack,
+					  TARGET_OBJECT_MEMORY, NULL,
+					  NULL, buf, memaddr, len);
+  else
+    return target_xfer_memory_partial (memaddr, buf, len, 1, err);
 }
 
 /* More generic transfers.  */

Re: [commit] Prefer xfer_partial for memory xfers

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 115 bytes --]

> 	(inf_ptrace_xfer_memory): Delete.

Oops, this was lost from the patch / commit, it's now really there.

Andrew


[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 4860 bytes --]

Index: inf-ptrace.c
===================================================================
RCS file: /cvs/src/src/gdb/inf-ptrace.c,v
retrieving revision 1.8
diff -p -u -r1.8 inf-ptrace.c
--- inf-ptrace.c	30 Sep 2004 16:18:57 -0000	1.8
+++ inf-ptrace.c	30 Sep 2004 16:46:10 -0000
@@ -95,145 +95,6 @@ inf_ptrace_resume (ptid_t ptid, int step
     perror_with_name ("ptrace");
 }
 
-/* Set an upper limit on alloca.  */
-#define GDB_MAX_ALLOCA 0x1000
-
-/* NOTE! I tried using PTRACE_READDATA, etc., to read and write memory
-   in the NEW_SUN_PTRACE case.  It ought to be straightforward.  But
-   it appears that writing did not write the data that I specified.  I
-   cannot understand where it got the data that it actually did
-   write.  */
-
-/* Copy LEN bytes to or from inferior's memory starting at MEMADDR to
-   debugger memory starting at MYADDR.  Copy to inferior if WRITE is
-   nonzero.  TARGET is ignored.
-
-   Returns the length copied, which is either the LEN argument or
-   zero.  This xfer function does not do partial moves, since
-   ptrace_ops_hack doesn't allow memory operations to cross below us in the
-   target stack anyway.  */
-
-int
-inf_ptrace_xfer_memory (CORE_ADDR memaddr, char *myaddr, int len, int write,
-			struct mem_attrib *attrib, struct target_ops *target)
-{
-  int i;
-  /* Round starting address down to longword boundary.  */
-  CORE_ADDR addr = memaddr & -(CORE_ADDR) sizeof (PTRACE_TYPE_RET);
-  /* Round ending address up; get number of longwords that makes.  */
-  int count = ((((memaddr + len) - addr) + sizeof (PTRACE_TYPE_RET) - 1)
-	       / sizeof (PTRACE_TYPE_RET));
-  int alloc = count * sizeof (PTRACE_TYPE_RET);
-  PTRACE_TYPE_RET *buffer;
-  struct cleanup *old_chain = NULL;
-
-#ifdef PT_IO
-  /* OpenBSD 3.1, NetBSD 1.6 and FreeBSD 5.0 have a new PT_IO request
-     that promises to be much more efficient in reading and writing
-     data in the traced process's address space.  */
-
-  {
-    struct ptrace_io_desc piod;
-
-    /* NOTE: We assume that there are no distinct address spaces for
-       instruction and data.  */
-    piod.piod_op = write ? PIOD_WRITE_D : PIOD_READ_D;
-    piod.piod_offs = (void *) memaddr;
-    piod.piod_addr = myaddr;
-    piod.piod_len = len;
-
-    if (ptrace (PT_IO, PIDGET (inferior_ptid), (caddr_t) &piod, 0) == -1)
-      {
-	/* If the PT_IO request is somehow not supported, fallback on
-	   using PT_WRITE_D/PT_READ_D.  Otherwise we will return zero
-	   to indicate failure.  */
-	if (errno != EINVAL)
-	  return 0;
-      }
-    else
-      {
-	/* Return the actual number of bytes read or written.  */
-	return piod.piod_len;
-      }
-  }
-#endif
-
-  /* Allocate buffer of that many longwords.  */
-  if (len < GDB_MAX_ALLOCA)
-    {
-      buffer = (PTRACE_TYPE_RET *) alloca (alloc);
-    }
-  else
-    {
-      buffer = (PTRACE_TYPE_RET *) xmalloc (alloc);
-      old_chain = make_cleanup (xfree, buffer);
-    }
-
-  if (write)
-    {
-      /* Fill start and end extra bytes of buffer with existing memory
-         data.  */
-      if (addr != memaddr || len < (int) sizeof (PTRACE_TYPE_RET))
-	{
-	  /* Need part of initial word -- fetch it.  */
-	  buffer[0] = ptrace (PT_READ_I, PIDGET (inferior_ptid),
-			      (PTRACE_TYPE_ARG3) addr, 0);
-	}
-
-      if (count > 1)		/* FIXME, avoid if even boundary.  */
-	{
-	  buffer[count - 1] =
-	    ptrace (PT_READ_I, PIDGET (inferior_ptid),
-		    ((PTRACE_TYPE_ARG3)
-		     (addr + (count - 1) * sizeof (PTRACE_TYPE_RET))), 0);
-	}
-
-      /* Copy data to be written over corresponding part of buffer.  */
-      memcpy ((char *) buffer + (memaddr & (sizeof (PTRACE_TYPE_RET) - 1)),
-	      myaddr, len);
-
-      /* Write the entire buffer.  */
-      for (i = 0; i < count; i++, addr += sizeof (PTRACE_TYPE_RET))
-	{
-	  errno = 0;
-	  ptrace (PT_WRITE_D, PIDGET (inferior_ptid),
-		  (PTRACE_TYPE_ARG3) addr, buffer[i]);
-	  if (errno)
-	    {
-	      /* Using the appropriate one (I or D) is necessary for
-	         Gould NP1, at least.  */
-	      errno = 0;
-	      ptrace (PT_WRITE_I, PIDGET (inferior_ptid),
-		      (PTRACE_TYPE_ARG3) addr, buffer[i]);
-	    }
-	  if (errno)
-	    return 0;
-	}
-    }
-  else
-    {
-      /* Read all the longwords.  */
-      for (i = 0; i < count; i++, addr += sizeof (PTRACE_TYPE_RET))
-	{
-	  errno = 0;
-	  buffer[i] = ptrace (PT_READ_I, PIDGET (inferior_ptid),
-			      (PTRACE_TYPE_ARG3) addr, 0);
-	  if (errno)
-	    return 0;
-	  QUIT;
-	}
-
-      /* Copy appropriate bytes out of the buffer.  */
-      memcpy (myaddr,
-	      (char *) buffer + (memaddr & (sizeof (PTRACE_TYPE_RET) - 1)),
-	      len);
-    }
-
-  if (old_chain != NULL)
-    do_cleanups (old_chain);
-  return len;
-}
-
 /* Wait for child to do something.  Return pid of child, or -1 in case
    of error; store status through argument pointer OURSTATUS.  */
 

Re: [commit] Prefer xfer_partial for memory xfers

[Daniel Jacobowitz]

On Thu, Sep 30, 2004 at 12:27:50PM -0400, Andrew Cagney wrote:
> +   NOTE: cagney/2004-09-30:
> +
> +   The old code tried to use four separate mechanisms for mapping an
> +   object:offset:len tuple onto an inferior and its address space: the
> +   target stack; the inferior's TO_SECTIONS; solib's SO_LIST;
> +   overlays.
> +
> +   This is stupid.
> +
> +   The code below is instead using a single mechanism (currently
> +   strata).  If that mechanism proves insufficient then re-factor it
> +   implementing another singluar mechanism (for instance, a generic
> +   object:annex onto inferior:object:annex say).  */

"singular".  Stray "say" (don't want an interjection at both the
beginning and the end of the phrase).

Does this mean that a bunch of new code would have to be written to
make this compatible with overlays?  How about solibs?


-- 
Daniel Jacobowitz

Re: [commit] Prefer xfer_partial for memory xfers

[Andrew Cagney]

> On Thu, Sep 30, 2004 at 12:27:50PM -0400, Andrew Cagney wrote:
> 
>>> +   NOTE: cagney/2004-09-30:
>>> +
>>> +   The old code tried to use four separate mechanisms for mapping an
>>> +   object:offset:len tuple onto an inferior and its address space: the
>>> +   target stack; the inferior's TO_SECTIONS; solib's SO_LIST;
>>> +   overlays.
>>> +
>>> +   This is stupid.
>>> +
>>> +   The code below is instead using a single mechanism (currently
>>> +   strata).  If that mechanism proves insufficient then re-factor it
>>> +   implementing another singluar mechanism (for instance, a generic
>>> +   object:annex onto inferior:object:annex say).  */

> Does this mean that a bunch of new code would have to be written to
> make this compatible with overlays?  How about solibs?

I tested on PPC/NetBSD and it showed no regressions so the solib case is 
definitly covered (and as the note implies, there's serious level of 
redundancy in the current "design").

Andrew