[SKIP SOF-TEST] Add new -i3/-i4 flags + fuzz_*features.conf files to add more CONFIG_ when fuzzing #9409
Conversation
|
Fixes for warnings in https://github.com/thesofproject/sof/actions/runs/10569644939/job/29282813058?pr=9409 already submitted in #9405. No other warning. |
marc-hb
requested review from
lgirdwood,
plbossart,
mmaka1,
lbetlej,
dbaluta and
kv2019i
as code owners
marc-hb
force-pushed
the
fuzz-features
branch
2 times, most recently
from
7c8d046
to
7443d6e
Compare
scripts/fuzz.sh
Outdated
| @@ -85,10 +87,13 @@ main() | |||
| local BUILD_ONLY=false | |||
| local FUZZER_STDOUT=/dev/stdout # bashism | |||
| local TEST_DURATION=3 | |||
| local IPC=3 | |||
There was a problem hiding this comment.
once this lands it will possibly break oss-fuzz
Please send a PR right after to fix https://github.com/google/oss-fuzz/blob/master/projects/sound-open-firmware/build.sh
There was a problem hiding this comment.
Thanks, I was afraid of something like this :-(
It will be even worse than "break" because -DCONFIG_IPC_MAJOR_3=y will silently take precedence over CONFIG_IPC_MAJOR_4 inside -DEXTRA_CONF_FILE=stub_build_all_ipc4.conf.
So this PR here is breaking backwards compatibility... I considered leaving some "undefined" IPC version in place but that would have added significant complexity.
There is one simpler thing I can do: drop the IPC=3 default and make either -3 or -4 required. So this will still break backwards compatibility but NOT silently. What do you think?
There was a problem hiding this comment.
That is fine, we go without a build for a day is not an issue. Backwards compatibility is not as much as an issue since we are worried about fuzzing going forward primarily and reproducibility rather than bisecability.
There was a problem hiding this comment.
That is fine, we go without a build for a day is not an issue.
You will go with a build. But the IPC4 fuzzing will actually fuzz IPC3 silently.
The more I think about it, the less I like it.
There was a problem hiding this comment.
So this PR here is breaking backwards compatibility... I considered leaving some "undefined" IPC version in place but that would have added significant complexity.
So it wasn't that complex after all.
Full backwards compatibility now, zero regression.
marc-hb
changed the title
| CONFIG_COMP_SRC_IPC4_FULL_MATRIX=y | ||
| CONFIG_COMP_CROSSOVER=y | ||
| CONFIG_COMP_DRC=y | ||
| CONFIG_COMP_KPB=y |
There was a problem hiding this comment.
everything should be one here, even if its only a stub, can you turn everything on?
There was a problem hiding this comment.
I ran into some Kconfig warnings when trying to enable more stuff and I'm running a bit out of SOF time... the main purpose of this PR is really to set the scripting "framework" in place so people who know better can come and just tweak the CONFIG_... / features. Does that make sense?
There was a problem hiding this comment.
A long time ago, I tried to find some -Werr option in kconfiglib.py but there is none, at least none for this. Because most people don't look at build logs, I'm afraid it means that CONFIG_ warnings and messes will never stop creeping in, see some examples in:
This comment is NOT fuzzer-specific unfortunately.
scripts/fuzz.sh
Outdated
| case "$opt" in | ||
| 3) IPC=3;; | ||
| 4) IPC=4;; |
There was a problem hiding this comment.
can they be made mutually-exclusive? Or make it --ipc=4?
There was a problem hiding this comment.
Both are possible but I think this script is growing too big. libfuzzer works well but it has been deprecated by LLVM. The more complex is this script, the more effort to maintain it.
There was a problem hiding this comment.
I changed it to -i3 / -i 4, small enough change and keeps the code simple and short. It's not actually mutually exclusive (last one still wins) but it's more obvious that you're doing something wrong.
| CONFIG_DAI=y | ||
|
|
||
| CONFIG_PM_DEVICE=y | ||
| CONFIG_POWER_DOMAIN=y |
There was a problem hiding this comment.
do we actually want / need to fuzz-test Zephyr Kconfig options or only SOF?
There was a problem hiding this comment.
There's a long comment at the top of this file... too long? :-)
Does line 20 answer the question?
There was a problem hiding this comment.
I think I was confusing things a bit. For some reason I decided that for fuzzing tests we also randomise .config, which isn't the case, is it? Our fuzzing .config is fully deterministic so it can be fixed as close to the production one as only possible?
There was a problem hiding this comment.
Yes that's the idea.
Even if we wanted to do it somewhere, kconfiglib.py has unfortunately no "randconfig" ability. At least not the last time I checked.
Only a shortcut for now but allow more IPC version-dependent logic later. Signed-off-by: Marc Herbert <[email protected]>
The goal of these new files is to: 1. Fuzz more code 2. Reduce the configuration gap between fuzzed SOF and the real thing. See the fuzz_features.conf header for more details. Signed-off-by: Marc Herbert <[email protected]>
marc-hb
changed the title
Stop hardcoding -DCONFIG_IPC_MAJOR_x=y and use the new -3 and -4 fuzz.sh CLI flags. Signed-off-by: Marc Herbert <[email protected]>
3 commits, see messages.
The goal of these new files is to:
cc: