-
Notifications
You must be signed in to change notification settings - Fork 211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Get fuzztest tests to build for oss-fuzz #1894
Conversation
To test locally, follow the instructions at the top of the file. Also, limit CIFuzz to when a PR is merged. This can be revisited at some point but first: - let's make sure new oss-fuzz targets are built and created on the oss-fuzz site - let's make sure we fix all fuzzer bugs if any - let's see how long the job really takes to build
The trick was to set the proper defines when building. After that, the right exports had to be set for fuzzing to happen. The following now does not time out: python3 infra/helper.py check_build libavif
tests/oss-fuzz/build.sh
Outdated
cmake -G Ninja -DBUILD_SHARED_LIBS=OFF \ | ||
-DAVIF_CODEC_DAV1D=LOCAL -DAVIF_LIBYUV=LOCAL .. | ||
cmake -G Ninja -DBUILD_SHARED_LIBS=OFF -DAVIF_CODEC_AOM=LOCAL -DAVIF_CODEC_DAV1D=LOCAL \ | ||
-DAVIF_CODEC_AOM_DECODE=ON -DAVIF_CODEC_AOM_ENCODE=ON \ |
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.
Set AVIF_CODEC_AOM_DECODE
to OFF
.
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 am doing it but we want to test all stable decoding libraries no ? I know libaom has its own fuzzers but we could kill two birds with one stone by letting it ON no?
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.
You are right. If we have or will have fuzzers that test all available AV1 decoders, please add it back.
I was assuming our fuzzers only test the default AV1 decoder.
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.
Done in #1898
With fuzztest, we actually test AOM, dav1d and auto (cf
libavif/tests/gtest/avif_fuzztest_helpers.h
Line 141 in 633f5d2
fuzztest::ElementOf<avifCodecChoice>({AVIF_CODEC_CHOICE_AUTO, |
$CXX $CXXFLAGS -std=c++11 -I../include \ | ||
../tests/oss-fuzz/avif_decode_fuzzer.cc -o $OUT/avif_decode_fuzzer \ | ||
$LIB_FUZZING_ENGINE libavif.a ../ext/dav1d/build/src/libdav1d.a \ | ||
../ext/libyuv/build/libyuv.a | ||
../ext/libyuv/build/libyuv.a ../ext/aom/build.libavif/libaom.a |
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 just realized that we should link with the sharpyuv library. Is libavif.a still a combined archive library?
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.
libavif.a is a combined archive and that target actually compiles and runs (it's the only fuzzer running on oss-fuzz for now). Once we are sure that the other ones are running there, this target will be converted to fuzztest and this hand-crafted compilation will be removed.
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.
My previous comment wasn't clear. I meant that this target must link with the sharpyuv library when libavif.a is no longer a combined archive library, so we should link with the sharpyuv library now in preparation for that.
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 agree, but we should be able to switch that binary in the next few days :) The builds were successful, we just need to make sure the reports are created properly (that will happen after today is over). So far, the fuzztest tests have not appeared on https://oss-fuzz.com/fuzzer-stats?project=libavif&fuzzer=libFuzzer&job=libfuzzer_asan_libavif&group_by=by-fuzzer
This now works! I had missed two defines in the first attempt. Thx @DavidKorczynski for the help!
The first commit is the cherry-picked original one, so that we keep track of what I had missed.