Add a single file compilation fuzzer

bshastry commented 10 months ago

What was wrong?

Single file compilation fuzzer was missing.

How was it fixed?

Adding a fuzzer harness.

To-Do

[ ] Fix build: Currently building a fuzzer library, what we need is a binary
[ ] Add entry to the release notes (may forgo for trivial changes)
[ ] Clean up commit history

sbillig commented 9 months ago

@bshastry The build process and fuzzer both seem to work fine without the changes you made to Cargo.toml. I'm doing cargo +nightly fuzz run single_file_fuzzer (with the latest nightly). Is there something else that isn't working that required the Cargo.toml changes?

bshastry commented 9 months ago

@bshastry The build process and fuzzer both seem to work fine without the changes you made to Cargo.toml. I'm doing cargo +nightly fuzz run single_file_fuzzer (with the latest nightly). Is there something else that isn't working that required the Cargo.toml changes?

You are right, I'm not sure what I did differently that required an update. Perhaps I did a rustup default nightly followed by cargo fuzz build... (assuming cargo +nightly fuzz is equivalent to rustup default nightly followed by cargo fuzz) that may have errored on parsing the legacy root Cargo.toml but I'm not exactly sure.

I updated this PR undoing changes to the root Cargo.toml.

sbillig commented 9 months ago

When I build this locally, the following changes are automatically applied to the Cargo.lock file. This is presumably the Cargo.lock file that the CI is expecting to see. I'm not sure why the Cargo.lock file isn't being modified in the same way on your machine. Maybe delete Cargo.lock (and maybe run cargo clean), build again, and see what's generated?

diff --git a/Cargo.lock b/Cargo.lock
index fe0e8426..bd61b124 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -87,12 +87,6 @@ dependencies = [
  "syn 2.0.39",
 ]

-[[package]]
-name = "arbitrary"
-version = "1.3.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7d5a26814d8dcb93b0e5a0ff3c6d80a8843bafb21b39e8e18a6f05471870e110"
-
 [[package]]
 name = "ark-ff"
 version = "0.3.0"
@@ -440,7 +434,6 @@ version = "1.0.83"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "f1174fb0b6ec23863f8b971027804a42614e347eafb0a95bf0b12cdae21fc4d0"
 dependencies = [
- "jobserver",
  "libc",
 ]

@@ -1213,16 +1206,6 @@ dependencies = [
  "vfs",
 ]

-[[package]]
-name = "fe-fuzzers"
-version = "0.26.0"
-dependencies = [
- "dir-test",
- "fe-common",
- "fe-driver",
- "libfuzzer-sys",
-]
-
 [[package]]
 name = "fe-library"
 version = "0.26.0"
@@ -1824,15 +1807,6 @@ version = "1.0.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "af150ab688ff2122fcef229be89cb50dd66af9e01a4ff320cc137eecc9bacc38"

-[[package]]
-name = "jobserver"
-version = "0.1.27"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8c37f63953c4c63420ed5fd3d6d398c719489b9f872b9fa683262f8edd363c7d"
-dependencies = [
- "libc",
-]
-
 [[package]]
 name = "js-sys"
 version = "0.3.66"
@@ -1885,17 +1859,6 @@ version = "0.2.150"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "89d92a4743f9a61002fae18374ed11e7973f530cb3a3255fb354818118b2203c"

-[[package]]
-name = "libfuzzer-sys"
-version = "0.4.7"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a96cfd5557eb82f2b83fed4955246c988d331975a002961b07c81584d107e7f7"
-dependencies = [
- "arbitrary",
- "cc",
- "once_cell",
-]
-
 [[package]]
 name = "libloading"
 version = "0.7.4"

sbillig commented 9 months ago

@bshastry I should have mentioned earlier that most of the compiler is in the process of being rewritten, on the fe-v2 branch. It would be more useful to add the fuzzer to that branch. The fe-v2 branch is expected to be ready for use in a few months, and we likely won't fix minor issues on the master branch. (That said, it would be useful to add any bugs you find to our test suite, to ensure that the same problems don't exist on the fe-v2 branch)

However, the fe-v2 branch driver crate (currently called driver2) doesn't have a simple entry point. You could add one to start fuzzing the new parser at least. The other components are maybe not quite ready for fuzzing, but some major changes should be merged into the fe-v2 branch someday soon.

bshastry commented 9 months ago

When I build this locally, the following changes are automatically applied to the Cargo.lock file. This is presumably the Cargo.lock file that the CI is expecting to see. I'm not sure why the Cargo.lock file isn't being modified in the same way on your machine. Maybe delete Cargo.lock (and maybe run cargo clean), build again, and see what's generated?

diff --git a/Cargo.lock b/Cargo.lock
index fe0e8426..bd61b124 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -87,12 +87,6 @@ dependencies = [
  "syn 2.0.39",
 ]

-[[package]]
-name = "arbitrary"
-version = "1.3.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7d5a26814d8dcb93b0e5a0ff3c6d80a8843bafb21b39e8e18a6f05471870e110"
-
 [[package]]
 name = "ark-ff"
 version = "0.3.0"
@@ -440,7 +434,6 @@ version = "1.0.83"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "f1174fb0b6ec23863f8b971027804a42614e347eafb0a95bf0b12cdae21fc4d0"
 dependencies = [
- "jobserver",
  "libc",
 ]

@@ -1213,16 +1206,6 @@ dependencies = [
  "vfs",
 ]

-[[package]]
-name = "fe-fuzzers"
-version = "0.26.0"
-dependencies = [
- "dir-test",
- "fe-common",
- "fe-driver",
- "libfuzzer-sys",
-]
-
 [[package]]
 name = "fe-library"
 version = "0.26.0"
@@ -1824,15 +1807,6 @@ version = "1.0.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "af150ab688ff2122fcef229be89cb50dd66af9e01a4ff320cc137eecc9bacc38"

-[[package]]
-name = "jobserver"
-version = "0.1.27"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8c37f63953c4c63420ed5fd3d6d398c719489b9f872b9fa683262f8edd363c7d"
-dependencies = [
- "libc",
-]
-
 [[package]]
 name = "js-sys"
 version = "0.3.66"
@@ -1885,17 +1859,6 @@ version = "0.2.150"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "89d92a4743f9a61002fae18374ed11e7973f530cb3a3255fb354818118b2203c"

-[[package]]
-name = "libfuzzer-sys"
-version = "0.4.7"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a96cfd5557eb82f2b83fed4955246c988d331975a002961b07c81584d107e7f7"
-dependencies = [
- "arbitrary",
- "cc",
- "once_cell",
-]
-
 [[package]]
 name = "libloading"
 version = "0.7.4"

I applied this patch locally and force pushed to the source branch. Now there are failing tests in the macOS CI. So definitely progress :-)

I hope the failing tests have nothing to do with the changes in this PR, at least that's what I feel after taking a cursory look at the failures in that CI.

bshastry commented 9 months ago

@bshastry I should have mentioned earlier that most of the compiler is in the process of being rewritten, on the fe-v2 branch. It would be more useful to add the fuzzer to that branch. The fe-v2 branch is expected to be ready for use in a few months, and we likely won't fix minor issues on the master branch. (That said, it would be useful to add any bugs you find to our test suite, to ensure that the same problems don't exist on the fe-v2 branch)

However, the fe-v2 branch driver crate (currently called driver2) doesn't have a simple entry point. You could add one to start fuzzing the new parser at least. The other components are maybe not quite ready for fuzzing, but some major changes should be merged into the fe-v2 branch someday soon.

Sure, would targeting this parser API be a good start (and targeting a different PR to fe-v2 instead of the master branch)?

https://github.com/ethereum/fe/blob/4643ced30f21df645944b9d7713c66b62fe1ddb7/crates/parser2/src/lib.rs#L12-L22

bshastry commented 9 months ago

Minor update: I created a fe-v2 fuzzer and a draft PR here #969

ethereum / fe