[binutils-gdb] inf-ptrace: Do not stop memory transfers after a single word

[Andreas Arnez <arnez at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=87c336f60eccc6506ff19369c29575f43fea02ea

commit 87c336f60eccc6506ff19369c29575f43fea02ea
Author: Andreas Arnez <arnez@linux.vnet.ibm.com>
Date:   Tue Mar 14 19:20:46 2017 +0100

    inf-ptrace: Do not stop memory transfers after a single word
    
    When inf_ptrace_xfer_partial performs a memory transfer via ptrace with
    PT_READ_I, PT_WRITE_I (aka PTRACE_PEEKTEXT, PTRACE_POKETEXT), etc., then
    it currently transfers at most one word.  This behavior yields degraded
    performance, particularly if the caller has significant preparation work
    for each invocation.  And indeed it has for writing, in
    memory_xfer_partial in target.c, where all of the remaining data to be
    transferred is copied to a temporary buffer each time, for breakpoint
    shadow handling.  Thus large writes have quadratic runtime and can take
    hours.
    
    Note: On GNU/Linux targets GDB usually does not use
    inf_ptrace_xfer_partial for large memory transfers, but attempts a single
    read/write from/to /proc/<pid>/mem instead.  However, the kernel may
    reject writes to /proc/<pid>/mem (such as kernels prior to 2.6.39), or
    /proc may not be mounted.  In both cases GDB falls back to the ptrace
    mechanism.
    
    This patch fixes the performance issue by attempting to fulfill the whole
    transfer request in inf_ptrace_xfer_partial, using a loop around the
    ptrace call.
    
    gdb/ChangeLog:
    
    	PR gdb/21220
    	* inf-ptrace.c (inf_ptrace_xfer_partial): In "case
    	TARGET_OBJECT_MEMORY", extract the logic for ptrace peek/poke...
    	(inf_ptrace_peek_poke): ...here.  New function.  Now also loop
    	over ptrace peek/poke until end of buffer or error.

Diff:
---
 gdb/ChangeLog    |   8 ++++
 gdb/inf-ptrace.c | 142 +++++++++++++++++++++++++++----------------------------
 2 files changed, 77 insertions(+), 73 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 63d7441..2658d917 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2017-03-14  Andreas Arnez  <arnez@linux.vnet.ibm.com>
+
+	PR gdb/21220
+	* inf-ptrace.c (inf_ptrace_xfer_partial): In "case
+	TARGET_OBJECT_MEMORY", extract the logic for ptrace peek/poke...
+	(inf_ptrace_peek_poke): ...here.  New function.  Now also loop
+	over ptrace peek/poke until end of buffer or error.
+
 2017-03-14  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* parse.c (length_of_subexp): Make static.
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 21578742..32794ec 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -446,6 +446,72 @@ inf_ptrace_wait (struct target_ops *ops,
   return pid_to_ptid (pid);
 }
 
+/* Transfer data via ptrace into process PID's memory from WRITEBUF, or
+   from process PID's memory into READBUF.  Start at target address ADDR
+   and transfer up to LEN bytes.  Exactly one of READBUF and WRITEBUF must
+   be non-null.  Return the number of transferred bytes.  */
+
+static ULONGEST
+inf_ptrace_peek_poke (pid_t pid, gdb_byte *readbuf,
+		      const gdb_byte *writebuf,
+		      ULONGEST addr, ULONGEST len)
+{
+  ULONGEST n;
+  unsigned int chunk;
+
+  /* We transfer aligned words.  Thus align ADDR down to a word
+     boundary and determine how many bytes to skip at the
+     beginning.  */
+  unsigned int skip = addr & (sizeof (PTRACE_TYPE_RET) - 1);
+  addr -= skip;
+
+  for (n = 0;
+       n < len;
+       n += chunk, addr += sizeof (PTRACE_TYPE_RET), skip = 0)
+    {
+      /* Restrict to a chunk that fits in the current word.  */
+      chunk = std::min (sizeof (PTRACE_TYPE_RET) - skip, len - n);
+
+      /* Use a union for type punning.  */
+      union
+      {
+	PTRACE_TYPE_RET word;
+	gdb_byte byte[sizeof (PTRACE_TYPE_RET)];
+      } buf;
+
+      /* Read the word, also when doing a partial word write.  */
+      if (readbuf != NULL || chunk < sizeof (PTRACE_TYPE_RET))
+	{
+	  errno = 0;
+	  buf.word = ptrace (PT_READ_I, pid,
+			     (PTRACE_TYPE_ARG3)(uintptr_t) addr, 0);
+	  if (errno != 0)
+	    break;
+	  if (readbuf != NULL)
+	    memcpy (readbuf + n, buf.byte + skip, chunk);
+	}
+      if (writebuf != NULL)
+	{
+	  memcpy (buf.byte + skip, writebuf + n, chunk);
+	  errno = 0;
+	  ptrace (PT_WRITE_D, pid, (PTRACE_TYPE_ARG3)(uintptr_t) addr,
+		  buf.word);
+	  if (errno != 0)
+	    {
+	      /* Using the appropriate one (I or D) is necessary for
+		 Gould NP1, at least.  */
+	      errno = 0;
+	      ptrace (PT_WRITE_I, pid, (PTRACE_TYPE_ARG3)(uintptr_t) addr,
+		      buf.word);
+	      if (errno != 0)
+		break;
+	    }
+	}
+    }
+
+  return n;
+}
+
 /* Implement the to_xfer_partial target_ops method.  */
 
 static enum target_xfer_status
@@ -491,79 +557,9 @@ inf_ptrace_xfer_partial (struct target_ops *ops, enum target_object object,
 	  return TARGET_XFER_EOF;
       }
 #endif
-      {
-	union
-	{
-	  PTRACE_TYPE_RET word;
-	  gdb_byte byte[sizeof (PTRACE_TYPE_RET)];
-	} buffer;
-	ULONGEST rounded_offset;
-	ULONGEST partial_len;
-
-	/* Round the start offset down to the next long word
-	   boundary.  */
-	rounded_offset = offset & -(ULONGEST) sizeof (PTRACE_TYPE_RET);
-
-	/* Since ptrace will transfer a single word starting at that
-	   rounded_offset the partial_len needs to be adjusted down to
-	   that (remember this function only does a single transfer).
-	   Should the required length be even less, adjust it down
-	   again.  */
-	partial_len = (rounded_offset + sizeof (PTRACE_TYPE_RET)) - offset;
-	if (partial_len > len)
-	  partial_len = len;
-
-	if (writebuf)
-	  {
-	    /* If OFFSET:PARTIAL_LEN is smaller than
-	       ROUNDED_OFFSET:WORDSIZE then a read/modify write will
-	       be needed.  Read in the entire word.  */
-	    if (rounded_offset < offset
-		|| (offset + partial_len
-		    < rounded_offset + sizeof (PTRACE_TYPE_RET)))
-	      /* Need part of initial word -- fetch it.  */
-	      buffer.word = ptrace (PT_READ_I, pid,
-				    (PTRACE_TYPE_ARG3)(uintptr_t)
-				    rounded_offset, 0);
-
-	    /* Copy data to be written over corresponding part of
-	       buffer.  */
-	    memcpy (buffer.byte + (offset - rounded_offset),
-		    writebuf, partial_len);
-
-	    errno = 0;
-	    ptrace (PT_WRITE_D, pid,
-		    (PTRACE_TYPE_ARG3)(uintptr_t)rounded_offset,
-		    buffer.word);
-	    if (errno)
-	      {
-		/* Using the appropriate one (I or D) is necessary for
-		   Gould NP1, at least.  */
-		errno = 0;
-		ptrace (PT_WRITE_I, pid,
-			(PTRACE_TYPE_ARG3)(uintptr_t)rounded_offset,
-			buffer.word);
-		if (errno)
-		  return TARGET_XFER_EOF;
-	      }
-	  }
-
-	if (readbuf)
-	  {
-	    errno = 0;
-	    buffer.word = ptrace (PT_READ_I, pid,
-				  (PTRACE_TYPE_ARG3)(uintptr_t)rounded_offset,
-				  0);
-	    if (errno)
-	      return TARGET_XFER_EOF;
-	    /* Copy appropriate bytes out of the buffer.  */
-	    memcpy (readbuf, buffer.byte + (offset - rounded_offset),
-		    partial_len);
-	  }
-
-	*xfered_len = partial_len;
-	return TARGET_XFER_OK;
-      }
+      *xfered_len = inf_ptrace_peek_poke (pid, readbuf, writebuf,
+					  offset, len);
+      return *xfered_len != 0 ? TARGET_XFER_OK : TARGET_XFER_EOF;
 
     case TARGET_OBJECT_UNWIND_TABLE:
       return TARGET_XFER_E_IO;