-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat!: update rust bindings #2138
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
Conversation
This aligns with other architectures
This test did not actually test for anything before
This crate contains generated Rust bindings to the C library via bindgen. It is independent from the main `unicorn-engine` bindings, which will leverage this
I'm okay with break changes and the overall changes look good to me. Honestly I have a private |
`unicorn-engine-sys` will provide the necessary constants & types
56854e1
to
13fb413
Compare
I created a tiny placeholder crate: https://crates.io/crates/unicorn-engine-sys |
Great, I'm looking into the windows error |
These tests are copied over from the C tests
The size of `T` is not guaranteed to be the size of `i32` - all we know is that `T` is `Into<i32>`, so we should first copy them over into an `i32` array
The notable changes are migrating to `actions-rust-lang/setup-rust-toolchain` for setting up Rust as it's maintained, and using `katyo/publish-crates` for publishing crates in a workspace
So The problem in here is that for some reason, the On my machine, I get:
but on an Ubuntu machine, I get:
If I explicitly write 0 to the PPC register Anyway, this should be ready to go now 🙂 |
I can have a look at the crash and review the PR asap but expect a bit delay. |
Wow excellent improvement! Will the rust binding in unicornafl also be adjusted? (BTW, I don't know why that binding need to copy all codes instead of re-exporting unicorn-engine crate) |
Sure, I could update it after this PR's merged |
I have been refactoring unicornafl to rust recently: https://github.com/AFLplusplus/unicornafl/tree/v3 and it already works as a normal persistent target, i.e. can be fuzzed with afl-fuzz without unicorn mode.
@Evian-Zhang You can have a look if you are interested. Currently the last thing is that I need maturin to build a python bindings.
…________________________________
From: Amaan Qureshi ***@***.***>
Sent: Friday, April 4, 2025 7:57:02 AM
To: unicorn-engine/unicorn ***@***.***>
Cc: lazymio ***@***.***>; Comment ***@***.***>
Subject: Re: [unicorn-engine/unicorn] feat!: update rust bindings (PR #2138)
Sure, I could update it after this PR's merged
—
Reply to this email directly, view it on GitHub<#2138 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AHJULO24BPI75PEY6PEHQLT2XXDE5AVCNFSM6AAAAAB2CABBH6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONZXGIZTKNJXHE>.
You are receiving this because you commented.Message ID: ***@***.***>
[amaanq]amaanq left a comment (unicorn-engine/unicorn#2138)<#2138 (comment)>
Sure, I could update it after this PR's merged
—
Reply to this email directly, view it on GitHub<#2138 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AHJULO24BPI75PEY6PEHQLT2XXDE5AVCNFSM6AAAAAB2CABBH6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONZXGIZTKNJXHE>.
You are receiving this because you commented.Message ID: ***@***.***>
|
OK, got it. Thank you for your great work! Wow it uses LibAFL. But someone has recently make a unicorn mode into LibAFL (I think it is maybe AFLplusplus/LibAFL#1054), what is the relationship between these two codes? |
I was migrating unicornafl to unicorn_engine::ffi solely. If this PR gets merged, it will rely on unicorn-engine-sys instead. libafl_unicorn instead still uses full unicorn_engine. For functionalities, unicornafl provides a full forkserver compatible harness while libafl_unicorn just provides a few utilities if I understand it correctly. That said, I’m also planing to migrate a few internal functions of unicornafl to libafl_unicorn for code reuse. |
Note
This PR is quite large - fortunately, every commit is atomic in nature, and should be easy to review if gone in order
Problem
The current Rust bindings have a few problems:
uc_context
, and reading from ARM coprocessor registers-sys
crate for unsafe FFI bindingsSolution
First, there were a couple of fixes I made in the C code when tying the bindings to the C code with bindgen. I noticed
UC_MIPS_REG
was in all caps, whereas all the other architectures have the enum name in lowercase.Then, I introduced a
-sys
crate (unicorn-engine-sys
), which leverages bindgen to generate a bindings file, as well as some QoL implementations.Afterwards, I refactored the Rust bindings to use a workspace
Cargo.toml
, and switched to usingunicorn-engine-sys
in theunicorn-engine
crate.Following this, I've added some missing functionality in the Rust bindings, which I noticed was missing while I was porting the C tests:
Lastly, I fixed UB in the
reg_read/write_batch
methods -&[T]
was being casted to*mut i32
, which wasn't sound becauseT
wasn't known to be of the same size asi32
- I changed this to instead copy the array ofT
into an array ofi32
, and constrainedT
to beCopy
to guarantee that copying the array is cheap.Verification
Running
cargo test
in the repo root works as is, and runningcargo clippy
shows that there's no lints to be address. Also,bindings/rust/src/sys/bindings.rs
contains all the public types and constants in the corresponding C headers.Additional Notes
I've updated the CI implementation so that publishing should work with the workspace setup. The notable changes are:
actions-rust-lang/setup-rust-toolchain@v1
for setting up Rust, as it's actively maintainedkatyo/publish-crates@v2
for publishing, as it automatically handles workspaces & the order to publish crates.Also, this is a breaking change, because I've renamed a C enum type and some Rust enum types. Since this is a large rewrite of the Rust bindings, I went ahead with including breaking changes as it made the rewrite easier, and maintaining 100% compatibility with the old, inconsistent bindings would be a lot more work.