Message ID | 20200611061355.31967-1-alxndr@bu.edu |
---|---|
State | New |
Headers | show |
Series | [v3] fuzz: add oss-fuzz build-script | expand |
Patchew URL: https://patchew.org/QEMU/20200611061355.31967-1-alxndr@bu.edu/ Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 20200611061355.31967-1-alxndr@bu.edu Subject: [PATCH v3] fuzz: add oss-fuzz build-script Type: series === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu - [tag update] patchew/20200611055807.15921-1-huth@tuxfamily.org -> patchew/20200611055807.15921-1-huth@tuxfamily.org Switched to a new branch 'test' 94a2568 fuzz: add oss-fuzz build-script === OUTPUT BEGIN === WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #28: new file mode 100755 ERROR: trailing whitespace #108: FILE: scripts/oss-fuzz/build.sh:76: +for i in $(ldd ./i386-softmmu/qemu-fuzz-i386 | cut -f3 -d' '); do $ total: 1 errors, 1 warnings, 106 lines checked Commit 94a2568dab4e (fuzz: add oss-fuzz build-script) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20200611061355.31967-1-alxndr@bu.edu/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
Hi Alex, On Thursday, 2020-06-11 at 02:13:55 -04, Alexander Bulekov wrote: > It is neater to keep this in the QEMU repo, since any change that > requires an update to the oss-fuzz build configuration, can make the > necessary changes in the same series. > > Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu> > --- > > In v3 I tried to make this build outside the oss-fuzz docker > environment. I wasn't able to find a way to use the Makefile to install > the pc-bios and qemu-fuzz binaries per Philippe's suggestion. > Additionally, right now I create a separate build directory within the > the tree for build. I am not sure whether this is a good approach, but > we must rely on some default that will work with both oss-fuzz and on > a developer's machine. I'm happy that it can be used outside of OSS-Fuzz to permit creating and testing without having to set up the whole OSS-Fuzz test environment, especially for small changes. Personally, I think not using the Makefile is fine for this specific use-case - it's a very specific environment/configuration. So, in general, Reviewed-by: Darren Kenny <darren.kenny@oracle.com> but there are a couple of comments below, mostly suggestions... > > MAINTAINERS | 1 + > scripts/oss-fuzz/build.sh | 99 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 100 insertions(+) > create mode 100755 scripts/oss-fuzz/build.sh > > diff --git a/MAINTAINERS b/MAINTAINERS > index 3abe3faa4e..094a37ebb3 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2336,6 +2336,7 @@ R: Bandan Das <bsd@redhat.com> > R: Stefan Hajnoczi <stefanha@redhat.com> > S: Maintained > F: tests/qtest/fuzz/ > +F: scripts/oss-fuzz/ > > Register API > M: Alistair Francis <alistair@alistair23.me> > diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh > new file mode 100755 > index 0000000000..4be6b21caf > --- /dev/null > +++ b/scripts/oss-fuzz/build.sh > @@ -0,0 +1,99 @@ > +#!/bin/sh It may be worth adding a '-e' option here, to have the script always fail on uncaught errors. > +# > +# OSS-Fuzz build script. See: > +# https://google.github.io/oss-fuzz/getting-started/new-project-guide/#buildsh > +# > +# The file is consumed by: > +# https://github.com/google/oss-fuzz/blob/master/projects/qemu/Dockerfiles > +# > +# This code is licensed under the GPL version 2 or later. See > +# the COPYING file in the top-level directory. > +# > + > +# build project > +# e.g. > +# ./autogen.sh > +# ./configure > +# make -j$(nproc) all > + > +# build fuzzers > +# e.g. > +# $CXX $CXXFLAGS -std=c++11 -Iinclude \ > +# /path/to/name_of_fuzzer.cc -o $OUT/name_of_fuzzer \ > +# $LIB_FUZZING_ENGINE /path/to/library.a > + > +# There seems to be a bug in clang-11 (used for builds on oss-fuzz) : > +# accel/tcg/cputlb.o: In function `load_memop': > +# accel/tcg/cputlb.c:1505: undefined reference to `qemu_build_not_reached' > +# > +# When building with optimization, the compiler is expected to prove that the > +# statement cannot be reached, and remove it. For some reason clang-11 doesn't > +# remove it, resulting in an unresolved reference to qemu_build_not_reached > +# Undefine the __OPTIMIZE__ macro which compiler.h relies on to choose whether > +# to " #define qemu_build_not_reached() g_assert_not_reached() " > +EXTRA_CFLAGS="$CFLAGS -U __OPTIMIZE__" > + > +if ! { [ -e "./COPYING" ] && > + [ -e "./MAINTAINERS" ] && > + [ -e "./Makefile" ] && > + [ -e "./docs" ] && > + [ -e "./VERSION" ] && > + [ -e "./linux-user" ] && > + [ -e "./softmmu" ];} ; then > + echo "Please run the script from the top of the QEMU tree" > + exit It's just a suggestion, but I've always favoured creating a specific function to handle exits, e.g.: fatal() { echo "Error: %{*}, exiting." exit 1 } and then using that anywhere there is an unexpected exit, like here. > +fi > + > +mkdir -p "./build-oss-fuzz/" > +cd "./build-oss-fuzz/" || exit fatal() also becomes useful here, to allow a meaningful exit message - very useful later if you're looking back at logs and wondering why it exited early. Might also be worth adding the || fatal '..' to the mkdir line too, which is more likely to fail first if the cd is going to fail. > + > + > +if [ -z ${LIB_FUZZING_ENGINE+x} ]; then > + LIB_FUZZING_ENGINE="-fsanitize=fuzzer" > +fi > + > +if [ -z ${OUT+x} ]; then > + DEST_DIR=$(realpath "./DEST_DIR") > +else > + DEST_DIR=$OUT > +fi > + > +mkdir -p "$DEST_DIR/lib/" # Copy the shared libraries here > + > +# Build once to get the list of dynamic lib paths, and copy them over > +../configure --disable-werror --cc="$CC" --cxx="$CXX" \ > + --extra-cflags="$EXTRA_CFLAGS" > + > +if ! make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" "-j$(nproc)" \ > + i386-softmmu/fuzz; then > + echo "Build failed. Please specify a compiler with fuzzing support"\ > + "using the \$CC and \$CXX environemnt variables, or specify a"\ > + "\$LIB_FUZZING_ENGINE compatible with your compiler" > + echo "For example: CC=clang CXX=clang++ $0" > + exit 0 This is more of an error condition, so probably would benefit from a non-zero exit code, otherwise something testing this build script for success would end up continuing when in reality it probably shouldn't. > +fi > + > +for i in $(ldd ./i386-softmmu/qemu-fuzz-i386 | cut -f3 -d' '); do > + cp "$i" "$DEST_DIR/lib/" > +done > +rm ./i386-softmmu/qemu-fuzz-i386 > + > +# Build a second time to build the final binary with correct rpath > +../configure --bindir="$DEST_DIR" --datadir="$DEST_DIR/data/" --disable-werror \ > + --cc="$CC" --cxx="$CXX" --extra-cflags="$EXTRA_CFLAGS" \ > + --extra-ldflags="-Wl,-rpath,'\$\$ORIGIN/lib'" > +make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" "-j$(nproc)" i386-softmmu/fuzz > + > +# Copy over the datadir > +cp -r ../pc-bios/ "$DEST_DIR/pc-bios" > + > +# Run the fuzzer with no arguments, to print the help-string and get the list > +# of available fuzz-targets. Copy over the qemu-fuzz-i386, naming it according > +# to each available fuzz target (See 05509c8e6d fuzz: select fuzz target using > +# executable name) > +for target in $(./i386-softmmu/qemu-fuzz-i386 | awk '$1 ~ /\*/ {print $2}'); > +do > + cp ./i386-softmmu/qemu-fuzz-i386 "$DEST_DIR/qemu-fuzz-i386-target-$target" > +done > + > +echo "Done. The fuzzers are located in $DEST_DIR" Add an 'exit 0' here. > -- > 2.26.2 Thanks, Darren.
diff --git a/MAINTAINERS b/MAINTAINERS index 3abe3faa4e..094a37ebb3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2336,6 +2336,7 @@ R: Bandan Das <bsd@redhat.com> R: Stefan Hajnoczi <stefanha@redhat.com> S: Maintained F: tests/qtest/fuzz/ +F: scripts/oss-fuzz/ Register API M: Alistair Francis <alistair@alistair23.me> diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh new file mode 100755 index 0000000000..4be6b21caf --- /dev/null +++ b/scripts/oss-fuzz/build.sh @@ -0,0 +1,99 @@ +#!/bin/sh +# +# OSS-Fuzz build script. See: +# https://google.github.io/oss-fuzz/getting-started/new-project-guide/#buildsh +# +# The file is consumed by: +# https://github.com/google/oss-fuzz/blob/master/projects/qemu/Dockerfiles +# +# This code is licensed under the GPL version 2 or later. See +# the COPYING file in the top-level directory. +# + +# build project +# e.g. +# ./autogen.sh +# ./configure +# make -j$(nproc) all + +# build fuzzers +# e.g. +# $CXX $CXXFLAGS -std=c++11 -Iinclude \ +# /path/to/name_of_fuzzer.cc -o $OUT/name_of_fuzzer \ +# $LIB_FUZZING_ENGINE /path/to/library.a + +# There seems to be a bug in clang-11 (used for builds on oss-fuzz) : +# accel/tcg/cputlb.o: In function `load_memop': +# accel/tcg/cputlb.c:1505: undefined reference to `qemu_build_not_reached' +# +# When building with optimization, the compiler is expected to prove that the +# statement cannot be reached, and remove it. For some reason clang-11 doesn't +# remove it, resulting in an unresolved reference to qemu_build_not_reached +# Undefine the __OPTIMIZE__ macro which compiler.h relies on to choose whether +# to " #define qemu_build_not_reached() g_assert_not_reached() " +EXTRA_CFLAGS="$CFLAGS -U __OPTIMIZE__" + +if ! { [ -e "./COPYING" ] && + [ -e "./MAINTAINERS" ] && + [ -e "./Makefile" ] && + [ -e "./docs" ] && + [ -e "./VERSION" ] && + [ -e "./linux-user" ] && + [ -e "./softmmu" ];} ; then + echo "Please run the script from the top of the QEMU tree" + exit +fi + +mkdir -p "./build-oss-fuzz/" +cd "./build-oss-fuzz/" || exit + + +if [ -z ${LIB_FUZZING_ENGINE+x} ]; then + LIB_FUZZING_ENGINE="-fsanitize=fuzzer" +fi + +if [ -z ${OUT+x} ]; then + DEST_DIR=$(realpath "./DEST_DIR") +else + DEST_DIR=$OUT +fi + +mkdir -p "$DEST_DIR/lib/" # Copy the shared libraries here + +# Build once to get the list of dynamic lib paths, and copy them over +../configure --disable-werror --cc="$CC" --cxx="$CXX" \ + --extra-cflags="$EXTRA_CFLAGS" + +if ! make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" "-j$(nproc)" \ + i386-softmmu/fuzz; then + echo "Build failed. Please specify a compiler with fuzzing support"\ + "using the \$CC and \$CXX environemnt variables, or specify a"\ + "\$LIB_FUZZING_ENGINE compatible with your compiler" + echo "For example: CC=clang CXX=clang++ $0" + exit 0 +fi + +for i in $(ldd ./i386-softmmu/qemu-fuzz-i386 | cut -f3 -d' '); do + cp "$i" "$DEST_DIR/lib/" +done +rm ./i386-softmmu/qemu-fuzz-i386 + +# Build a second time to build the final binary with correct rpath +../configure --bindir="$DEST_DIR" --datadir="$DEST_DIR/data/" --disable-werror \ + --cc="$CC" --cxx="$CXX" --extra-cflags="$EXTRA_CFLAGS" \ + --extra-ldflags="-Wl,-rpath,'\$\$ORIGIN/lib'" +make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" "-j$(nproc)" i386-softmmu/fuzz + +# Copy over the datadir +cp -r ../pc-bios/ "$DEST_DIR/pc-bios" + +# Run the fuzzer with no arguments, to print the help-string and get the list +# of available fuzz-targets. Copy over the qemu-fuzz-i386, naming it according +# to each available fuzz target (See 05509c8e6d fuzz: select fuzz target using +# executable name) +for target in $(./i386-softmmu/qemu-fuzz-i386 | awk '$1 ~ /\*/ {print $2}'); +do + cp ./i386-softmmu/qemu-fuzz-i386 "$DEST_DIR/qemu-fuzz-i386-target-$target" +done + +echo "Done. The fuzzers are located in $DEST_DIR"
It is neater to keep this in the QEMU repo, since any change that requires an update to the oss-fuzz build configuration, can make the necessary changes in the same series. Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Alexander Bulekov <alxndr@bu.edu> --- In v3 I tried to make this build outside the oss-fuzz docker environment. I wasn't able to find a way to use the Makefile to install the pc-bios and qemu-fuzz binaries per Philippe's suggestion. Additionally, right now I create a separate build directory within the the tree for build. I am not sure whether this is a good approach, but we must rely on some default that will work with both oss-fuzz and on a developer's machine. MAINTAINERS | 1 + scripts/oss-fuzz/build.sh | 99 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100755 scripts/oss-fuzz/build.sh