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

Exercise all bpf instructions #552

Merged
merged 4 commits into from
Apr 30, 2024
Merged

Exercise all bpf instructions #552

merged 4 commits into from
Apr 30, 2024

Conversation

seanyoung
Copy link

This tests checks that all the alu instructions have the same output for jit and interpreted.

@codecov-commenter
Copy link

codecov-commenter commented Feb 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (179a0f9) 88.34% compared to head (b1fdeb9) 89.43%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #552      +/-   ##
==========================================
+ Coverage   88.34%   89.43%   +1.08%     
==========================================
  Files          24       23       -1     
  Lines       10282     9931     -351     
==========================================
- Hits         9084     8882     -202     
+ Misses       1198     1049     -149     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@seanyoung seanyoung force-pushed the fuzz branch 2 times, most recently from 4180b00 to b1fdeb9 Compare February 18, 2024 10:04
@seanyoung seanyoung requested a review from Lichtso February 18, 2024 10:47
@Lichtso
Copy link

Lichtso commented Feb 19, 2024

Looks good in general. However, we already have a bunch of fuzzers all with different interfaces, capabilities, etc. Would be nice to have a more structured approach instead of many individual ad-hoc solutions.

@seanyoung
Copy link
Author

Looks good in general. However, we already have a bunch of fuzzers all with different interfaces, capabilities, etc. Would be nice to have a more structured approach instead of many individual ad-hoc solutions.

What do you have in mind?

I can extend this test to include branch tests and load/store tests, which would cover almost all of the instructions. I also feel this would be a useful addition for any future arm64 jitter.

@LucasSte
Copy link

Is there any updates on the status of this PR? I was looking forward to using this fuzzer.

@LucasSte
Copy link

LucasSte commented Feb 29, 2024

I believe the PR is really good to ensure the consistency between the jitter and the interpreter and I thought it would be nice to even cover the SBFv1 instructions. There are instructions that the compiler does not emit today, but that the RPBF crate supports, so having this test ensures everything works in tandem if we were to enable their emission.

@seanyoung seanyoung changed the title Fuzz the alu 64/32 instructions Exercise all bpf instructions Apr 8, 2024
@seanyoung
Copy link
Author

@Lichtso @LucasSte I've updated the PR, now all instructions are exercised.

tests/exercise_instructions.rs Show resolved Hide resolved
tests/exercise_instructions.rs Outdated Show resolved Hide resolved
tests/exercise_instructions.rs Show resolved Hide resolved
exit"
);

test_interpreter_and_jit_asm!(asm.as_str(), input, (), TestContextObject::new(cu));
Copy link

Choose a reason for hiding this comment

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

Are we testing call and exit?

Copy link
Author

Choose a reason for hiding this comment

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

That's true, however testing those is a bit trivial/uninteresting

@Lichtso Lichtso merged commit f21dbb6 into solana-labs:main Apr 30, 2024
12 checks passed
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.

4 participants