Fixed invalid addresses in read hooks #70
Merged
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
saibotk
force-pushed
the
fix-read-addresses
branch
2 times, most recently
from
215a55d to
84dc5ff
Compare
When using the new QEMU version, the read hooks sometimes returned invalid addresses like 0x1 or 0x1000. For reference this commit in upstream QEMU is relevant, where the handling for addresses was adjusted: https://gitlab.com/qemu-project/qemu/-/commit/d5920b7280762d4d696bff87f50dbce453adef06 And in https://gitlab.com/qemu-project/qemu/-/commit/eb9d02f24b1ce877a60ffaf6cc1ecc8484740b37 we can see, that QEMU temporarily stores the address in a TCGTemp variable, because it can get corrupted by the operation via overlaps. To mimic QEMUs behavior, we copied the `plugin_maybe_preserve_addr` function, and used it in the same places as QEMU also does for its memory plugin callbacks. Note that we copied this and technically repeat the same code, but QEMU disables all those functions when the plugin feature is disabled. And we do not add extra logic that is found in the plugin mem callback generation function `plugin_gen_mem_callbacks` because in those specific cases the function will always just use the preserved address anyway. The function just contains the same logic as a fallback when no copy_addr was given as a parameter. Additionally, we are now correctly using the original memop variable in the same way the plugin callback does. This should now be more consistent.
rmalmain
force-pushed
the
fix-read-addresses
branch
from
84dc5ff to
f029f47
Compare
|
Looks good to me, thanks for the PR |
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.
Suggestion cannot be applied right now. Please check back later.
When using the new QEMU version, the read hooks sometimes returned invalid addresses like 0x1 or 0x1000.
For reference this commit in upstream QEMU is relevant, where the handling for addresses was adjusted: https://gitlab.com/qemu-project/qemu/-/commit/d5920b7280762d4d696bff87f50dbce453adef06
And in https://gitlab.com/qemu-project/qemu/-/commit/eb9d02f24b1ce877a60ffaf6cc1ecc8484740b37 we can see, that QEMU temporarily stores the address in a TCGTemp variable, because it can get corrupted by the operation via overlaps.
To mimic QEMUs behavior, we copied the
plugin_maybe_preserve_addrfunction, and used it in the same places as QEMU also does for its memory plugin callbacks.Note that we copied this and technically repeat the same code, but QEMU disables all those functions when the plugin feature is disabled.
And we do not add extra logic that is found in the plugin mem callback generation function
plugin_gen_mem_callbacksbecause in those specific cases the function will always just use the preserved address anyway. The function just contains the same logic as a fallback when no copy_addr was given as a parameter.Additionally, we are now correctly using the original memop variable in the same way the plugin callback does. This should now be more consistent.