Re: [RFA] Crasher bug in infptrace.c

[Andrew Cagney <ac131313 at cygnus dot com>]

> Anyway, looking at the code, I'm wondering if it would actually be
>> better to just eliminate that bounce buffer and, instead just transfer
>> the data directly.  This might leave the buffer in an undefined state, I
>> think, however, that is ok.
> 
> 
> I spent an hour thinking about how to do that (without significantly
> uglifying the code), and decided it was more trouble than I wanted to
> go to.  I agree with you -- the function doesn't require a buffer 
> at all.  Anyone who wants to rewrite the function to that extent
> is more than welcome to by me.  ;-)


How does the attached work-in-progress look as a starting point?  Ran it 
against the testsuite and that came up with a few regressions ..... 
However, it isn't totally broken.

One thing I didn't realize is that I can't trust the alignment of the 
hosts buffer and consequently I can't actually avoid the double buffering.

Andrew
Index: infptrace.c
===================================================================
RCS file: /cvs/src/src/gdb/infptrace.c,v
retrieving revision 1.20
diff -p -r1.20 infptrace.c
*** infptrace.c	2002/01/08 00:59:29	1.20
--- infptrace.c	2002/01/14 01:40:42
***************
*** 26,31 ****
--- 26,32 ----
  #include "target.h"
  #include "gdb_string.h"
  #include "regcache.h"
+ #include "gdb_assert.h"
  
  #include "gdb_wait.h"
  
*************** child_xfer_memory (CORE_ADDR memaddr, ch
*** 505,510 ****
--- 506,603 ----
  		   struct mem_attrib *attrib ATTRIBUTE_UNUSED,
  		   struct target_ops *target)
  {
+ #if 1
+   CORE_ADDR addr = memaddr;
+   long nr_bytes = len;
+   PTRACE_XFER_TYPE buffer[1000];
+   while (nr_bytes > 0)
+     {
+       CORE_ADDR bufaddr;
+       long count;
+       long sizeof_buffer;
+       long addr_off;
+       long addr_len;
+       long addr_pad;
+       long i;
+ 
+       /* Round starting address down to longword boundary to pick up
+          all of the first byte.  */
+       bufaddr = addr & -(CORE_ADDR) sizeof (PTRACE_XFER_TYPE);
+ 
+       /* Round ending address up to ensure that all of the last word
+          is copied up and then adjust that to fit in BUFFER.  */
+       sizeof_buffer = (addr - bufaddr) + len + sizeof (PTRACE_XFER_TYPE) - 1;
+       if (sizeof_buffer > sizeof (buffer))
+ 	sizeof_buffer = sizeof (buffer);
+       count = sizeof_buffer / sizeof (PTRACE_XFER_TYPE);
+       sizeof_buffer = count * sizeof (PTRACE_XFER_TYPE); /* adjust */
+ 
+       /* Break the buffer down into three byte regions: ADDR_OFF
+ 	 specifies the offset into BUFFER that ADDR starts;
+ 	 ADDR_LEN specifies the number of ADDR bytes actually in
+ 	 the buffer; and ADDR_PAD is the number of left over bytes
+ 	 in the buffer.  Either/or ADDR_OFF and ADDR_PAD can be
+ 	 zero.  */
+       addr_off = addr - bufaddr;
+       addr_len = sizeof_buffer - addr_off;
+       if (addr_len > len)
+ 	{
+ 	  addr_len = len;
+ 	}
+       addr_pad = sizeof_buffer - (addr_len + addr_off);
+       gdb_assert (addr_off + addr_len + addr_pad == sizeof_buffer);
+       
+       /* For writes, pre-fill the buffer with data that has to go out.  */
+       if (write)
+ 	{
+ 	  /* If the start is misaligned, a read - modify - write is
+              needed.  */
+ 	  if (addr_off > 0)
+ 	    buffer[0] = ptrace (PT_READ_I, PIDGET (inferior_ptid), 
+ 				(PTRACE_ARG3_TYPE) bufaddr, 0);
+ 	  /* If the end is misaligned, a read - modify - write is
+              needed.  */
+ 	  if (addr_pad > 0 && count > 1)
+ 	    {
+ 	      buffer[count - 1] =
+ 		ptrace (PT_READ_I, PIDGET (inferior_ptid),
+ 			((PTRACE_ARG3_TYPE)
+ 			 (bufaddr + (count - 1) * sizeof (PTRACE_XFER_TYPE))), 0);
+ 	    }
+ 	  /* Fill in LEN bytes in the middle. */
+ 	  memcpy (buffer + addr_off, myaddr, addr_len);
+ 	}
+ 
+       /* Transfer COUNT words. */
+       for (i = 0; i < count; i++)
+ 	{
+ 	  CORE_ADDR a = bufaddr + i * sizeof (PTRACE_XFER_TYPE);
+ 	  if (write)
+ 	    {
+ 	      ptrace (PT_WRITE_D, PIDGET (inferior_ptid), 
+ 		      (PTRACE_ARG3_TYPE) a, buffer[i]);
+ 	    }
+ 	  else
+ 	    {
+ 	      buffer[i] = ptrace (PT_READ_I, PIDGET (inferior_ptid),
+ 				  (PTRACE_ARG3_TYPE) a, 0);
+ 	    }
+ 	}
+ 
+       /* Copy any read data into MYADDR.  */
+       if (!write)
+ 	memcpy (myaddr, buffer + addr_off, addr_len);
+ 
+       /* Update all counters.  */
+       addr += addr_len;
+       nr_bytes -= addr_len;
+     }
+   return len;
+ #ifdef CLEAR_INSN_CACHE
+   if (write)
+     CLEAR_INSN_CACHE ();
+ #endif
+ #else
    int i;
    /* Round starting address down to longword boundary.  */
    CORE_ADDR addr = memaddr & -(CORE_ADDR) sizeof (PTRACE_XFER_TYPE);
*************** child_xfer_memory (CORE_ADDR memaddr, ch
*** 592,597 ****
--- 685,691 ----
    if (old_chain != NULL)
      do_cleanups (old_chain);
    return len;
+ #endif
  }