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

Improve fuzzing tests #109

Closed
kaleidawave opened this issue Jan 29, 2024 · 11 comments · Fixed by #111
Closed

Improve fuzzing tests #109

kaleidawave opened this issue Jan 29, 2024 · 11 comments · Fixed by #111
Labels
internal-improvements Related to internal publishing, building, workflows, formatting & linting parser Related to Ezno's syntax parser, AST definitions and output

Comments

@kaleidawave
Copy link
Owner

kaleidawave commented Jan 29, 2024

The fuzzing of the parser is really good 🎉 some of the errors found in the parser so far:

  • Array elements != function arguments. Array elements are allowed holes (e.g. [2,,3] is valid), but function arguments are not.
  • When rendering x++ + 5 it is really important to have the whitespace between the ++ + as x+++5 is invalid and has ambiguous parsing.
  • If there is (function () { return 2 }) in statement position, the parenthesis need to be kept as otherwise it is parsed as a statement and it is invalid as it needs a name (similarly for object literals and class expression).
  • Lots of others...

However the fuzzing tests are still not passing. They are only really picking up extreme edge cases now , so it isn't urgent. I am hoping the fixes are near the end. But I was wondering if there could be some improvements:

  • When a parse -> render -> parse (second parse) fail happens it shows input as what is mismatched. It is possible to print the first output as where the mismatch was. See action run here, I got confused at first as I thought the test was expected to match the input exactly (which is very difficult as it is just an AST parser)
  • If there is a fail on either of the tests is it possible to keep going and maybe pick up say three or more errors. At the moment it short circuits on the first fail. Wondering if it is possible to gather more errors to fix as at the moment attempting to parse them is a very slow trial & commit. I couldn't find the input arguments in the docs: Guide - Rust Fuzz Book (rust-fuzz.github.io), don't know if it is customisable. Even if it is just temporary for getting the first green light?
  • I was wondering whether the fetching of boa's fuzzing cases in build.rs should get from a specific fixed ref, rather than the main branch?

Pinging @addisoncrump & @jasikpark the fuzzing experts

@kaleidawave kaleidawave added parser Related to Ezno's syntax parser, AST definitions and output internal-improvements Related to internal publishing, building, workflows, formatting & linting labels Jan 29, 2024
@addisoncrump
Copy link
Contributor

experts is strong 😉

  1. parse -> render -> parse: Is the concern about the clarity of output? This can be adjusted quite easily, but what that is saying is that the input is parse-able, but the rendered output is not.
  2. Picking up more errors is possible, but potentially not desirable. You may have multiple outputs which represent different bugs (or potentially a linear combination of bugs!). The relevant CLI is: cargo fuzz run ... -- -fork=1 -ignore_crashes=1
  3. They should probably be fixed, yes 😂 Random breakages are never fun for CI.

@kaleidawave
Copy link
Owner Author

Awesome, I will look into that and try those out in a new PR. Do the fuzzing tests use libFuzzer and is that where the options are documented? https://llvm.org/docs/LibFuzzer.html#options

@kaleidawave kaleidawave linked a pull request Jan 30, 2024 that will close this issue
11 tasks
@kaleidawave
Copy link
Owner Author

kaleidawave commented Jan 30, 2024

Ah exactly what I wanted! More cases printed 🎉: https://github.com/kaleidawave/ezno/actions/runs/7713102547/job/21022255510 good for fixing the current issues. Once green then can go back to short-circuited version.

@addisoncrump
Copy link
Contributor

If you want to be a bit experimental: try the following replacement in fuzz/Cargo.toml:

