Skip to content

cifuzzer: provide fuzzing_engine parameter #13351

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
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

cifuzzer: provide fuzzing_engine parameter #13351

wants to merge 3 commits into from

Conversation

LebedevRI
Copy link
Contributor

Is this something oss-fuzz wans to support?

It is mildly bad that it is currently impossible
to actually CI all of the builds the oss-fuzz proper will perform, since only the libfuzzer engine can be "configured".

This makes the engine configurable.
At least the building works, i'm not sure about the rest.

There are most likely things missing here.

I'm also not sure how coverage/introspector builds should be handledm what should their engine be?

Refs. #13346

@jonathanmetzman
Copy link
Contributor

I think the goal of CIFuzz isn't to be CI for OSS-Fuzz but to find bugs in the projects before merging code. So I think this is out of scope and against the goal of simplicity I had for CIFuzz. I can merge this if it's important enough to you but it won't be supported.

@LebedevRI
Copy link
Contributor Author

I think the goal of CIFuzz isn't to be CI for OSS-Fuzz but to find bugs in the projects before merging code. So I think this is out of scope and against the goal of simplicity I had for CIFuzz. I can merge this if it's important enough to you but it won't be supported.

My point is that i've spent a number of hours trying to placate the existing CIFuzz builds to pass
with the changes to build.sh i've wanted, only to later discover that oss-fuzz's project build
went red anyways. And this is not the first time this has happened. Those are bugs in the project.

@LebedevRI LebedevRI force-pushed the cifuzz branch 3 times, most recently from ef93cca to 581a884 Compare May 22, 2025 06:50
@LebedevRI
cifuzzer: provide fuzzing_engine parameter
b1ab360 
It is mildly bad that it is currently impossible
to actually CI all of the builds the oss-fuzz proper will perform,
since only the libfuzzer engine can be "configured".

This makes the engine configurable.
At least the building works, i'm not sure about the rest.

There are most likely things missing here.

I'm also not sure how coverage/introspector builds should be handledm
what should their engine be?

Refs. google#13346
@LebedevRI
Copy link
Contributor Author

LebedevRI commented May 22, 2025 •
edited
Loading

Ok, this is close to done i think.
The builds work: https://github.com/LebedevRI/rawspeed/actions/runs/15180640659/job/42689198535
For coverage, apparently libfuzzer engine is needed, not none. Should this be enforced?
But introspector does not work with either. I don't know how to do that build.
Running non-libfuzzer-based jobs results in failures, however:
AFL just hangs in there, and honggfuzz somehow does not accept corpus dir:

