From 1a815c6bfa4d5dc2cbef0df62dc12b7ce0d84be7 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sun, 11 Jun 2023 12:23:40 +0200 Subject: [PATCH] Fix split post-processing of LLVM-based coverage `--experimental_split_coverage_postprocessing` now works in combination with `--experimental_generate_llvm_lcov`, which required adding the runtime objects as inputs to the separate coverage post-processing spawn. Previously, they were only added to the runfiles of the test action, which aren't (and shouldn't) be added to the coverage spawn. As a side effect, this provides a more natural fix for #15121 then bb6f1a7ce79168055ccd62629da07d46a52b930d, which added an additional check to only add those runtime objects to the `llvm-cov` command line that were present in runfiles. Now, since all runtime objects are staged as inputs, they are known to be available. This improves coverage accuracy when e.g. runtime objects are packaged into jars and loaded at runtime. --- .../builtins_bzl/common/cc/cc_binary.bzl | 4 +- .../bazel/bazel_coverage_cc_test_llvm.sh | 49 +++++++ .../bazel/remote/remote_execution_test.sh | 123 ++++++++++++++++++ tools/test/collect_cc_coverage.sh | 4 +- 4 files changed, 175 insertions(+), 5 deletions(-) diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl index 2f1ef9ed66d6ed..4701a1c8f4bf21 100644 --- a/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl @@ -182,7 +182,7 @@ def _add(ctx, linking_statically): def _get_file_content(objects): result = [] for obj in objects: - result.append(obj.short_path) + result.append(obj.path) result.append("\n") return "".join(result) @@ -192,7 +192,7 @@ def _add_transitive_info_providers(ctx, cc_toolchain, cpp_config, feature_config runtime_objects_list = ctx.actions.declare_file(ctx.label.name + "runtime_objects_list.txt") file_content = _get_file_content(runtime_objects_for_coverage) ctx.actions.write(output = runtime_objects_list, content = file_content, is_executable = False) - additional_meta_data = [runtime_objects_list] + additional_meta_data = [runtime_objects_list] + runtime_objects_for_coverage instrumented_files_provider = cc_helper.create_cc_instrumented_files_info( ctx = ctx, diff --git a/src/test/shell/bazel/bazel_coverage_cc_test_llvm.sh b/src/test/shell/bazel/bazel_coverage_cc_test_llvm.sh index d2cf0dab910e94..c3aeaef51c970a 100755 --- a/src/test/shell/bazel/bazel_coverage_cc_test_llvm.sh +++ b/src/test/shell/bazel/bazel_coverage_cc_test_llvm.sh @@ -154,6 +154,55 @@ end_of_record" assert_equals "$expected_result" "$(cat $(get_coverage_file_path_from_test_log))" } +function test_cc_test_llvm_coverage_produces_lcov_report_with_split_postprocessing() { + local -r clang="/usr/bin/clang" + if [[ ! -x ${clang} ]]; then + return + fi + local -r clang_version=$(clang --version | grep -o "clang version [0-9]*" | cut -d " " -f 3) + if [ "$clang_version" -lt 9 ] || [ "$clang_version" -eq 10 ] || [ "$clang_version" -eq 11 ]; then + # No lcov produced with <9.0, no branch coverage with 10.0 and 11.0. + echo "clang versions <9.0 as well as 10.0 and 11.0 are not supported." && return + fi + + local -r llvm_profdata="/usr/bin/llvm-profdata" + if [[ ! -x ${llvm_profdata} ]]; then + return + fi + + local -r llvm_cov="/usr/bin/llvm-cov" + if [[ ! -x ${llvm_cov} ]]; then + return + fi + + setup_a_cc_lib_and_t_cc_test + + BAZEL_USE_LLVM_NATIVE_COVERAGE=1 GCOV=$llvm_profdata CC=$clang \ + BAZEL_LLVM_COV=$llvm_cov bazel coverage --experimental_generate_llvm_lcov \ + --experimental_split_coverage_postprocessing --experimental_fetch_all_coverage_outputs \ + --test_output=all //:t &>$TEST_log || fail "Coverage for //:t failed" + + + + local expected_result="SF:a.cc +FN:3,_Z1ab +FNDA:1,_Z1ab +FNF:1 +FNH:1 +DA:3,1 +DA:4,1 +DA:5,1 +DA:6,1 +DA:7,0 +DA:8,0 +DA:9,1 +LH:5 +LF:7 +end_of_record" + + assert_equals "$expected_result" "$(cat $(get_coverage_file_path_from_test_log))" +} + function test_cc_test_with_runtime_objects_not_in_runfiles() { local -r clang="/usr/bin/clang" if [[ ! -x ${clang} ]]; then diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index 10ec8d80feb919..adb89a1e2abc34 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -2457,6 +2457,129 @@ EOF fi } +# Runs coverage with `cc_test` using llvm-cov and RE, then checks that the coverage file is +# returned non-empty. +# See the above `test_java_rbe_coverage_produces_report` for more information. +function test_cc_rbe_coverage_produces_report_with_llvm() { + if [[ "$PLATFORM" == "darwin" ]]; then + # TODO(b/37355380): This test is disabled due to RemoteWorker not supporting + # setting SDKROOT and DEVELOPER_DIR appropriately, as is required of + # action executors in order to select the appropriate Xcode toolchain. + return 0 + fi + + local test_dir="a/cc/coverage_test" + mkdir -p $test_dir + + cat > "$test_dir"/BUILD <<'EOF' +package(default_visibility = ["//visibility:public"]) + +cc_library( + name = "hello-lib", + srcs = ["hello-lib.cc"], + hdrs = ["hello-lib.h"], +) + +cc_binary( + name = "hello-world", + srcs = ["hello-world.cc"], + deps = [":hello-lib"], +) + +cc_test( + name = "hello-test", + srcs = ["hello-world.cc"], + deps = [":hello-lib"], +) + +EOF + + cat > "$test_dir"/hello-lib.cc <<'EOF' +#include "hello-lib.h" + +#include + +using std::cout; +using std::endl; +using std::string; + +namespace hello { + +HelloLib::HelloLib(const string& greeting) : greeting_(new string(greeting)) { +} + +void HelloLib::greet(const string& thing) { + cout << *greeting_ << " " << thing << endl; +} + +} // namespace hello + +EOF + + cat > "$test_dir"/hello-lib.h <<'EOF' +#ifndef HELLO_LIB_H_ +#define HELLO_LIB_H_ + +#include +#include + +namespace hello { + +class HelloLib { + public: + explicit HelloLib(const std::string &greeting); + + void greet(const std::string &thing); + + private: + std::unique_ptr greeting_; +}; + +} // namespace hello + +#endif // HELLO_LIB_H_ + +EOF + + cat > "$test_dir"/hello-world.cc <<'EOF' +#include "hello-lib.h" + +#include + +using hello::HelloLib; +using std::string; + +int main(int argc, char** argv) { + HelloLib lib("Hello"); + string thing = "world"; + if (argc > 1) { + thing = argv[1]; + } + lib.greet(thing); + return 0; +} + +EOF + + BAZEL_USE_LLVM_NATIVE_COVERAGE=1 GCOV=llvm-profdata BAZEL_LLVM_COV=llvm-cov CC=clang \ + bazel coverage \ + --test_output=all \ + --experimental_fetch_all_coverage_outputs \ + --experimental_generate_llvm_lcov \ + --experimental_split_coverage_postprocessing \ + --spawn_strategy=remote \ + --remote_executor=grpc://localhost:${worker_port} \ + //"$test_dir":hello-test >& $TEST_log \ + || fail "Failed to run coverage for cc_test" + + # Different gcov versions generate different outputs. + # Simply check if this is empty or not. + if [[ ! -s bazel-testlogs/a/cc/coverage_test/hello-test/coverage.dat ]]; then + echo "Coverage is empty. Failing now." + return 1 + fi +} + function test_grpc_connection_errors_are_propagated() { # Test that errors when creating grpc connection are propagated instead of crashing Bazel. # https://github.com/bazelbuild/bazel/issues/13724 diff --git a/tools/test/collect_cc_coverage.sh b/tools/test/collect_cc_coverage.sh index 91747c76521def..d043562680cddd 100755 --- a/tools/test/collect_cc_coverage.sh +++ b/tools/test/collect_cc_coverage.sh @@ -85,9 +85,7 @@ function llvm_coverage_lcov() { while read -r line; do if [[ ${line: -24} == "runtime_objects_list.txt" ]]; then while read -r line_runtime_object; do - if [[ -e "${RUNFILES_DIR}/${TEST_WORKSPACE}/${line_runtime_object}" ]]; then - object_param+=" -object ${RUNFILES_DIR}/${TEST_WORKSPACE}/${line_runtime_object}" - fi + object_param+=" -object ${line_runtime_object}" done < "${line}" fi done < "${COVERAGE_MANIFEST}"