Page MenuHomePhabricator

[Process] Fix WriteMemory return value
ClosedPublic

Authored by mib on Mar 29 2019, 6:36 PM.

Details

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 <medismail.bennani@gmail.com>

Event Timeline

mib created this revision.Mar 29 2019, 6:36 PM
mib edited the summary of this revision. (Show Details)Mar 29 2019, 6:47 PM
mib added reviewers: jasonmolenda, friss, jingham.
mib set the repository for this revision to rLLDB LLDB.
mib added subscribers: Restricted Project, lldb-commits.
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2019, 6:47 PM
davide added a subscriber: davide.Mar 29 2019, 6:51 PM
davide added inline comments.
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py
19–20

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 added inline comments.
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py
10

Why does this test depend on XML support?

mib updated this revision to Diff 192954.EditedMar 29 2019, 6:59 PM

Removed TraceOn and XMLSupport Check

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 marked an inline comment as done.Apr 1 2019, 11:26 AM
jasonmolenda added inline comments.
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py
19–20

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.

mib marked 2 inline comments as done.Apr 1 2019, 11:28 AM
davide accepted this revision.Apr 1 2019, 11:58 AM

LGTM

This revision is now accepted and ready to land.Apr 1 2019, 11:58 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2019, 12:07 PM
labath added a subscriber: labath.Apr 2 2019, 1:59 AM

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

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).