Changing memory that could contain breakpoint instructions

Changing memory that could contain breakpoint instructions

[Mark Wielaard]

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

Hi,

Teresa tripped over the limitation of the LogicalMemoryBuffer that made
the buffer read-only. This prevented changing the value static variables
found at a specific memory locations as described in bug #5201. I
changed LogicMemoryBuffer to allow writing to locations that don't have
a breakpoint instruction installed. This should cover almost all cases
we use now since code and variables live at different locations
normally, so it should never happen that you put a breakpoint
instruction on some code location and that same location also is the
address of a memory variable. Luckily there were already tests written
for this, so I only had to enable them for LogicMemoryBuffer.

2007-10-20  Mark Wielaard  <mwielaard@redhat.com>

    Fixes bug #5201
    * LogicalMemoryBuffer.java (poke): Allow poke when no breakpoints
    in the way.
    * TestByteBuffer.java: Enable LogicalMemoryBuffer write tests.

For the general case of changing code while the core might have a
breakpoint installed on top of the code to be changed I opened a new bug
report http://sourceware.org/bugzilla/show_bug.cgi?id=5202

        Currently when trying to poke code which has breakpoint
        installed through the LogicalMemoryBuffer will fail. You can
        only change memory which the core isn't using itself at the
        moment. When changing memory on which the core has put software
        breakpoint instructions the set-aside memory of the Instruction
        should be changed (so that if the breakpoint is removed or
        stepped the new code is used).
        
        This is a little tricky to change. When a Breakpoint is found
        installed the underlying Instruction class should be altered
        (but Instruction is currently a immutable class). Care should be
        taken if the Breakpoint is currently stepped, or if the memory
        is changed asynchronous from the event thread (which means it
        could intersect with a breakpoint being placed right at the same
        time).
        
Cheers,

Mark

[-- Attachment #2: logic-memory-poke.patch --]
[-- Type: text/x-patch, Size: 3850 bytes --]

Index: frysk-core/frysk/proc/live/LogicalMemoryBuffer.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/live/LogicalMemoryBuffer.java,v
retrieving revision 1.3
diff -u -r1.3 LogicalMemoryBuffer.java
--- frysk-core/frysk/proc/live/LogicalMemoryBuffer.java	2 Aug 2007 11:23:28 -0000	1.3
+++ frysk-core/frysk/proc/live/LogicalMemoryBuffer.java	21 Oct 2007 04:57:42 -0000
@@ -128,14 +128,37 @@
       }
   }
 
+  /**
+   * Pokes the value at the given index. Unless a breakpoint is set at
+   * that location (FIXME: this limitation should be lifted).
+   */
   protected void poke (long index, int value)
   {
-    throw new UnsupportedOperationException("read only memory buffer");
+    Breakpoint breakpoint = breakpoints.getBreakpoint(index);
+    if (breakpoint != null)
+      throw new UnsupportedOperationException("breakpoint set at: " + index);
+
+    super.poke(index, value);
   }
   
+  /**
+   * Pokes the given bytes from offset at the index plus the given
+   * lenght. Unless a breakpoint is set in that range (FIXME: this
+   * limitation should be lifted).
+   */
   protected int poke(long index, byte[] bytes, int offset, int length)
   {
-    throw new UnsupportedOperationException("read only memory buffer");
+    synchronized (breakpoints)
+      {
+	Iterator it;
+	it = breakpoints.getBreakpoints(index, index + length);
+	if (it.hasNext())
+	  throw new UnsupportedOperationException("breakpoint set between "
+						  + index + " and "
+						  + index + length);
+      }
+
+    return super.poke(index, bytes, offset, length);
   }
 
   protected ByteBuffer subBuffer(ByteBuffer parent,
Index: frysk-core/frysk/proc/live/TestByteBuffer.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/live/TestByteBuffer.java,v
retrieving revision 1.4
diff -u -r1.4 TestByteBuffer.java
--- frysk-core/frysk/proc/live/TestByteBuffer.java	21 Sep 2007 16:30:53 -0000	1.4
+++ frysk-core/frysk/proc/live/TestByteBuffer.java	21 Oct 2007 04:57:42 -0000
@@ -83,8 +83,6 @@
       // Cheat with the proc, it is not actually used if no
       // breakpoints are set (testing with breakpoints set is done through
       // TestTaskObserverCode in the various BreakpointMemoryView tests).
-      // LogicalMemory isn't writable atm, so we exclude it from those
-      // tests (the raw variant is of course).
       frysk.proc.Proc proc = new frysk.proc.dummy.Proc();
       BreakpointAddresses breakpoints = new BreakpointAddresses(proc);
       memorySpaceByteBuffer = new LogicalMemoryBuffer(pid,
@@ -179,8 +177,7 @@
     public void testModifyAddressBuffers()
     {
       for (int i = 0; i < addressBuffers.length; i++)
-	if (! (addressBuffers[i] instanceof LogicalMemoryBuffer))
-	  verifyModify(addressBuffers[i], LocalMemory.getCodeAddr());
+	verifyModify(addressBuffers[i], LocalMemory.getCodeAddr());
     }
 
     private void verifyAsyncModify(ByteBuffer buffer, long addr) {
@@ -236,9 +233,8 @@
 
     public void testAsyncAddressBuffers() {
       for (int i = 0; i < addressBuffers.length; i++)
-	if (! (addressBuffers[i] instanceof LogicalMemoryBuffer))
-	  verifyAsyncModify(addressBuffers[i],
-			    LocalMemory.getCodeAddr());
+	verifyAsyncModify(addressBuffers[i],
+			  LocalMemory.getCodeAddr());
     }
 
     public void verifyPeeks(ByteBuffer buffer, long addr, byte[] origBytes)
@@ -357,9 +353,8 @@
   public void testBulkPutAddressBuffers()
   {
     for (int i = 0; i < addressBuffers.length; i++)
-      if (! (addressBuffers[i] instanceof LogicalMemoryBuffer))
-	verifyBulkPut(addressBuffers[i], LocalMemory.getCodeAddr(),
-		      LocalMemory.getCodeBytes().length);
+      verifyBulkPut(addressBuffers[i], LocalMemory.getCodeAddr(),
+		    LocalMemory.getCodeBytes().length);
   }
 
 }