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

test: add vm-level fuzzing #5462

Merged
merged 4 commits into from
Nov 30, 2021
Merged

Conversation

matklad
Copy link
Contributor

@matklad matklad commented Nov 25, 2021

Finally got to publishing fuzzing infra we have been using in ad-hoc ways before. This just runs the contracts to make sure that VM doesn't crash. The code is setup to easily compare execution with different VM Kinds.

Note that at the moment fuzzing does not generally exercise host function calls. I need to finish bytecodealliance/wasm-tools#286 for that 😅

Copy link
Collaborator

@nagisa nagisa left a comment

Choose a reason for hiding this comment

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

This seems reasonable to me. I'm amazed how well arbitrary has caught on that there's even an implementation of Arbitrary to generate wasm modules!

}

fn create_context(input: Vec<u8>) -> VMContext {
VMContext {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Aren't some of these potentially controlled by the user? Would be ideal if the user-influenced parts were Arbitrary too. Potentially as a separate fuzz target?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's a good idea. Let me issue this... #5465

@mina86
Copy link
Contributor

mina86 commented Nov 25, 2021

pytest and nightly changes LGTM

@matklad matklad force-pushed the m/runner-fuzzer branch 3 times, most recently from 641529e to d200ea8 Compare November 26, 2021 16:58
def run(dir: str, fuzz_target: str) -> int:
args = ('cargo', 'fuzz', 'run', fuzz_target, '--', '-len_control=0'
'-prefer_small=0', '-max_len=4000000', '-rss_limit_mb=10240')
os.environ['RUSTC_BOOTSTRAP'] = '1'
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, why it's needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fuzzing is an unstable features -- it can break with new compiler releases. So we need to opt-into using unstable features.

The official way to do this is via running a nightly toolchain, but that means that you need a separate nightly toolchain, and you need to manually correlate it with the stable version (you don't want to use current nightly, you want the nightly from the night where the stable release was cut).

The unofficial way to do this is to set this magical env var, which just unlocks all nightly features on stable. It is used by the rust compiler itself for bootstrapping: compiler internally uses nightly Rust features, but it can be build with the previous release.

Copy link
Contributor Author

@matklad matklad Nov 29, 2021

Choose a reason for hiding this comment

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

To spell this explicitly, by butting fuzzing into our CI we make it our responsibility to deal with any breakage related to compiler version upgrades. This in contrast to all over Rust functionality we use -- if something stable&documented breaks with an upgrade, we can and should complain upstream about that.

[package]
name = "near-vm-runner-fuzz"
version = "0.0.0"
authors = ["Automatically generated"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is. Though as authors field is now optional, it's better to remove it here (and upstream in the generator itself as well, cc @nagisa ).

@near-bulldozer near-bulldozer bot merged commit 5011d28 into near:master Nov 30, 2021
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