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

Disable ARM hacks for Capstone 6 #4159

Merged
merged 1 commit into from
Jan 27, 2024
Merged

Conversation

XVilka
Copy link
Member

@XVilka XVilka commented Jan 26, 2024

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
  • I've added tests that prove my fix is effective or that my feature works (if possible)
  • I've updated the rizin book with the relevant information (if needed)

Detailed description

After auto-sync updates in Capstone, there is no need for those anymore. Moreover, they were implemented inefficiently, so that it leads to a small increase in performance (5-15% depending on the file).

Test plan

CI is green

@XVilka XVilka added this to the 0.7.0 milestone Jan 26, 2024
@XVilka XVilka requested a review from wargio as a code owner January 26, 2024 10:59
@github-actions github-actions bot added the ARM label Jan 26, 2024
@XVilka

This comment was marked as resolved.

@wargio
Copy link
Member

wargio commented Jan 26, 2024

That fail on ccmp kinda make sense.

-cmpp sp, x13
+subps xzr, sp, x13

@XVilka XVilka requested a review from Rot127 January 26, 2024 15:14
@Rot127
Copy link
Member

Rot127 commented Jan 27, 2024

Output of llvm matches with the new one:

> echo "<bytes>" | llvm-mc-18 --triple=arm64 --mattr=mte --disassemble --print-imm-hex --show-encoding
	stzg	x13, [x3, #0x0]!                // encoding: [0x6d,0x0c,0x60,0xd9]
	stzg	x13, [x3]                       // encoding: [0x6d,0x08,0x60,0xd9]
	ldg	x13, [x3]                       // encoding: [0x6d,0x00,0x60,0xd9]
	stg	x13, [x3], #0x0                 // encoding: [0x6d,0x04,0x20,0xd9]
	subps	xzr, sp, x13                    // encoding: [0xff,0x03,0xcd,0xba]
	addg	x12, x8, #0x200, #0x2           // encoding: [0x0c,0x09,0xa0,0x91]
	addg	x0, sp, #0x80, #0x4             // encoding: [0xe0,0x13,0x88,0x91]

So they are correct.

Copy link
Member

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the tests and we are good to go

@wargio
Copy link
Member

wargio commented Jan 27, 2024

@XVilka you should probably also fix the capstone hacks and output the correct subps xzr

@XVilka
Copy link
Member Author

XVilka commented Jan 27, 2024

@XVilka you should probably also fix the capstone hacks and output the correct subps xzr

I don't want to invest time into something that will be relatively soon deleted, to be honest.

@XVilka XVilka merged commit 6c016fc into dev Jan 27, 2024
57 checks passed
@XVilka XVilka deleted the dist-asan-fuzz-disable-arm-hacks branch January 27, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants