Skip to content
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.

Sign up for GitHub

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

ci: add fuzz test targets to ci #7949

Merged
merged 8 commits into from
Aug 22, 2019
Merged

ci: add fuzz test targets to ci #7949

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0c0953a
add fuzz test targets to ci
asraa Aug 16, 2019
ee200ce
add bazel query and azp
asraa Aug 20, 2019
35bbf8c
change tag name and add iterations
asraa Aug 21, 2019
f2833fc
change to cc_test
asraa Aug 21, 2019
56255d9
remove one if and change test_env
asraa Aug 21, 2019
94c46bb
fix single case
asraa Aug 21, 2019
0045071
update docs
asraa Aug 21, 2019
545814f
move fuzz_test_target and add comment in bazelrc
asraa Aug 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .azure-pipelines/linux.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ jobs:
CI_TARGET: 'bazel.gcc'
compile_time_options:
CI_TARGET: 'bazel.compile_time_options'
fuzz:
CI_TARGET: 'bazel.fuzz'
dependsOn: [] # this removes the implicit dependency on previous stage and causes this to run in parallel.
timeoutInMinutes: 360
pool:
Expand Down
2 changes: 1 addition & 1 deletion bazel/envoy_test.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def envoy_cc_fuzz_test(name, corpus, deps = [], tags = [], **kwargs):
testonly = 1,
data = [corpus_name],
deps = [":" + test_lib_name],
tags = ["manual"] + tags,
tags = ["manual", "fuzzer"] + tags,
)

# Envoy C++ test targets should be specified with this function.
Expand Down
2 changes: 2 additions & 0 deletions ci/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ The `./ci/run_envoy_docker.sh './ci/do_ci.sh <TARGET>'` targets are:
* `bazel.coverity` &mdash; build Envoy static binary and run Coverity Scan static analysis.
* `bazel.tsan` &mdash; build and run tests under `-c dbg --config=clang-tsan` with clang.
* `bazel.tsan <test>` &mdash; build and run a specified test or test dir under `-c dbg --config=clang-tsan` with clang.
* `bazel.fuzz` &mdash; build and run fuzz tests under `-c dbg --config=asan-fuzzer` with clang.
* `bazel.fuzz <test>` &mdash; build and run a specified fuzz test or test dir under `-c dbg --config=asan-fuzzer` with clang.
* `bazel.compile_time_options` &mdash; build Envoy and run tests with various compile-time options toggled to their non-default state, to ensure they still build.
* `bazel.compile_time_options <test>` &mdash; build Envoy and run a specified test or test dir with various compile-time options toggled to their non-default state, to ensure they still build.
* `bazel.clang_tidy` &mdash; build and run clang-tidy over all source files.
Expand Down
16 changes: 16 additions & 0 deletions ci/do_ci.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,19 @@ CI_TARGET=$1

if [[ $# -gt 1 ]]; then
shift
# If this is a fuzz test, add _with_libfuzzer
if [[ "$CI_TARGET" == "bazel.fuzz" ]]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need this two ifs here? Move them to below in elif [[ "$CI_TARGET" == "bazel.fuzz" ]]; then and ingest from $TEST_TARGETS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ingested from $TEST_TARGET, the only awkward thing about it is that the user has to specify the "_with_libfuzzer" name for this to work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that's fine, at least wildcards like //test/server/... works, (which didn't work with previous one), it is hard to make this perfect anyway.

FUZZ_TEST_TARGETS=$*"_with_libfuzzer"
fi
TEST_TARGETS=$*
else
TEST_TARGETS=//test/...
if [[ "$CI_TARGET" == "bazel.fuzz" ]]; then
FUZZ_TEST_TARGETS="$(bazel query "attr('tags','fuzzer',//test/...)")"
fi
fi


if [[ "$CI_TARGET" == "bazel.release" ]]; then
# When testing memory consumption, we want to test against exact byte-counts
# where possible. As these differ between platforms and compile options, we
Expand Down Expand Up @@ -252,6 +260,14 @@ elif [[ "$CI_TARGET" == "bazel.coverity" ]]; then
"${ENVOY_BUILD_DIR}"/envoy-coverity-output.tgz \
"${ENVOY_DELIVERY_DIR}"/envoy-coverity-output.tgz
exit 0
elif [[ "$CI_TARGET" == "bazel.fuzz" ]]; then
setup_clang_toolchain
echo "bazel ASAN libFuzzer build with fuzz tests ${FUZZ_TEST_TARGETS}"
echo "Building envoy fuzzers and executing 100 fuzz iterations..."
for t in ${FUZZ_TEST_TARGETS}; do
bazel_with_collection run ${BAZEL_BUILD_OPTIONS} --config=asan-fuzzer $t -- -runs=10
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to run multiple target at same time, otherwise the CI time will be long, and not utilizing the resources we have in RBE.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like build and test can take multiple test targets, but run can't.
Would enough time be saved if I built the targets all at once, but ran them sequentially?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then my next question is: can you make fuzzer target as cc_test? cc_test is basically same as cc_binary with extra env vars, and you can pass args with --test_arg? The parallel run will not only save time but also network because testing with RBE doesn't download the binary (which is around 800MB per binary) to test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes! That worked swimmingly with something like
bazel test --config=asan-fuzzer //test/server/config_validation:config_fuzz_test_with_libfuzzer --test_arg="-runs=10"

It seems like it's linking dependencies now with ubsan (so it's crashing on ubsan errors in external libraries). Is this a difference between cc_binary and cc_test as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like it's linking dependencies now with ubsan (so it's crashing on ubsan errors in external libraries). Is this a difference between cc_binary and cc_test as well?

That shouldn't, though I don't know why are you seeing this, can you push your change so I can see what's happening in CI?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, the errors look like
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior external/com_google_protobuf/src/google/protobuf/map_type_handler.h:636:17

they should probably be caught by the fuzzer after the 10 iterations in at least some of the fuzz tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this is because UBSAN_OPTIONS and ASAN_OPTIONS are specified as --test_env here, so UBSAN failure is doesn't halt when you do bazel run but halts when you do bazel test, so the failures are legitimate I think? If you want override those options add --test_env in asan-fuzzer config.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Yeah, those are legitimate UBSan faults, but not in our test code.

done;
exit 0
elif [[ "$CI_TARGET" == "fix_format" ]]; then
echo "fix_format..."
./tools/check_format.py fix
Expand Down