projects/sound-open-firmware: New fuzzing framework #10342
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -20,39 +20,50 @@ | |||||
|
|
||||||
| FROM gcr.io/oss-fuzz-base/base-builder | ||||||
|
|
||||||
| # Base packages | ||||||
| RUN apt-get -y update | ||||||
| RUN apt-get install -y \ | ||||||
| gettext git libc6-dev-i386 libglib2.0-dev libncurses5-dev \ | ||||||
| libtool ninja-build python3-pip | ||||||
| RUN pip3 install west | ||||||
|
|
||||||
| # Zephyr SDK: | ||||||
| # | ||||||
| # Zephyr doesn't provide a clean "get the latest version" URL, but | ||||||
| # note that the use of the /latest/ path component at least ensures | ||||||
| # this will fail on a new release. | ||||||
| ARG SDK_VER=0.16.1 | ||||||
| WORKDIR /root | ||||||
| RUN curl -L -o sdktmp.tar.xz https://github.com/zephyrproject-rtos/sdk-ng/releases/latest/download/zephyr-sdk-${SDK_VER}_linux-x86_64_minimal.tar.xz | ||||||
| RUN tar xf sdktmp.tar.xz; rm sdktmp.tar.xz | ||||||
| RUN zephyr-sdk-*/setup.sh -h | ||||||
|
|
||||||
| WORKDIR $SRC | ||||||
|
|
||||||
| # SOF itself (also pulls in Zephyr + modules via west) | ||||||
| RUN west init -m https://github.com/thesofproject/sof sof | ||||||
|
Contributor
There was a problem hiding this comment. Could you use git instead? This is breaking some of our tooling that finds revisions (it assumes git or svn usage like all other users).
Contributor
Author
There was a problem hiding this comment. West is a git wrapper, analogous to repo. It's possible, but error prone, to manually clone all the relevant repositories. The structure isn't complicated, $SRC/sof is the top level directory which contains $SRC/sof/sof, which is the git repo corresponding to the project itself, and $SRC/sof/zephyr, and a few other bits under $SRC/sof/modules/*. What are the requirements here? Can I symlink you something to the top level that matches what you want to see?
Contributor
Author
There was a problem hiding this comment. OK, I see it. srcmap assumes that the git archive was pulled from a remote with the name "origin". This is true by default with an unadorned git clone. It's not required, and in fact west allows multiple remotes with names like "thesofproject" or "upstream". FWIW: a straightforward fix to srcmap, which replaces the assumption that "there is always a remote named origin" with "we only care about the first remote listed" is something like (untested): --- a/infra/base-images/base-builder/srcmap
+++ b/infra/base-images/base-builder/srcmap
@@ -35,7 +35,7 @@ fi
for DOT_GIT_DIR in $(find $PATHS_TO_SCAN -name ".git" -type d); do
GIT_DIR=$(dirname $DOT_GIT_DIR)
cd $GIT_DIR
- GIT_URL=$(git config --get remote.origin.url)
+ GIT_URL=$(git config --get remote.$(git remote | head -1).url)
GIT_REV=$(git rev-parse HEAD)
jq_inplace $SRCMAP ".\"$GIT_DIR\" = { type: \"git\", url: \"$GIT_URL\", rev: \"$GIT_REV\" }"
doneIt's worth pointing out that these are still just assumptions. In the general case it's not possible to look at a .git directory and ask the question "from where was this pulled?". Maybe it would be better to allow projects to specify the srcmap themselves for edge cases like this? Is there a way for me to run srcmap locally in a way that corresponds to your build failures to test this? I'm digging around and getting a little lost.
Contributor
Author
There was a problem hiding this comment. Alternatively, I could update the project code to go in and deliberately add a "origin" remote that copies the one already there. That's more code and won't help anyone else, but might be more palatable?
Contributor
There was a problem hiding this comment. I think the origin trick would work. If you're using the There was a problem hiding this comment. FWIW
Contributor
Author
There was a problem hiding this comment. Global fix up at #10383 Will look at the project-side workaround now. Honestly didn't even know about west list, but that's cleaner than There was a problem hiding this comment. |
||||||
| RUN cd sof && west update | ||||||
|
||||||
| RUN cd sof && west update | |
| RUN cd sof && west update --fetch-opt=--filter=tree:0 |
It's better than --depth because it breaks neither git describe nor git merge-base thesofproject/linux#2556
More info in zephyrproject-rtos/west#638 and others
There was a problem hiding this comment.
Hm... getting fatal: --filter can only be used when extensions.partialClone is set when I try this? Maybe this is too bleeding edge for the git in the 20.04 image? Or maybe I just need to read some more docs. CI optimization is pretty far from my happy zone.
There was a problem hiding this comment.
Maybe this is too bleeding edge for the git in the 20.04 image?
Yes: I can reproduce with git 2.25.1 in Ubuntu 20.04 but not with git 2.34.1 in Ubuntu 22.04.
This happens only when trying to fetch with the --filter optimization on an existing, not already "filtered" clone (and only on 20.04).
This happens because west clones missing modules in two steps: 1. git initialization of a "non-filtered". 2. git fetch --filter=... (boom)
A tested workaround is git config remote.upstream.partialclonefilter tree:0 between 1. and 2. but of course you can't inject any command between west steps 1. and 2.
https://git-scm.com/docs/repository-version
Ubuntu 22.04 is not affected because git fetch with version 2.34.1 automagically adds the git config above.
Do you really need to use Ubuntu 20.04? We recently upgraded everything to 22.04 and we experienced a few bugs but absolutely nothing related to toolchains or building anything.
There was a problem hiding this comment.
You will get a 2x speed up with this recent and wonderful git option.
Finally woke up and realized this is run at image creation time; to sort-of "warm up a git cache". So while this git optimization trick is very good to know it's totally off-topic here, sorry the distraction. Resolving this comment.
EDIT: I can't resolve comments here for some reason. My shame will stay on display.
There was a problem hiding this comment.
@marc-hb its because we are not part of the repo
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,12 +19,30 @@ | |
| # | ||
| ################################################################################ | ||
|
|
||
| # Environment: Zephyr has its own toolchain selection mechanism, and | ||
| # the oss-fuzz environment generates warnings and confuses things when | ||
| # building some of the (un-fuzzed) build-time tools that aren't quite | ||
| # clang-compatible. | ||
| export ZEPHYR_TOOLCHAIN_VARIANT=llvm | ||
| unset CC | ||
| unset CCC | ||
| unset CXX | ||
| unset CFLAGS | ||
| unset CXXFLAGS | ||
|
Comment on lines
+22
to
+31
Contributor
There was a problem hiding this comment. Not really a fan of this. LibFuzzer is bundled with clang and using your own clang can cause many other problems. I guess if you do this, we can't help you with build issues.
Contributor
There was a problem hiding this comment. I may even revert this
Contributor
Author
There was a problem hiding this comment. This isn't being used to build the fuzzed binary, which is grabbing clang off of PATH. It's there because there's a build-time tool in SOF (that actually isn't used except on real firmware) that doesn't build with clang due to routine warning collisions. AFAICT @marc-hb fixed this upstream, not sure if that patch merged or not.
Contributor
There was a problem hiding this comment. OK, false alarm, this seems fine. There was a problem hiding this comment.
Yes it's merged: |
||
|
|
||
| BASE_CFG=( | ||
| -DCONFIG_ASSERT=y | ||
| -DCONFIG_SYS_HEAP_BIG_ONLY=y | ||
| -DCONFIG_ZEPHYR_NATIVE_DRIVERS=y | ||
| -DCONFIG_ARCH_POSIX_LIBFUZZER=y | ||
| -DCONFIG_ARCH_POSIX_FUZZ_TICKS=100 | ||
| -DCONFIG_ASAN=y | ||
| ) | ||
|
|
||
| cd $SRC/sof/sof | ||
|
|
||
| west build -p -b native_posix ./app -- "${BASE_CFG[@]}" | ||
| cp build/zephyr/zephyr.exe $OUT/sof-ipc3 | ||
|
|
||
| west build -p -b native_posix ./app -- "${BASE_CFG[@]}" -DCONFIG_IPC_MAJOR_4=y | ||
| cp build/zephyr/zephyr.exe $OUT/sof-ipc4 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,18 @@ | ||
| homepage: "https://thesofproject.github.io" | ||
| primary_contact: "andyross@google.com" | ||
| language: c | ||
| auto_ccs: | ||
| - "cujomalainey@chromium.org" | ||
| - "ranjani.sridharan@intel.corp-partner.google.com" | ||
| - "lgirdwood@gmail.com" | ||
| - "harsha.p.n@intel.corp-partner.google.com" | ||
| - "sathyanarayana.nujella@intel.corp-partner.google.com" | ||
| - "adrian.bonislawski@intel.com" | ||
| - "michal.wasko@intel.com" | ||
| fuzzing_engines: | ||
| - libfuzzer | ||
| sanitizers: | ||
| - address | ||
| architectures: | ||
| - i386 | ||
| main_repo: "https://github.com/thesofproject/sof" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be overriden on some --arg command line? If yes maybe say it for the Docker noobs among us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the docker level, yes.
docker build --build-arg SDK_VER=0.16.2would have the expected effect. I have no idea if that's wired meaingfully to the oss-fuzz infrastructure or not.Really this is just here to define a local variable for clarity. Docker has two variable tyes: ARG works like this and can be set externally, but only works within the dockerfile itself. ENV cannot be set externally, but also persists into the default environment variables of processes spawned in the container, which is undesirable here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bleh if the versions weren't part of the release file name this would be much easier. Seems like a common problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we will have to leave this as it, not much hope for fixing it without some additional logic