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

Jump to bottom

Add coverage support for fuzzing_regression_test #174

Closed
wants to merge 1 commit into from
Closed

Add coverage support for fuzzing_regression_test #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 12 additions & 0 deletions .bazelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,15 @@ build:asan-jazzer --@rules_fuzzing//fuzzing:cc_engine_instrumentation=jazzer
build:asan-jazzer --@rules_fuzzing//fuzzing:cc_engine_sanitizer=asan
# Workaround for https://github.com/bazelbuild/bazel/issues/11128
build:asan-jazzer --//fuzzing:cc_engine_sanitizer=asan

# Coverage with Replay (C/C++ only)
coverage --//fuzzing:cc_engine=//fuzzing/engines:replay
coverage --@rules_fuzzing//fuzzing:cc_engine_instrumentation=none
coverage --@rules_fuzzing//fuzzing:cc_engine_sanitizer=none
coverage --instrument_test_targets
coverage --action_env=BAZEL_USE_LLVM_NATIVE_COVERAGE=1
coverage --action_env=GCOV=llvm-profdata-10
coverage --action_env=BAZEL_LLVM_COV=llvm-cov-10
coverage --combined_report=lcov
coverage --experimental_use_llvm_covmap
coverage --experimental_generate_llvm_lcov
stefanbucur marked this conversation as resolved.
Show resolved Hide resolved
22 changes: 22 additions & 0 deletions .github/workflows/bazel_test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,28 @@ jobs:
bazel test --verbose_failures --test_output=all \
--build_tag_filters=fuzz-test --config=${{ matrix.config }} \
//examples:all
coverage:
name: Coverage gathering (C++)
runs-on: ubuntu-latest
timeout-minutes: 30
strategy:
matrix:
target: ["//examples:empty_fuzz_test", "//examples:re2_fuzz_test"]
steps:
- name: Checkout repository
uses: actions/checkout@v2
- name: Install dependencies
run: |
sudo apt-get update && sudo apt-get install -yq \
clang-10 \
libunwind-dev \
libblocksruntime-dev
- name: Gather coverage
run: |
bazel coverage ${{ matrix.target }}
- name: Check coverage gathered is non-empty
run: |
[[ -s bazel-out/_coverage/_coverage_report.dat ]]
fuzzer_run_tests_java:
name: Brief fuzz test runs (Java)
runs-on: ubuntu-latest
Expand Down
4 changes: 4 additions & 0 deletions fuzzing/private/binary.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@ def _fuzzing_binary_impl(ctx):
engine_info = ctx.attr.engine[FuzzingEngineInfo],
options_file = ctx.file.options,
),
coverage_common.instrumented_files_info(
ctx,
dependency_attributes = ["binary"],
),
]

_common_fuzzing_binary_attrs = {
Expand Down
19 changes: 18 additions & 1 deletion fuzzing/private/regression.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,14 @@ exec '{engine_launcher}'
runfiles = ctx.runfiles()
runfiles = runfiles.merge(ctx.attr.binary[DefaultInfo].default_runfiles)
runfiles = runfiles.merge(binary_info.engine_info.launcher_runfiles)
return [DefaultInfo(executable = script, runfiles = runfiles)]

return [
DefaultInfo(executable = script, runfiles = runfiles),
coverage_common.instrumented_files_info(
ctx,
dependency_attributes = ["binary"],
),
]

fuzzing_regression_test = rule(
implementation = _fuzzing_regression_test_impl,
Expand All @@ -61,6 +68,16 @@ Executes a fuzz test on its seed corpus.
cfg = "target",
mandatory = True,
),
"_lcov_merger": attr.label(
default = "@bazel_tools//tools/test:lcov_merger",
executable = True,
cfg = "host",
),
"_collect_cc_coverage": attr.label(
default = "@bazel_tools//tools/test:collect_cc_coverage",
executable = True,
cfg = "host",
),
Comment on lines +71 to +80
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I need to understand this better: Why do we need to add these dependencies? Are these going to be built even in non-coverage mode?

(It would help if you could point me to some documentation discussing this.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

collect_cc_coverage runs the gcov/llvm-cov commands to gather coverage.

lcov_merger merges different coverage output files into a single output file for consumption.

I found these dependencies from the implementation of the native cc_test rule.

They will be built in non-coverage mode. Native rules with coverage support use late-bound attributes to avoid building these dependencies unnecessarily, but that's not yet an option in Starlark.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately I'm reluctant to accept this change as-is. I understand this is a Starlark limitation, but the workaround of including these dependencies on all builds is inadequate IMO.

Let me do some additional research to better understand all of our alternatives. If coverage support is not mature enough yet in Bazel, I would postpone the addition of this feature for now.

Copy link
Member

Choose a reason for hiding this comment

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

This should be fixed by bazelbuild/bazel#13983. Until then, I don't know of a better solution.

Copy link
Member

Choose a reason for hiding this comment

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

The underlying issue is now a release blocker for Bazel 5, so it's likely that lcov_merger will be provided by default in a way that it would be built only with bazel coverage. The same does not apply to collect_cc_coverage, but that is merely a shell script and thus causes no compilation overhead.

Copy link
Member

Choose a reason for hiding this comment

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

My fix will not be accepted, so this will very likely not be possible with Bazel 5.

Copy link
Member

Choose a reason for hiding this comment

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

@zhenyudg I haven't forgotten about this. If you want to know whether there has been progress in getting the underlying issue resolved in Bazel, you can track bazelbuild/bazel#8736.

Copy link
Member

@fmeum fmeum Feb 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@zhenyudg While working on the upstream fix, I may have found a workaround for Bazel 4+: Use an alias as the default for _lcov_merger and in that alias, use a select on a config_setting for collect_code_coverage to switch between the real lcov_merger target and a dummy target (e.g., an empty executable file). Let me know if you need help with this.

},
test = True,
)