[SKIP SOF-TEST] Remove old fuzzer

[andyross]

Re-submission for review of the fuzzer removal PR.

Note that this won't apply to main until the first version is reverted. done

[cujomalainey]

is this line still not true?

[andyross]

It is, but it's "3m" now and done with the newer rig

[cujomalainey]

Why 3m? We just want to smoke test in this CI and let OSS-fuzz handle the longer runs

[marc-hb]

Possibly naive question: I understand that the IPCn code is OS-independent for the most part but is it 100% independent? Someone could want to fuzz non-Zephyr on the main branch?

To allow more time for any non-Zephyr discussion yet fix CI immediately we could fast track the first commit in a separate PR?

[marc-hb]

What is prompting this removal: the old non-zephyr based IPC fuzzer started failing as shown below.

Probably since google/oss-fuzz#10342 was merged?

https://github.com/thesofproject/sof/actions/runs/5053499443/jobs/9067397260?pr=7670

Step 16/17 : RUN touch empty.c; gcc -m32 -c empty.c; ar rs /usr/lib32/libstdc++.a empty.o
 ---> Running in 8946b67e7c0e
ar: creating /usr/lib32/libstdc++.a
Removing intermediate container 8946b67e7c0e
 ---> 6676a0f04077
Step 17/17 : COPY build.sh $SRC/
 ---> 45b7670f108f
Successfully built 45b7670f108f
Successfully tagged gcr.io/oss-fuzz/sound-open-firmware:latest
2023-05-23 05:25:11,601 - root - ERROR - Failed to detect repo:
ERROR:root:No git repos with specific commit: None found in /src

2023-05-23 05:25:11,602 - root - ERROR - Could not detect repo.
2023-05-23 05:25:11,602 - root - ERROR - Error building fuzzers for (commit: 0e77530ea75d532f46df7a7015afbbc550c2b8de, pr_ref: refs/pull/7670/merge).

[marc-hb]

tools: Remove older fuzz framework

This is an older pre-Zephyr fuzzing rig that is being replaced by the
newer framework based on Zephyr native_posix builds and a "posix"
platform layer in SOF. Pre-zephyr firmware builds are now happening
out of release branches and can continue to use this integrations
there, if needed.

Actually, the old fuzzer was just broken on stable-v2.2 exactly the same:

• [SKIP SOF-TEST] .github: ipc_fuzzer: just a stable-v2.2 test #7676

I guess because google/oss-fuzz#10342 does not care - as expected - about SOF branches?

I see two options to fix stable-v2.2:

• Point SOF stable-v2.2 at a more static and older old oss-fuzz image. Basically: "branch" oss-fuzz together with SOF stable-v2.2

• Fix the older, non-Zephyr fuzzer in stable-v2.2. But in that case it's just easier to fix it in the main SOF branch and cherry-pick the patches to stable-v2.2! In other words, don't branch the fuzzing "for real" until it's actually needed. In yet other words don't really branch the "test code" until the production code actually requires it, otherwise it's unnecessary divergence and extra maintenance for no reason. The less divergence the better.

Here's a recent, stable-v2.2 example of NOT diverging unnecessarily: #7551

[cujomalainey]

Possibly naive question: I understand that the IPCn code is OS-independent for the most part but is it 100% independent? Someone could want to fuzz non-Zephyr on the main branch?

I mean, not really given the spec like DAI is definitely tied to a HW backend, but there are no fuzzing rigs for Xtensa (heck there are only portion on arm compared to x86) so I would not be as concerned here.

To allow more time for any non-Zephyr discussion yet fix CI immediately we could fast track the first commit in a separate PR?

Yes I think that is a fair request.

I guess because google/oss-fuzz#10342 does not care - as expected - about SOF branches?

That is correct, it grabs the latest and greatest and just hits it with a hammer

I see two options to fix stable-v2.2:

I think we can just delete it. There is no benefit to fuzzing stable-v2.2 without porting the new rig down as it has already been fuzzed to high heaven with the old rig and it hasn't found a bug in months. The branch should have basically nothing changing W.R.T IPC so i am not as concerned as it should be slow fixes going forward. That being said I would be open to at least a estimate of the lift effort to get the new fuzzer going.

[marc-hb]

