There doesn't seem to be a way to run all fuzz targets in PRs

[evverx]

Both CFLite and CIFuzz exit as soon as they discover a bug but it would be great if it was possible to let them run all fuzz targets and report all the bugs. It would be consistent with how test suites work in general but other than that it should help in scenarios where bugs unrelated to PRs are more likely to pop up because the latest builds aren't available: systemd/systemd@d38363b

[evverx]

FWIW that commit mentions #84 but even if it was possible to trim artifacts and upload the latest builds I think it would make sense to run all fuzz targets there because the latest public OSS-Fuzz corpora (systemd/systemd@69aa498) can trigger bugs that have nothing to do with PRs where patches are backported to stable branches

[jonathanmetzman]

I think I can add this feature, but can you clarify why you want to find bugs in PRs when the bugs were not caused by PRs?

[evverx]

CFLIte tests stable branches that are always behind upstream and I can imagine a scenario where bugs found after releases are cut are fixed upstream but still present in releases. Since the latest OSS-Fuzz corpora are in sync with the upstream repository they can trigger bugs like that in stable branches. They should be fixed there by backporting relevant patches but until then it should be possible for CFLite to also catch other bugs (possibly introduced in PRs) to let people backporting stuff focus on one patchset at a time. I hope it makes sense.

[jonathanmetzman]

Actually instead of adding this feature, maybe we can leverage some that we already have.
Can you try to change this mode to batch: https://github.com/systemd/systemd/blob/main/.github/workflows/cflite_pr.yml#L38

[evverx]

Thanks! I'll try to experiment with it tomorrow.

I was actually leaning towards borrowing the "pruning" part where fuzzers don't stop as soon as they discover a bug: https://github.com/evverx/libbpf/runs/4895317343 but I don't think that I need this level of granularity for systemd forks.

[evverx]

Looking at https://github.com/evverx/libbpf/actions/runs/1728560391 it seems artifacts are always uploaded in that mode and I can't do that on every PR

[evverx]

My bad. It seems they were uploaded because the fuzzer failed there. If artifacts aren't uploaded unconditionally it should be fine.

[jonathanmetzman]

O you're right, it will also upload the corpus.
OK I need to implement the feature you want.

[evverx]

@jonathanmetzman I tested it anyway to figure out whether it works with more than one fuzz target. In evverx/systemd#8 I broke two fuzz targets and judging by https://github.com/evverx/systemd/runs/5116939636?check_suite_focus=true CFLite was able to catch it. Looking at the log it's hard to tell what failed there exactly. It would probably make sense to accumulate and report issues all at once at the end. Apart from that, those fuzz targets shouldn't have passed the "bad build" check but it wasn't mentioned anywhere (allowed-broken-targets-percentage seems to work though!). I'll try to open new issues.

[jonathanmetzman]

Actually, I think this feature exists, I just need to expose it to users.
After my PRs land, set keep-unaffected-fuzz-targets: true in the build_fuzzers action to do this.

[evverx]

Thanks! When it lands could you point me in the direction of the SHA hash of the base builder image with that feature included?

[jonathanmetzman]

Thanks! When it lands could you point me in the direction of the SHA hash of the base builder image with that feature included?

I don't think this is affected by base-builder.
I think once I land the PRs you'll need to change the SHA your workflows point to.

[jonathanmetzman]

I've delayed this because I'm concerned about how all of this fine grained configuration interacts with the implicit configuration of the coarse grained stuff. For example, if I make an option to keep fuzzing regardless of crashes, if that's set to true, it's clear that in batch fuzzing and code-change fuzzing the correct behavior is to keep fuzzing. But if it's set to False (as it will be by default) it's only clear what to do in the code-change case, but not so in batch fuzzing (the point of batch fuzzing is to run everything).

Creating a flag that controls whether corpus is uploaded has the same problem, if False, it's clear what to do in batch fuzzing and code-change fuzzing. If it's True, then what should we do in code-change fuzzing? I guess we could upload the corpus but I really don't think it makes sense.

All-in-all too much of these knobs adds a lot of if-cases to the code and makes things harder to maintain.

[evverx]

But if it's set to False (as it will be by default) it's only clear what to do in the code-change case, but not so in batch fuzzing (the point of batch fuzzing is to run everything).

I think this setting should be applicable to "code-change" only. It should probably be ignored everywhere else.

All-in-all too much of these knobs adds a lot of if-cases to the code and makes things harder to maintain.

Agreed.

[jonathanmetzman]

I decided to land it.

[evverx]

@jonathanmetzman could you reopen this issue? Even with keep-unaffected-fuzz-targets CFlite reports the fist bug and bails out while this issue is supposed to cover another use case where it should be possible to collect all the crashes instead of reporting them one by one.

It's useful to get around issues caused by broken coverage though so it's great that CFLite supports it too.

[inferno-chromium]

Reopened. @jonathanmetzman @oliverchang as fyi.