[Process] Fix WriteMemory return value

[mib]

Summary

In case of a breakpoint site overlapping with the destination address,
the WriteMemory method reported an incorrect memory size.

Instead of returning the right amount of bytes written, it falls through
the scope and returned 0.

Signed-off-by: Med Ismail Bennani <[e-mail]>

[davide]

packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py

19–20 ↗

(On Diff #192951)

I'm not sure you really need this, we can commit it if we need to debug. Please note that the bots always run with TraceOn() == True.

[JDevlieghere]

packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py

10 ↗

(On Diff #192951)

Why does this test depend on XML support?

[mib]

Removed TraceOn and XMLSupport Check

[JDevlieghere]

Thanks Ismail, the patch LGTM, assuming I understand the code correctly.

FWIW I think this function is a good example of how early returns might prevent something like this from happening in the future. If you agree and have the bandwidth, feel free to refactor it, either as part of this patch or as a separate patch.

[jasonmolenda]

packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py

19–20 ↗

(On Diff #192951)

no big deal, but fwiw I put this sequence in every time I write a gdb remote client test - it's essential for debugging any problems with them. The amount of output is very small, it's not like a real debug session.

[davide]

LGTM

[labath]

Just an FYI: I was hitting llvm_unreachable in Platform::GetSoftwareBreakpointTrapOpcode in this test. I fixed it in r357456 by specifying an explicit triple in the test. (I'm not sure how this wasn't hit on the darwin bots.)

packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py

19–20 ↗

(On Diff #192951)

I think it would make sense to teach the general architecture of these tests (I guess that would be the MockGDBServer class) to dump some additional info in the TraceOn() case.

BTW, when running a test locally, you can pass --channel "gdb-remote packets" to dotest to have it output the relevant logs to a file (the file will go to the test traces directory).

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py

+from __future__ import print_function
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestWriteMemory(GDBRemoteTestBase):
+
+    def test(self):
+
+        class MyResponder(MockGDBServerResponder):
+            def setBreakpoint(self, packet):
+                return "OK"
+
+        self.server.responder = MyResponder()
+        target = self.dbg.CreateTarget('')
+        process = self.connect(target)
+
+        bp = target.BreakpointCreateByAddress(0x1000)
+        self.assertTrue(bp.IsValid())
+        self.assertEqual(bp.GetNumLocations(), 1)
+        bp.SetEnabled(True)
+        self.assertTrue(bp.IsEnabled())
+
+        err = lldb.SBError()
+        data = str("\x01\x02\x03\x04")
+        result = process.WriteMemory(0x1000, data, err)
+        self.assertEqual(result, 4)

lldb/trunk/source/Target/Process.cpp

else {
           // directly into the breakpoint opcode buffer
           ::memcpy(bp->GetSavedOpcodeBytes() + opcode_offset,
                    ubuf + bytes_written, intersect_size);
           bytes_written += intersect_size;
         }
       });
 
       if (bytes_written < size)
-        WriteMemoryPrivate(addr + bytes_written, ubuf + bytes_written,
+        return bytes_written + WriteMemoryPrivate(addr + bytes_written,
+                                                  ubuf + bytes_written,
                                                   size - bytes_written, error);
     }
   } else {
     return WriteMemoryPrivate(addr, buf, size, error);
   }
 
   // Write any remaining bytes after the last breakpoint if we have any left
   return 0; // bytes_written;
 }