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

Remove Capstone 3 (fix CS 4 & 5) #3934

Merged
merged 19 commits into from
Oct 25, 2023
Merged

Remove Capstone 3 (fix CS 4 & 5) #3934

merged 19 commits into from
Oct 25, 2023

Conversation

Rot127
Copy link
Member

@Rot127 Rot127 commented Oct 21, 2023 •
edited
Loading

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

Addresses #3210

Since it is on a dist branch, this PR will also fix the bugs introduced with ARM and PPC CS v6 merge.

Test plan

All green.

Closing issues

closes #3210

@Rot127 Rot127 mentioned this pull request Oct 21, 2023
Closed
5 tasks
@Rot127 Rot127 force-pushed the dist-remove-capstone-v3 branch from 172b812 to d86d3c6 Compare October 21, 2023 18:06
@Rot127 Rot127 marked this pull request as ready for review October 22, 2023 15:23
@Rot127 Rot127 requested a review from Heersin October 22, 2023 15:23
@Rot127
Copy link
Member Author

Rot127 commented Oct 22, 2023

@Heersin Please check the changes in 13742eb

@Rot127 Rot127 force-pushed the dist-remove-capstone-v3 branch from 13742eb to b3a5fe8 Compare October 22, 2023 15:57
@Rot127
Copy link
Member Author

Rot127 commented Oct 22, 2023

The clang-format failed test is weird. I ran clang-format-18 on it with the result here (also looks more correct as before).
So probably a version mismatch between 16 and 18. Should I bump the version here as well? Or add a new PR for it.

XVilka
XVilka approved these changes Oct 22, 2023
View reviewed changes
Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

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

But for the clang-format it's better to stick to LLVM 16, I think. 18 version is unreleased, we can't rely on it.

@Rot127
Copy link
Member Author

Rot127 commented Oct 22, 2023

"endian" issue (which is none) is fixed with capstone-engine/capstone#2180

@Rot127 Rot127 force-pushed the dist-remove-capstone-v3 branch from aa7f226 to 972353b Compare October 23, 2023 14:02
@Rot127
Copy link
Member Author

Rot127 commented Oct 23, 2023

Really not ontop of the game currently. Missed this one: capstone-engine/capstone#2182

@Rot127 Rot127 force-pushed the dist-remove-capstone-v3 branch from 972353b to 084ae01 Compare October 25, 2023 04:27
@XVilka XVilka merged commit b16471f into dev Oct 25, 2023
54 of 55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wargio wargio wargio left review comments

@XVilka XVilka XVilka approved these changes

@thestr4ng3r thestr4ng3r Awaiting requested review from thestr4ng3r thestr4ng3r is a code owner

@kazarmy kazarmy Awaiting requested review from kazarmy kazarmy is a code owner

@ret2libc ret2libc Awaiting requested review from ret2libc ret2libc is a code owner

@Heersin Heersin Awaiting requested review from Heersin

Assignees
No one assigned
Labels
ARM infrastructure MIPS PPC RzAnalysis X86
Projects
None yet
Milestone
0.7.0
Development

Successfully merging this pull request may close these issues.

Drop Capstone 3 support
3 participants
@Rot127 @XVilka @wargio