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

Optionaly show stats during fuzzing session #144

Merged
merged 5 commits into from
Jun 7, 2024
Merged

Conversation

lukacan
Copy link
Collaborator

@lukacan lukacan commented Mar 13, 2024

Currently, it is not possible to determine (or somehow debug) whether the fuzzing session is actually fuzzing something and if the instructions are successfully executed. This means that based solely on the fuzzer's output and zero crashes, the user cannot tell if their program has no errors or if none of the targeted instructions were successfully executed.

This PR introduces a simple stats logging mechanism.

The best option would be to show stats for the fuzzing session right after the session ends. However, the honggfuzz workflow is as follows:

  1. In the main function, call fuzz_threadsStart.
  2. In the fuzz_threadsStart function, call fuzz_threadNew.
  3. Next, inside fuzz_threadNew, call fuzz_fuzzLoop, which, I guess, starts a new subprocess for the fuzz target, and we can see an infinite loop also in the fuzz_threadNew function.
  4. Lastly, we can see that the subprocess is forcefully closed if conditions are met, let's say the max number of iterations was hit.

Due to the SIGKILL, I think it is not possible to time the end of a fuzzing subprocess (fuzzing session) = we do not exactly know when to output the stats.

So, this PR aims to provide stats during the whole fuzzing session. A new function called run_fuzzer_with_stats inside the commander is created.

The stats collect an accumulating number of invocations and successful invocations of corresponding instructions. Then, at the end of a fuzzing iteration, stats are printed. If every instruction was successfully executed, the whole structure with a success message is printed; on the other hand, if any instruction has 0 successful invocations, an error message is printed.

This means as the underlying fuzzer tries to explore as many branches as possible, this can lead to two scenarios:

  1. A lot of success outputs at the end as it converges to exploring all possible branches.
  2. A lot of error outputs at the end as it cannot explore the code any further.

Lastly, we have two options for indicating to the fuzzer that we want to see stats.

  • Use a cfg flag (similar to how fuzzing is utilized by honggfuzz).
  • Set our environment variable and evaluate it during execution.

I find the second option better because it has no impact on performance and for the first option, we can use:

Command::new("cargo")
            .env("HFUZZ_RUN_ARGS", fuzz_args)
            .env("CARGO_TARGET_DIR", cargo_target_dir)
            .env("HFUZZ_WORKSPACE", hfuzz_workspace)
            // tell fuzzer to output stats
            .env("RUSTFLAGS", "--cfg fuzzing_with_stats")
            .arg("hfuzz")
            .arg("run")
            .arg(target)
            .spawn()?;

However, this approach results in a full re-compilation even if trident fuzz run <FUZZ_TARGET> was already called.

NOTE: regarding the text_generator.rs changes. The cargo clippy had problem with (used) unused variable SUCCESS within the lib.rs. So I just changed FINISH -> SUCCESS.

@lukacan lukacan force-pushed the feat/fuzzer-stats-logging branch 2 times, most recently from 1beb7b8 to 62969c9 Compare March 14, 2024 10:48
@lukacan lukacan marked this pull request as ready for review March 14, 2024 10:59
@lukacan lukacan requested a review from Ikrk March 14, 2024 11:00
@lukacan lukacan changed the title 🚧 WIP: log stats Optionaly show stats during fuzzing session Mar 14, 2024
Copy link
Collaborator

@Ikrk Ikrk left a comment

Choose a reason for hiding this comment

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

As discussed off-line: one possible solution might be to intercept the err output of the fuzzer and parse the logs with tables of successful and failed instructions. That way we might have the continuously updated cumulative values of successful and failed instructions and only output the final stats at the end of fuzzing.

crates/client/derive/fuzz_test_executor/src/lib.rs Outdated Show resolved Hide resolved
crates/client/src/fuzzer/program_test_client_blocking.rs Outdated Show resolved Hide resolved
crates/cli/src/command/fuzz.rs Outdated Show resolved Hide resolved
crates/client/src/fuzzer/program_test_client_blocking.rs Outdated Show resolved Hide resolved
@lukacan lukacan force-pushed the feat/fuzzer-stats-logging branch 8 times, most recently from 518f50c to 278a2ae Compare March 17, 2024 22:32
@lukacan
Copy link
Collaborator Author

lukacan commented Mar 17, 2024

Yes, so I updated the feature implementation. Essentially:

  • The stdout of the cargo hfuzz command is marked as piped and is parsed within Trident after the fuzzing session.
  • During the fuzzing session, invoked/successfully invoked instructions are harvested into a HashMap and then serialized into JSON format.
    • The accumulation process is no longer present, as we need to output the stats in every iteration so that the stats can be summed up in the Trident after.
  • This JSON-formatted output is then collected inside Trident at the end of the session and parsed back into a HashMap (line by line).
  • Lastly, the HashMap is displayed (as table), containing the statistics.

Points to note:

  • The cargo hfuzz stdout is piped, not stderr. This is because redirecting stderr would also redirect the compilation output, which is undesirable.

@lukacan lukacan requested a review from Ikrk March 17, 2024 22:46
@lukacan lukacan force-pushed the feat/fuzzer-stats-logging branch 2 times, most recently from 041b0be to d0b375c Compare March 22, 2024 14:34
@lukacan
Copy link
Collaborator Author

lukacan commented Mar 22, 2024

@Ikrk Ready for review

@lukacan lukacan force-pushed the feat/fuzzer-stats-logging branch 5 times, most recently from fce9204 to 7eb533f Compare March 22, 2024 23:50
Copy link
Collaborator

@Ikrk Ikrk left a comment

Choose a reason for hiding this comment

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

It looks much better and seems to be working besides some synchronizaiton issues. Please have a look at my comments.

crates/client/derive/fuzz_test_executor/src/lib.rs Outdated Show resolved Hide resolved
crates/client/src/commander.rs Outdated Show resolved Hide resolved
crates/client/src/fuzzer/fuzzing_stats.rs Outdated Show resolved Hide resolved
@lukacan lukacan force-pushed the feat/fuzzer-stats-logging branch 4 times, most recently from e4e56e7 to dead3a1 Compare May 3, 2024 06:20
@lukacan lukacan requested a review from Ikrk May 13, 2024 08:44
@Ikrk Ikrk force-pushed the feat/fuzzer-stats-logging branch from dead3a1 to add07bc Compare May 31, 2024 12:54
@Ikrk
Copy link
Collaborator

Ikrk commented Jun 7, 2024

I added also stats of failed invariants checks and I created a new Jira task to potentially extend the stats also with unhandled panics: https://ackeeblockchain.atlassian.net/browse/TRD-81

@Ikrk Ikrk merged commit 8e0d235 into develop Jun 7, 2024
8 checks passed
@Ikrk Ikrk deleted the feat/fuzzer-stats-logging branch June 7, 2024 14:06
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.

None yet

2 participants