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

RISCV basic support #1318

Open
wants to merge 2 commits into
base: dev-v1.0
Choose a base branch
from
Open

Conversation

Antwy
Copy link
Contributor

@Antwy Antwy commented Apr 3, 2024

Hi! Added basic support for RISCV instruction set. This covers most of IMC standard ISA extensions for RV32 & RV64.
It would be great if you could give some review and merge it.

Some details:

  • The provided architecture(ARCH_RVxx) contains compressed instructions by default
  • As RV32 is a subset of RV64 (except 1 instruction), both of them are situated in a single namespace riscv alike x86, the difference is the register sizes
  • RISCV in Capstone needs refactoring so it can fail to process some instructions (issue #2278)
  • As Capstone doesn't have enumeration of pseudo instructions yet (like alias id for arm), they are detected while building semantics of corresponding instruction

@JonathanSalwan
Copy link
Owner

JonathanSalwan commented Apr 3, 2024

Awesome. Thanks for a such MR. Let me few weeks to review this. Can you try to fix CIs?

@JonathanSalwan
Copy link
Owner

JonathanSalwan commented Apr 3, 2024

@cnheitman can you take a look at this too so that we have at least two reviews for a such MR?

@cnheitman
Copy link
Collaborator

Awesome! Great PR!

@JonathanSalwan Yes, I'll review it, most probably sometime next week.

@Antwy Antwy force-pushed the riscv-support branch 2 times, most recently from efccaaa to b38a401 Compare April 4, 2024 11:39
@Antwy
Copy link
Contributor Author

Antwy commented Apr 4, 2024

Well, I guess one way to fix CI is to update Capstone version from 4.0.2 to 5.0+. But if you need the older version, I can try to put riscv code under defines

@cnheitman
Copy link
Collaborator

The PR looks good, great work @Antwy o/

This PR was based on master (I think) and does not include the commit upgrading Bitwuzla from v0.2.0 to v0.4.0 of dev-v1.0. However, there should be no conflicts rebasing on top of that change.

I did not dive much into the semantics file. On a quick overview they look good. If we want to increase our confidence on the code, we can add some more tests (for instance, we have a binary with different optimization levels which we use to test the ARM semantics). However, basic unittests were included so it seems fine for now.

Ideally, I would add a RV32 and RV64 version of this custom crackme so we can have an example of a full working binary, as we do with AARCH64 and ARM.

Regarding the CI and Capstone. I think we can move on to version 5.0.1. Version 4.0.2 is from 2020 (iirc) and as far as I can tell we don't have any specific reason to keep supporting it.

@Antwy
Copy link
Contributor Author

Antwy commented May 3, 2024

Now rebased on dev-v1.0 and some semantics issues fixed (but still not sure in taint spreading variants).
Adding binary test seems reasonable as basic unittest suite doesn't cover controlflow transfers.

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