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.

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

fix(arm): correct write to ARM coprocessor #2099

Merged
merged 1 commit into from
Feb 13, 2025

Conversation

amaanq
Copy link
Contributor

@amaanq amaanq commented Feb 11, 2025

Problem

Any attempt to write to UC_ARM_REG_C1_C0_2 (The ARM Coprocessor Access Control Register) fails with UC_ERR_ARG.
The specific switch case handling writes to this register was commented out since 2021, but by default, the error code was initialized to UC_REG_OK, so there was no error returned until #1835, where this was changed to be initialized to UC_REG_ERR_ARG. As a result, any write to UC_ARM_REG_C1_C0_2 returned an error.

Solution

In this PR, I've uncommented and properly implement the CPACR register write with appropriate size checking. I've also added a unit test to verify that writes no longer fail - before this PR, the test does indeed fail.

@wtdcode
Copy link
Member

wtdcode commented Feb 11, 2025

Looks like commenting out by accident? Btw, you should target dev branch.

@amaanq amaanq changed the base branch from master to dev February 11, 2025 02:52
This code was commented out since 2021, but by default, the error
codewas initialized to `UC_REG_OK`, so there was no error returned
untila result, any write to `UC_ARM_REG_C1_C0_2` returned an error.
@amaanq amaanq force-pushed the arm-coprocessor-write branch from 6233b76 to 952794c Compare February 11, 2025 02:55
@amaanq
Copy link
Contributor Author

amaanq commented Feb 11, 2025

Done, thanks

@wtdcode
Copy link
Member

wtdcode commented Feb 13, 2025

Will merge after I get CI fixed. Please wait a bit.

@wtdcode wtdcode merged commit 6b9c1c8 into unicorn-engine:dev Feb 13, 2025
36 checks passed
@wtdcode
Copy link
Member

wtdcode commented Feb 13, 2025

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants