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 for incorrect register usages in ARM64 MOV/MOVN instructions. #1977

Closed

Conversation

stevielavern
Copy link
Contributor

As reported by issues #1652 and #1591 there were issues in register usages reported by cstool (and associated APIs):

./cstool -d arm64 '00 00 80 D2'
 0  00 00 80 d2  mov	x0, #0
	ID: 488 (mov)
	op_count: 2
		operands[0].type: REG = x0
		operands[0].access: READ | WRITE  // should be WRITE only
		operands[1].type: IMM = 0x0
	Registers read: x0
	Registers modified: x0

This PR fixes the issue and adds a unit test.
Like most other register usage issues, the problem was arch/AArch64/AArch64MappingInsnOp.inc and it would be better to fix the script that was used to generate it, but it's sadly not committed as part of Capstone.

@thomasdangl
Copy link

Thanks! This might be a duplicate of #1974.

I think my PR covers more affected instructions, however, I do not currently have a unit test. Perhaps it would make sense to combine both PRs? : )

Best regards,
Thomas

@stevielavern
Copy link
Contributor Author

Ah yes indeed, that's a PR collision :). Yours definitely covers more things, maybe it makes more sense to use it. Do you want to reuse my unit test? It should hopefully be quite easy to adapt. Thanks!

@thomasdangl
Copy link

Sounds good! I have added your unit test to my PR and included some more interesting test cases (aliased instructions and instructions with system registers as operands). Thanks again for your work!

@kabeor
Copy link
Member

kabeor commented Mar 19, 2023

Thanks again. This PR will be closing cause #1974.

@kabeor kabeor closed this Mar 19, 2023
@stevielavern stevielavern deleted the fix_arm64_accesses_mov branch March 20, 2023 14:28
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.

None yet

3 participants