Just noticed by chance that we still have this: https://thesofproject.github.io/latest/developer_guides/fuzzing/fuzzing_in_docker.html
https://github.com/thesofproject/sof-docs/tree/master/developer_guides/fuzzing

[andyross]

I've removed the big removal patch for now pending more discussion. This is just the workflow change that unbreaks CI.

FWIW: I'm not sure I'm following the discussion on the 2.2 branch issue. As I understand it the way the older scheme worked, and has always worked, is to pull current oss-fuzz images/code and build the current oss-fuzz integration (to be clear: I don't really understand why it's failing, those github actions are greek to me). It doesn't build against the checked-out SOF code nor try to match versions to whatever we're using. oss-fuzz is a separate project and it doesn't branch according to SOF rules. This was never going to work the way it was set up.

So, the way to fix that it to make the fuzzer build locally in our CI images. Which is exactly what's been merged. Can't we just use that and call it a fix?

[marc-hb]

So, the way to fix that it to make the fuzzer build locally in our CI images. Which is exactly what's been merged. Can't we just use that and call it a fix?

That's not going to work out of the box for stable-v2.2 which is quite far away with barely any Zephyr support.

That being said I would be open to at least a estimate of the lift effort to get the new fuzzer going.

@andyross any idea?

There is no benefit to fuzzing stable-v2.2 without porting the new rig down as it has already been fuzzed to high heaven with the old rig and it hasn't found a bug in months. The branch should have basically nothing changing W.R.T IPC so i am not as concerned as it should be slow fixes going forward.

This looks like a very good stable-v2.2 commit message :-)

As I understand it the way the older scheme worked, and has always worked, is to pull current oss-fuzz images/code and build the current oss-fuzz integration (to be clear: I don't really understand why it's failing, those github actions are greek to me). It doesn't build against the checked-out SOF code nor try to match versions to whatever we're using.

Instead of stopping to fuzz stable-v2.2 alltogether, another, very cheap stable-v2.2 option could be:

--- a/.github/workflows/ipc_fuzzer.yml
+++ b/.github/workflows/ipc_fuzzer.yml
@@ -31,12 +31,12 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Build Fuzzers
-        uses: google/oss-fuzz/infra/cifuzz/actions/build_fuzzers@master
+        uses: google/oss-fuzz/infra/cifuzz/actions/build_fuzzers@version_just_before_andys_10342
         with:
           oss-fuzz-project-name: 'sound-open-firmware'
 
       - name: Run Fuzzers
-        uses: google/oss-fuzz/infra/cifuzz/actions/run_fuzzers@master
+        uses: google/oss-fuzz/infra/cifuzz/actions/run_fuzzers@same
         with:
           oss-fuzz-project-name: 'sound-open-firmware'
           language: c

While not always necessary, it's pretty common to branch test code (oss-fuzz) with production code (sof stable-v2.2)

[andyross]

The new fuzzing code is obligate zephyr and relies on a fairly recent version. We'd need to update that, and then build SOF in a configuration that doesn't match the shipping firmware on that branch. Honestly if we want to preserve the older fuzzing code I think a better tactic would be to update it to run in the SOF CI container, just like the new stuff. That would lose the smoke test facility for making sure oss-fuzz works, but then none of that code is changing anymore. I doubt that would be difficult, honestly. Mostly just a "fuzz-old.sh" script.

I guess my gut says that fuzzing is a technique for validation of new code. It has minimal (but sure, not zero) value on maintenance branches that are expected to be protocol-compatible in perpetuity.

[cujomalainey]

@kv2019i when this lands we should start watching the CI again

[cujomalainey]

I guess my gut says that fuzzing is a technique for validation of new code. It has minimal (but sure, not zero) value on maintenance branches that are expected to be protocol-compatible in perpetuity.

Exactly my thoughts, and its isolated to finding holes in communication paths. I think a better option would be to mandate fixes for IPC3 go back to that branch if they apply rather than trying to fuzz it directly.

[marc-hb]

There may be a point in the future where IPC3 is gone from the main branch though.

[cujomalainey]

Yes which means it really shouldn't be being modified on the branch

[marc-hb]

stable-v2.2 discussion moved to new

• [CLEANUP] Legacy Fuzzer #7709