diff --git a/parser/fuzz/Cargo.toml b/parser/fuzz/Cargo.toml
index 2abb8b9..7caea01 100644
--- a/parser/fuzz/Cargo.toml
+++ b/parser/fuzz/Cargo.toml
@@ -18,7 +18,7 @@ boa_ast = { git = "https://github.com/boa-dev/boa.git", features = [
 boa_interner = { git = "https://github.com/boa-dev/boa.git", features = [
   "arbitrary",
 ] }
-libfuzzer-sys = "0.4"
+libfuzzer-sys = { git = "https://github.com/AFLplusplus/LibAFL.git", branch = "libfuzzer-best", package = "libafl_libfuzzer" }
 pretty_assertions = "1.3.0"
 proc-macro2 = "1.0.66"

This uses the libafl_libfuzzer which we have since stabilised. It'll spit out some maybe undesirable backtraces, but it finds things way faster than libfuzzer.

@kaleidawave
Copy link
Owner Author

Is it a bug that ==2546933== ERROR: libFuzzer: deadly signal are printed but they do not print at the end?

image

Also what does each part of this tracing line mean?

#2202576: cov: 7616 ft: 29739 corp: 7037 exec/s 4639 oom/timeout/crash: 0/0/2 time: 496s job: 31 dft_time: 0

Does one of these numbers display how many fuzzing runs it has completed?

@addisoncrump
Copy link
Contributor

addisoncrump commented Feb 22, 2024

Huh, curiously, it seems that forking mode output doesn't have any documentation. Let me explain this piecewise.

The ==2546933== ERROR: libFuzzer: deadly signal indicate that the fuzzer found an input which triggered a "deadly" signal (e.g. a SIGABRT or SIGSEGV). This just means that the fuzzer is doing it's job -- not a bug.

Let's break apart the line you asked about:

  • #2202576: : "on execution number 2202576, here are the current statistics:"
  • cov: 7616: "7616 coverage points (i.e. unique program edges, pruned if redundant) have been discovered"
  • ft: 29739: "29739 input/program features have been discovered" (this isn't really well-defined, but can be considered as "anything that the fuzzer finds interesting enough to keep" like inputs that discover new sequences of bytes that commonly appear, or edge combinations, etc.)
  • corp: 7037: "we have 7037 corpus entries" (i.e. 7037 stashed inputs)
  • exec/s 4639: "we are executing 4639 testcases per second"
  • oom/timeout/crash: 0/0/2: "we have 0 out-of-memory errors, 0 timeouts, and 2 crashes (i.e. deadly signals)"
  • time: 496s: "we have so far run for 496 seconds"
  • job: 31: "we have started 31 different instances of the fuzzer" (e.g. to recover from crashes or to clear memory if some persists)
  • dft_time: 0: "we have spent zero seconds doing dataflow tracing" (you aren't using this feature and shouldn't)

@kaleidawave
Copy link
Owner Author

Nice! so job: 31 means it is currently running 31 in parallel?


The ==2546933== ERROR: libFuzzer: deadly signal indicate that the fuzzer found an input which triggered a "deadly" signal (e.g. a SIGABRT or SIGSEGV). This just means that the fuzzer is doing it's job -- not a bug.

That is what I thought. Do you know why they are not logged at the end of the test then? (it is under the -fork=1 -ignore_crashes=1 mode for 10 minutes btw)
https://github.com/kaleidawave/ezno/actions/runs/7989637543/job/21816634859

@addisoncrump
Copy link
Contributor

Nice! so job: 31 means it is currently running 31 in parallel?

No, it means that it's launched 31 individual instances. You only ever have one running at a time, but libfuzzer may stop "jobs" and start new ones if e.g. there is persistent memory detected, if the job crashes, or for other mysterious internal reasons.

@addisoncrump
Copy link
Contributor

It seems that cargo-fuzz only emits crash data if the command is not deemed successful; with max_total_time, the exit code is zero, so it's marked successful. Let me see if I have a workaround for this.

@addisoncrump
Copy link
Contributor

Sadly no easy way around it, and it seems that the exit code is determined by the exit code of the last exited job -- which may or may not be 0.

However, you can add the following POSIX-compatible snippet to the end of your test to just dump the output:

if test -d fuzz/artifacts; then find fuzz/artifacts -type f -print -exec xxd {} \; -exec cargo fuzz fmt -s none module_roundtrip_structured {} \;; false; fi

@kaleidawave
Copy link
Owner Author

kaleidawave commented Feb 22, 2024

Ah I think I understand 👍 will try that in the future. That probably explains my confusion when I wondered why running for 10 minutes (with -fork=1 -ignore_crashes=1) was green, but it is was red when I put it back to short-circuiting and running for 2 minutes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal-improvements Related to internal publishing, building, workflows, formatting & linting parser Related to Ezno's syntax parser, AST definitions and output
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants