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

Add fuzz scripts #281

Merged
merged 1 commit into from Feb 26, 2017
Merged

Add fuzz scripts #281

merged 1 commit into from Feb 26, 2017

Conversation

@Manishearth
Copy link
Member

Manishearth commented Feb 21, 2017

Add a simple script for fuzzing with https://github.com/rust-fuzz/cargo-fuzz

Mostly autogenerated by cargo-fuzz; the contents of go() in parser.rs were
manually written.

cargo-fuzz is still WIP so the format here may change.

This can be fuzzed (on Linux only) with cargo fuzz --target parser

r? @SimonSapin


This change is Reviewable

Copy link
Member

SimonSapin left a comment

r=me with two changes.

path = "fuzzers/parse.rs"

[workspace]
members = ["."]

This comment has been minimized.

@SimonSapin

SimonSapin Feb 26, 2017

Member

Instead of creating a new workspace here, this crate should be added to the workspace at the root of this repository.

@@ -0,0 +1,3 @@

target
libfuzzer

This comment has been minimized.

@SimonSapin

SimonSapin Feb 26, 2017

Member

With the shared workspace (see other comment) there shouldn’t be a target directory here. As to libfuzzer, I’d prefer it to be fuzz/libfuzzer in .gitignore at the root of the repo.

@Manishearth
Copy link
Member Author

Manishearth commented Feb 26, 2017

Both those files are autogenerated by cargo-fuzz (the workspace=. is a generic solution so that the fuzz crate doesn't break when placed within a system that has an arbitrary workspace setup). It's better to not share target directories since we build with RUSTFLAGS and this may end up polluting dependencies in the regular target dir with sanitizer coverage. IIRC rustc does not rebuild when rustflags change.

@SimonSapin
Copy link
Member

SimonSapin commented Feb 26, 2017

I think Cargo does rebuild when RUSTFLAGS changes, but ok.

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Feb 26, 2017

📌 Commit 82ccae4 has been approved by SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Feb 26, 2017

Testing commit 82ccae4 with merge a03542e...

bors-servo added a commit that referenced this pull request Feb 26, 2017
Add fuzz scripts

Add a simple script for fuzzing with https://github.com/rust-fuzz/cargo-fuzz

Mostly autogenerated by cargo-fuzz; the contents of `go()` in parser.rs were
manually written.

cargo-fuzz is still WIP so the format here may change.

This can be fuzzed (on Linux only) with `cargo fuzz --target parser`

r? @SimonSapin

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-url/281)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 26, 2017

☀️ Test successful - status-travis
Approved by: SimonSapin
Pushing a03542e to master...

@bors-servo bors-servo merged commit 82ccae4 into servo:master Feb 26, 2017
3 of 4 checks passed
3 of 4 checks passed
code-review/reviewable 3 files, 2 discussions left (SimonSapin)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.