2025-05-22 07:41:29,126 - root - INFO - Running fuzzer: PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree.
2025-05-22 07:41:29,126 - root - INFO - Downloading corpus from OSS-Fuzz: https://storage.googleapis.com/librawspeed-backup.clusterfuzz-external.appspot.com/corpus/libFuzzer/librawspeed_PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree/public.zip
2025-05-22 07:41:29,128 - urllib3.connectionpool - DEBUG - Starting new HTTPS connection (1): storage.googleapis.com:443
2025-05-22 07:41:29,365 - urllib3.connectionpool - DEBUG - [https://storage.googleapis.com:443](https://storage.googleapis.com/) "GET /librawspeed-backup.clusterfuzz-external.appspot.com/corpus/libFuzzer/librawspeed_PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree/public.zip HTTP/1.1" 200 2752954
2025-05-22 07:41:30,140 - root - INFO - Starting fuzzing
2025-05-22 07:41:30,165 - root - INFO - Fuzzer: PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree. Detected bug.
Fuzzing logs:
Accepting input from '/github/workspace/cifuzz-corpus/PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree'
Usage for fuzzing: honggfuzz -P [flags] -- /github/workspace/build-out/PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree
[2025-05-22T07:41:30+0000][E][396] HonggfuzzRunFromFile():103 Couldn't read data from stdin: Is a directory
2025-05-22 07:41:30,165 - root - INFO - Trying to reproduce crash using: /tmp/tmpy3v98b63/tmpgtyfi59r.
2025-05-22 07:41:30,395 - root - INFO - Reproduce command returned: 0. Not reproducible on /github/workspace/build-out/PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree.
2025-05-22 07:41:30,395 - root - INFO - Crash is not reproducible.
2025-05-22 07:41:30,395 - root - INFO - Deleting corpus and seed corpus of PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree to save disk.
2025-05-22 07:41:30,487 - root - INFO - Deleting fuzz target: PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree.
2025-05-22 07:41:30,488 - root - INFO - Done deleting.
2025-05-22 07:41:30,488 - root - INFO - Fuzzer PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree finished running without reportable crashes.

I am mostly interested in just checking that it builds fine,
so maybe that is okay...

Looks like there is some preexisting brokenness with detecting the changed fuzz targets
when running on a branch (not a PR), and it gets force-pushed:

2025-05-22 07:31:57,119 - root - INFO - Removing unaffected fuzz targets.
2025-05-22 07:31:57,122 - root - DEBUG - Diffing against base_commit: 35ea44e25375a851974aa88a394c71d1403dabd8.
2025-05-22 07:31:57,122 - root - INFO - Diffing against 35ea44e25375a851974aa88a394c71d1403dabd8.
2025-05-22 07:31:57,124 - root - DEBUG - Stderr of command "git diff --name-only 35ea44e25375a851974aa88a394c71d1403dabd8... --" is: fatal: Invalid symmetric difference expression 35ea44e25375a851974aa88a394c71d1403dabd8...

Why did it pick that commit? It is the original commit It should diff against the

@LebedevRI LebedevRI changed the title [WIP] cifuzzer: provide fuzzing_engine parameter cifuzzer: provide fuzzing_engine parameter May 22, 2025
@LebedevRI
Copy link
Contributor Author

For coverage, apparently libfuzzer engine is needed, not none. Should this be enforced?

I'm actually confused by the purpose of none engine.
It still passes -DLIB_FUZZING_ENGINE:STRING=/usr/lib/libFuzzingEngine.a,
but that file does not exist.

@jonathanmetzman
Copy link
Contributor

For coverage, apparently libfuzzer engine is needed, not none. Should this be enforced?

I'm actually confused by the purpose of none engine. It still passes -DLIB_FUZZING_ENGINE:STRING=/usr/lib/libFuzzingEngine.a, but that file does not exist.

Where is that?

@jonathanmetzman
Copy link
Contributor

Ok, this is close to done i think. The builds work: https://github.com/LebedevRI/rawspeed/actions/runs/15180640659/job/42689198535 For coverage, apparently libfuzzer engine is needed, not none. Should this be enforced? But introspector does not work with either. I don't know how to do that build. Running non-libfuzzer-based jobs results in failures, however: AFL just hangs in there, and honggfuzz somehow does not accept corpus dir:

2025-05-22 07:41:29,126 - root - INFO - Running fuzzer: PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree.
2025-05-22 07:41:29,126 - root - INFO - Downloading corpus from OSS-Fuzz: https://storage.googleapis.com/librawspeed-backup.clusterfuzz-external.appspot.com/corpus/libFuzzer/librawspeed_PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree/public.zip
2025-05-22 07:41:29,128 - urllib3.connectionpool - DEBUG - Starting new HTTPS connection (1): storage.googleapis.com:443
2025-05-22 07:41:29,365 - urllib3.connectionpool - DEBUG - [https://storage.googleapis.com:443](https://storage.googleapis.com/) "GET /librawspeed-backup.clusterfuzz-external.appspot.com/corpus/libFuzzer/librawspeed_PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree/public.zip HTTP/1.1" 200 2752954
2025-05-22 07:41:30,140 - root - INFO - Starting fuzzing
2025-05-22 07:41:30,165 - root - INFO - Fuzzer: PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree. Detected bug.
Fuzzing logs:
Accepting input from '/github/workspace/cifuzz-corpus/PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree'
Usage for fuzzing: honggfuzz -P [flags] -- /github/workspace/build-out/PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree
[2025-05-22T07:41:30+0000][E][396] HonggfuzzRunFromFile():103 Couldn't read data from stdin: Is a directory
2025-05-22 07:41:30,165 - root - INFO - Trying to reproduce crash using: /tmp/tmpy3v98b63/tmpgtyfi59r.
2025-05-22 07:41:30,395 - root - INFO - Reproduce command returned: 0. Not reproducible on /github/workspace/build-out/PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree.
2025-05-22 07:41:30,395 - root - INFO - Crash is not reproducible.
2025-05-22 07:41:30,395 - root - INFO - Deleting corpus and seed corpus of PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree to save disk.
2025-05-22 07:41:30,487 - root - INFO - Deleting fuzz target: PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree.
2025-05-22 07:41:30,488 - root - INFO - Done deleting.
2025-05-22 07:41:30,488 - root - INFO - Fuzzer PrefixCodeDecoderFuzzer-LUTWithLookupVsLUTWithTree finished running without reportable crashes.

I am mostly interested in just checking that it builds fine, so maybe that is okay...

Looks like there is some preexisting brokenness with detecting the changed fuzz targets when running on a branch (not a PR), and it gets force-pushed:

2025-05-22 07:31:57,119 - root - INFO - Removing unaffected fuzz targets.
2025-05-22 07:31:57,122 - root - DEBUG - Diffing against base_commit: 35ea44e25375a851974aa88a394c71d1403dabd8.
2025-05-22 07:31:57,122 - root - INFO - Diffing against 35ea44e25375a851974aa88a394c71d1403dabd8.
2025-05-22 07:31:57,124 - root - DEBUG - Stderr of command "git diff --name-only 35ea44e25375a851974aa88a394c71d1403dabd8... --" is: fatal: Invalid symmetric difference expression 35ea44e25375a851974aa88a394c71d1403dabd8...

Why did it pick that commit? It is the original commit It should diff against the

I recommend only doing builds for these other fuzzers or configs. I'm pretty sure we do the basic smoke test in that step anyway. The smoketest is meant to support all of the fuzzers not just libfuzzer.

@jonathanmetzman
Copy link
Contributor

Looks like there is some preexisting brokenness with detecting the changed fuzz targets
when running on a branch (not a PR), and it gets force-pushed:

I'm not sure. I don't have the time to look into this though. Feel free to send a PR to disable change detection in this case (I think that might be the fallback behavior anyway).

@jonathanmetzman
Copy link
Contributor

I think the goal of CIFuzz isn't to be CI for OSS-Fuzz but to find bugs in the projects before merging code. So I think this is out of scope and against the goal of simplicity I had for CIFuzz. I can merge this if it's important enough to you but it won't be supported.

My point is that i've spent a number of hours trying to placate the existing CIFuzz builds to pass with the changes to build.sh i've wanted, only to later discover that oss-fuzz's project build went red anyways. And this is not the first time this has happened. Those are bugs in the project.

Sure, I want to help make my users' lives easier. But I frankly think OSS-Fuzz has a little bit too many build configs to support in CIFuzz and even for projects (I have some ideas for paring this down in the future).

@LebedevRI
Copy link
Contributor Author

@jonathanmetzman thank you for taking a look!

I recommend only doing builds for these other fuzzers or configs. I'm pretty sure we do the basic smoke test in that step anyway. The smoketest is meant to support all of the fuzzers not just libfuzzer.

Sure, should that be enforced in the RunFuzzersConfig of config_utils.py?

Looks like there is some preexisting brokenness with detecting the changed fuzz targets
when running on a branch (not a PR), and it gets force-pushed:

I'm not sure. I don't have the time to look into this though. Feel free to send a PR to disable change detection in this case (I think that might be the fallback behavior anyway).

I'm not sure how to detect that situation, and i don't particularly care about it, i'm just posting my observation.

I think the goal of CIFuzz isn't to be CI for OSS-Fuzz but to find bugs in the projects before merging code. So I think this is out of scope and against the goal of simplicity I had for CIFuzz. I can merge this if it's important enough to you but it won't be supported.

My point is that i've spent a number of hours trying to placate the existing CIFuzz builds to pass with the changes to build.sh i've wanted, only to later discover that oss-fuzz's project build went red anyways. And this is not the first time this has happened. Those are bugs in the project.

Sure, I want to help make my users' lives easier. But I frankly think OSS-Fuzz has a little bit too many build configs to support in CIFuzz and even for projects (I have some ideas for paring this down in the future).

I mainly just think that being unable to ensure that oss-fuzz builds don't go red via the project's CI is a rather bad UX.
This PR closes that gap (other than introspection build), so i would really hope this can be merged and supported/not removed?

@LebedevRI
Copy link
Contributor Author

For coverage, apparently libfuzzer engine is needed, not none. Should this be enforced?

I'm actually confused by the purpose of none engine. It still passes -DLIB_FUZZING_ENGINE:STRING=/usr/lib/libFuzzingEngine.a, but that file does not exist.

Where is that?

Here's an example job with

    sanitizer: coverage
    fuzzing_engine: none

https://github.com/LebedevRI/rawspeed/actions/runs/15180497856/job/42688808194#step:4:2302

@jonathanmetzman
Copy link
Contributor

For coverage, apparently libfuzzer engine is needed, not none. Should this be enforced?

I'm actually confused by the purpose of none engine. It still passes -DLIB_FUZZING_ENGINE:STRING=/usr/lib/libFuzzingEngine.a, but that file does not exist.

Where is that?

Here's an example job with

    sanitizer: coverage
    fuzzing_engine: none

https://github.com/LebedevRI/rawspeed/actions/runs/15180497856/job/42688808194#step:4:2302

Could you use libfuzzer? It's more realistic. I don't think we do coverage reports for engine: no

@LebedevRI
Copy link
Contributor Author

For coverage, apparently libfuzzer engine is needed, not none. Should this be enforced?

I'm actually confused by the purpose of none engine. It still passes -DLIB_FUZZING_ENGINE:STRING=/usr/lib/libFuzzingEngine.a, but that file does not exist.

Where is that?

Here's an example job with

    sanitizer: coverage
    fuzzing_engine: none

https://github.com/LebedevRI/rawspeed/actions/runs/15180497856/job/42688808194#step:4:2302

Could you use libfuzzer? It's more realistic. I don't think we do coverage reports for engine: no

That was my solution, yes. Again, i'm just pointing out my observations in case that is something not known,
or in case that should be prohibited via config_utils.py?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@LebedevRI @jonathanmetzman