Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

qemu/tcg: fix UC_HOOK_MEM_READ on aarch64. #2028

Merged
merged 4 commits into from
Jan 4, 2025
Merged

qemu/tcg: fix UC_HOOK_MEM_READ on aarch64. #2028

merged 4 commits into from
Jan 4, 2025

Conversation

glennsec
Copy link
Contributor

Directly jump into the slow path when there is any hookmem enabled. This fixes #1908.

@glennsec glennsec mentioned this pull request Oct 11, 2024
Open
@glennsec glennsec force-pushed the hotfix/tcg_aarch64_hookmem branch from 646985e to 5b5a461 Compare October 11, 2024 19:33
@glennsec
qemu/tcg: fix UC_HOOK_MEM_READ on aarch64.
376c169 
Directly jump into the slow path when there is any hookmem enabled. This
fixes unicorn-engine#1908.

Signed-off-by: Glenn Baker <[email protected]>
@glennsec
qemu/tcg: fix UC_HOOK_MEM_READ on ppc64.
bfe3acb 
Directly jump into the slow path when there is any hookmem enabled.

Signed-off-by: Glenn Baker <[email protected]>
@glennsec glennsec force-pushed the hotfix/tcg_aarch64_hookmem branch from 5b5a461 to bfe3acb Compare October 22, 2024 12:52
@glennsec
Copy link
Contributor Author

Hi, code updated to cover ppc64 as well.

@PhilippTakacs
Copy link
Contributor

I think you missed at least UC_HOOK_MEM_READ_AFTER.

Also it would be nice to use HOOK_EXISTS_BOUNDED. This way the fast way is taken when no hook exists at the used address. But I don't know if this is possible at this place (you need the emulated physical address).

@glennsec
qemu/tcg: check for UC_HOOK_MEM_READ_AFTER.
d84208e 
Use has_hookmem() helper to determine wether "slow-path" TLB read is
needed. Add this helper to x86 architecture as well so that to check for
all hookmem.

Signed-off-by: Glenn Baker <[email protected]>
@glennsec
qemu/tcg: factor out has_hookmem().
7a5711c 
It's the same implementation for all architectures, so factor out
has_hookmem() into tcg_uc_has_hookmem().

Signed-off-by: Glenn Baker <[email protected]>
@glennsec
Copy link
Contributor Author

I think you missed at least UC_HOOK_MEM_READ_AFTER.

Also it would be nice to use HOOK_EXISTS_BOUNDED. This way the fast way is taken when no hook exists at the used address. But I don't know if this is possible at this place (you need the emulated physical address).

Added check for UC_HOOK_MEM_READ_AFTER, including for i386. Factored out the code along the way. I am not sure either I can use HOOK_EXISTS_BOUNDED() as TCGContext.pc_start is set to the end of the BB at the point we call it for codegen.

@droe
Copy link
Contributor

droe commented Jan 3, 2025

fwiw, I'm using this fix as part of a bug fix stable branch on top of the 2.1.1 release. It resolves #1908 for me on arm64 without any issues so far.

What is needed to get this merged?

@wtdcode wtdcode merged commit 8442eb6 into unicorn-engine:dev Jan 4, 2025
36 checks passed
@wtdcode
Copy link
Member

wtdcode commented Jan 4, 2025

Sorry for the delay. Thanks for the contributions!

@wtdcode wtdcode mentioned this pull request Jan 4, 2025
Closed
hyunmin-furiosa pushed a commit to hyunmin-furiosa/unicorn that referenced this pull request Jan 17, 2025
@glennsec @hyunmin-furiosa
qemu/tcg: fix UC_HOOK_MEM_READ on aarch64. (unicorn-engine#2028)
203056c 
* qemu/tcg: fix UC_HOOK_MEM_READ on aarch64.

Directly jump into the slow path when there is any hookmem enabled. This
fixes unicorn-engine#1908.

Signed-off-by: Glenn Baker <[email protected]>

* qemu/tcg: fix UC_HOOK_MEM_READ on ppc64.

Directly jump into the slow path when there is any hookmem enabled.

Signed-off-by: Glenn Baker <[email protected]>

* qemu/tcg: check for UC_HOOK_MEM_READ_AFTER.

Use has_hookmem() helper to determine wether "slow-path" TLB read is
needed. Add this helper to x86 architecture as well so that to check for
all hookmem.

Signed-off-by: Glenn Baker <[email protected]>

* qemu/tcg: factor out has_hookmem().

It's the same implementation for all architectures, so factor out
has_hookmem() into tcg_uc_has_hookmem().

Signed-off-by: Glenn Baker <[email protected]>

---------

Signed-off-by: Glenn Baker <[email protected]>
@glennsec glennsec deleted the hotfix/tcg_aarch64_hookmem branch January 17, 2025 13:17
glennsec added a commit to glennsec/unicorn that referenced this pull request Jan 17, 2025
@glennsec
qemu/tcg: fix UC_HOOK_MEM_READ on aarch64. (unicorn-engine#2028)
1b54ab9 
* qemu/tcg: fix UC_HOOK_MEM_READ on aarch64.

Directly jump into the slow path when there is any hookmem enabled. This
fixes unicorn-engine#1908.

Signed-off-by: Glenn Baker <[email protected]>

* qemu/tcg: fix UC_HOOK_MEM_READ on ppc64.

Directly jump into the slow path when there is any hookmem enabled.

Signed-off-by: Glenn Baker <[email protected]>

* qemu/tcg: check for UC_HOOK_MEM_READ_AFTER.

Use has_hookmem() helper to determine wether "slow-path" TLB read is
needed. Add this helper to x86 architecture as well so that to check for
all hookmem.

Signed-off-by: Glenn Baker <[email protected]>

* qemu/tcg: factor out has_hookmem().

It's the same implementation for all architectures, so factor out
has_hookmem() into tcg_uc_has_hookmem().

Signed-off-by: Glenn Baker <[email protected]>

---------

Signed-off-by: Glenn Baker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@glennsec @PhilippTakacs @droe @wtdcode