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.

Sign up for GitHub

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

Jump to bottom

Fix recent FuzzBench cloud experiment failures #2023

Merged
merged 6 commits into from
Aug 15, 2024
Merged

Fix recent FuzzBench cloud experiment failures #2023

merged 6 commits into from
Aug 15, 2024

Conversation

DonggeLiu
Copy link
Contributor

@DonggeLiu DonggeLiu commented Aug 12, 2024 •
edited
Loading

  1. Fix TypeError: expected str, bytes or os.PathLike object, not NoneType in 2024-08-10-test.
Traceback (most recent call last):
  File "/src/experiment/runner.py", line 468, in experiment_main
    runner.conduct_trial()
  File "/src/experiment/runner.py", line 290, in conduct_trial
    self.set_up_corpus_directories()
  File "/src/experiment/runner.py", line 275, in set_up_corpus_directories
    _unpack_clusterfuzz_seed_corpus(target_binary, input_corpus)
  File "/src/experiment/runner.py", line 144, in _unpack_clusterfuzz_seed_corpus
    seed_corpus_archive_path = get_clusterfuzz_seed_corpus_path(
  File "/src/experiment/runner.py", line 98, in get_clusterfuzz_seed_corpus_path
    fuzz_target_without_extension = os.path.splitext(fuzz_target_path)[0]
  File "/usr/local/lib/python3.10/posixpath.py", line 118, in splitext
    p = os.fspath(p)
TypeError: expected str, bytes or os.PathLike object, not NoneType

This happens on many benchmarks+fuzzers.
To be investigated later:

  1. Why fuzz_target_path is None.

  2. Why this did not happen in other recent experiments.

  3. I thought I had seen this a long ago, Déjà vu?

  4. Fixing No such file or directory: '/work/measurement-folders/<benchmark>-<fuzzer>/merged.json:

Traceback (most recent call last):
  File "/work/src/experiment/measurer/coverage_utils.py", line 74, in generate_coverage_report
    coverage_reporter.generate_coverage_summary_json()
  File "/work/src/experiment/measurer/coverage_utils.py", line 141, in generate_coverage_summary_json
    result = generate_json_summary(coverage_binary,
  File "/work/src/experiment/measurer/coverage_utils.py", line 269, in generate_json_summary
    with open(output_file, 'w', encoding='utf-8') as dst_file:
FileNotFoundError: [Errno 2] No such file or directory: '/work/measurement-folders/lcms_cms_transform_fuzzer-centipede/merged.json'
  1. Remove incompatible benchmarks: openh264_decoder_fuzzer, stb_stbi_read_fuzzer

@DonggeLiu DonggeLiu mentioned this pull request Aug 12, 2024
Closed
@DonggeLiu
Copy link
Contributor Author

/gcbrun run_experiment.py -a --experiment-config /opt/fuzzbench/service/experiment-config.yaml --experiment-name 2024-08-12-dg --fuzzers aflplusplus centipede honggfuzz libfuzzer --benchmarks stb_stbi_read_fuzzer openh264_decoder_fuzzer

@DonggeLiu
Copy link
Contributor Author

/gcbrun run_experiment.py -a --experiment-config /opt/fuzzbench/service/experiment-config.yaml --experiment-name 2024-08-12-2023 --fuzzers aflplusplus centipede honggfuzz libfuzzer --benchmarks stb_stbi_read_fuzzer openh264_decoder_fuzzer

@DonggeLiu
Copy link
Contributor Author

Experiment 2024-08-12-2023 data and results will be available later at:
The experiment data.
The experiment report.
The experiment report(experimental).

@DonggeLiu
Copy link
Contributor Author

DonggeLiu commented Aug 13, 2024 •
edited
Loading

This failed likely because both fuzz targets failed to generate coverage repots, e.g.:
image

Not sure if this related: OSS-Fuzz's build status page shows openh264_decoder_fuzzer failed.

@DonggeLiu
Copy link
Contributor Author

/gcbrun run_experiment.py -a --experiment-config /opt/fuzzbench/service/experiment-config.yaml --experiment-name 2024-08-13-2023-libfuzzer-1 --fuzzers libfuzzer

@DonggeLiu
Copy link
Contributor Author

Experiment 2024-08-13-2023-libfuzzer-1 data and results will be available later at:
The experiment data.
The experiment report.
The experiment report(experimental).

@DonggeLiu DonggeLiu changed the title Fix bug when fuzz_target_path is None Fix recent FuzzBench cloud experiment failures Aug 13, 2024
@DonggeLiu
Copy link
Contributor Author

Report is back : ) @addisoncrump
I will wait a bit longer before merging this to ensure the report stays alive.
Once I merge this to master, could you please update your PR and bring back the changes you added?
Thanks!

@addisoncrump
Copy link
Contributor

Sure, I'll rebase.

@addisoncrump
Copy link
Contributor

@DonggeLiu I am able to build both openh264 and stb_stbi fuzzers as in master locally with no issue. Like #2021, I think this is a cache issue.

@tokatoka tokatoka mentioned this pull request Aug 13, 2024
Open
@DonggeLiu
Copy link
Contributor Author

@DonggeLiu I am able to build both openh264 and stb_stbi fuzzers as in master locally with no issue. Like #2021, I think this is a cache issue.

I see, thanks for the info!
Given that you are investigating this, is there any help I can provide?
For example, if you think some more cloud build logs can save you time debugging, please feel free to add them and request experiments.
I can run them for you and send you the related logs.

@DonggeLiu
Copy link
Contributor Author

Report on this PR is still not ready, likely due to some VMs were preemptied.
I will give it one more day just to be 100% safe.

@addisoncrump
Copy link
Contributor

Given that you are investigating this, is there any help I can provide?

Ah, I was investigating the specific issue with the bug benchmark. I don't think I can offer much help with the CI or the fuzzbench infra directly. I can say, however, that the coverage benchmarks you removed do work as expected locally with test-run. I need to check if the coverage measurer works as anticipated; maybe this needs to be updated instead.

@addisoncrump
Copy link
Contributor

addisoncrump commented Aug 14, 2024 •
edited
Loading

Ah, @DonggeLiu, try running make test-run-coverage-all. It complains that it can't find bloaty_fuzz_target on master 👀

@tokatoka
Copy link
Contributor

@DonggeLiu I am able to build both openh264 and stb_stbi fuzzers as in master locally with no issue. Like #2021, I think this is a cache issue.

For me the same, they are working. I don't think they should be removed

@DonggeLiu
Copy link
Contributor Author

I see, thanks @addisoncrump and @tokatoka .
I've brought them back.

The experiment is about to finish, I will merge this tmr morning.

@addisoncrump
Copy link
Contributor

I confirmed the coverage measurers build locally as well. Will test when everything has finished building.

@addisoncrump addisoncrump mentioned this pull request Aug 14, 2024
Merged
@addisoncrump
Copy link
Contributor

Yup, I tested openh264 and stb benchmarks locally and they do perform measurements as anticipated. The issue is with the GCP runs, I would presume a build cache issue.

@DonggeLiu
Copy link
Contributor Author

DonggeLiu commented Aug 15, 2024 •
edited
Loading

Yup, I tested openh264 and stb benchmarks locally and they do perform measurements as anticipated. The issue is with the GCP runs, I would presume a build cache issue.

I see, I reckon this could be due to impatible GCP vm environment and llvm?
I will look into this once I finish other tasks in hand.

Just to double-check @addisoncrump :
When you test them locally, did you remove their old local images beforehand?

@DonggeLiu
Copy link
Contributor Author

Thanks for the information again, @addisoncrump!

@DonggeLiu
Copy link
Contributor Author

TBR by @jonathanmetzman.

The experiment that proving this works:
#2023 (comment)

@addisoncrump
Copy link
Contributor

When you test them locally, did you remove their old local images beforehand?

Yes, I do a docker system prune --all before every experiment.

@DonggeLiu
Copy link
Contributor Author

When you test them locally, did you remove their old local images beforehand?

Yes, I do a docker system prune --all before every experiment.

I see, thanks for confirming.
I will merge this then.

@DonggeLiu DonggeLiu merged commit b2f87ff into master Aug 15, 2024
5 checks passed
@DonggeLiu DonggeLiu deleted the fix-non-path branch August 15, 2024 22:01
@DonggeLiu DonggeLiu mentioned this pull request Aug 16, 2024
Merged
DonggeLiu added a commit that referenced this pull request Aug 16, 2024
@DonggeLiu
Disable incompatible benchmarks for cloud experiments (#2030)
adff65e 
Temporarily disable benchmark `stb_stbi_read_fuzzer` and
`openh264_decoder_fuzzer`from cloud experiments, becaue they are
[proven](#2023 (comment))
to be incompatible in cloud build/run environment.

@addisoncrump kindly confirmed that they [work in local
experiments](#2023 (comment)).
@tokatoka
Copy link
Contributor

I thought I had seen this a long ago, Déjà vu?

The same bug happened 1 year ago
#1886

@DonggeLiu
Copy link
Contributor Author

The same bug happened 1 year ago #1886

Thanks for noticing this, let me see if @jonathanmetzman has more insight once he is back.

DonggeLiu
DonggeLiu commented Aug 28, 2024
View reviewed changes
experiment/runner.py
@@ -95,6 +95,8 @@ def _clean_seed_corpus(seed_corpus_dir):
def get_clusterfuzz_seed_corpus_path(fuzz_target_path):
"""Returns the path of the clusterfuzz seed corpus archive if one exists.
Otherwise returns None."""
if not fuzz_target_path:
return None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add an error log here because this is unexpected.

Copy link
Contributor

@tokatoka tokatoka Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

One question, why is this function even called?

https://github.com/google/fuzzbench/blob/master/experiment/runner.py#L277
I think this is the line that eventually calls this line. But for example, when we observed the error for addison's experiment, the ossfuzz corpus was NOT used right? (unless they specified oss-fuzz-corpus: true)
then why we would unpack the clusterfuzz seed corpus at all?

aren't the seed corpus already prepared in build.sh or Dockerfile in each of the benchmarks?

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know if this env var CUSTOM_SEED_CORPUS_DIR set in normal(?) run or not?

Copy link
Contributor

Choose a reason for hiding this comment

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

To me these two lines seem wrong

        elif not environment.get('CUSTOM_SEED_CORPUS_DIR'):
            _unpack_clusterfuzz_seed_corpus(target_binary, input_corpus)

even if we don't use custom_seed_corpus_dir we don't necessarily need clusterfuzz seed corpus, do we??

although why this target_binary is None is another problem that needs investigation

@addisoncrump
Copy link
Contributor

Just to reiterate, this is a major threat to validity -- especially when cached data is used. The cache completely overwrites the report, so the final report generated is simply showing only the last successful experiment. This effectively invalidates all future Fuzzbench reports until this issue is resolved.

I think the report generation issue indicates that safeguards should be put in place that simply terminate the experiment in such degenerative cases, since the results are effectively guaranteed to be invalid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tokatoka tokatoka tokatoka left review comments

@jonathanmetzman jonathanmetzman Awaiting requested review from jonathanmetzman

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DonggeLiu @addisoncrump @tokatoka