gdbstub: only let Execute breakpoints write/restore BKPT opcodes into memory #4603
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wwylele
approved these changes
Jan 28, 2019
Code LGTM. I am not very familiar with gdbstub. Anyone else with more experience can give a review?
zhaowenlan1779
approved these changes
Jan 28, 2019
The code looks good, but I don't have much experience with gdbstub either :p
Luffykun007
approved these changes
Jan 31, 2019
bunnei
added a commit
to yuzu-emu/yuzu
that referenced
this issue
Feb 7, 2019
Port citra-emu/citra#4603: "gdbstub: only let Execute breakpoints write/restore BKPT opcodes into target memory"
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Back in August last year, commit bd658a8 implemented logic for code breakpoints to be hit via a
bkpt 0opcode that temporarily overwrites the targeted instruction (by theCommitBreakpoint/RemoveBreakpointfunctions). However, this is also done inadvertently for watchpoints (Read-, Write-, or Access-type breakpoint types), which can corrupt data used by the software (#4502). This PR adds a simple check to both functions.Note to users/testers: Currently, CPU JIT isn't designed around memory watchpoints, so make sure it's off.
Speaking of which, although my MS2017 debugger reveals that watchpoints are being hit internally (i.e. calls
CheckMemoryBreakpoint(VAddr, type)->GDBStub::Break(true)), the deferred logic I'm seeing (in the interpreter loop?) seems to do nothing: no signal is sent to the GDB client. and the game keeps running.This other issue is separate from the issue this PR fixes (#4502, even though the OP and I have also mentioned it there) and exists on latest master on my Windows release build. I use the
arm-none-eabi-gdbclient, and recall getting watchpoints to fire on Luma3ds (real 3DS).This change is