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.

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

Prototype support for llvm coverage #5036

Closed
wants to merge 1 commit into from

Conversation

ulfjack
Copy link
Contributor

@ulfjack ulfjack commented Apr 17, 2018

@ulfjack ulfjack requested review from hlopko and iirina April 17, 2018 14:25
@iirina
Copy link
Contributor

iirina commented Apr 17, 2018

This looks reasonable. Does it work for multiple tests? Did you try to generate the HTML report?

@ulfjack
Copy link
Contributor Author

ulfjack commented Apr 18, 2018

Right now, it's hard-coding llvm-profdata, so we shouldn't merge it like this. I think we should merge #5040 first and then 'abuse' GCOV to pass the right binary name to the collect-coverage.sh script. You can't use both gcov and llvm coverage in the same invocation anyway.

With that change (and a small change here to use GCOV), usage is like this:
BAZEL_USE_LLVM_NATIVE_COVERAGE=1 GCOV=llvm-profdata-3.9 CC=clang-3.9 bazel coverage --instrumentation_filter=src/main/cpp src/test/cpp:all

The patch works with sandboxing, i.e., it successfully copies the coverage data out of the sandbox to the local workspace.

I successfully generated an HTML report at least for an individual test, like this:
(LLVM_COV_CMD="llvm-cov-3.9 show -format=html -output-dir=$(pwd)/report/ -instr-profile bazel-out/k8-fastbuild/testlogs/src/test/cpp/option_processor_test/coverage.dat bazel-out/k8-fastbuild/bin/src/test/cpp/option_processor_test $(cat bazel-out/k8-fastbuild/bin/src/test/cpp/option_processor_test.runfiles_manifest | cut -d' ' -f 2 | egrep ".so$" | xargs -n 1 -I xxx echo -n "-object xxx ")" && cd bazel-os-bazel && $LLVM_COV_CMD)

The HTML report partially ignores the --instrumentation_filter option - it shows all the files in the HTML view, even those that weren't covered. There's probably a way to make it work by changing the llvm-cov command - we have a list of all the files that should be covered.

fi

if [[ "$USES_LLVM_COV" ]]; then
llvm-profdata-3.9 merge -output "${COVERAGE_OUTPUT_FILE}" "${COVERAGE_DIR}"/*.profraw
Copy link
Member

Choose a reason for hiding this comment

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

My machine has llvm-profdata-3.8 and llvm-profdata-4.0 :) Quick googling around suggests that its only llvm-profdata on osx. To make it run correctly on osx with xcode installed I read we should invoke it via xcrun (xcrun llvm-profdata).

One solution that we can use is to generate this file similarly to how crosstool is generated and expect users to provide relevant env variables. Thoughts?

https://clang.llvm.org/docs/SourceBasedCodeCoverage.html

Usage:
BAZEL_USE_LLVM_NATIVE_COVERAGE=1 GCOV=llvm-profdata-3.9 CC=clang-3.9 bazel coverage --instrumentation_filter=src/main/cpp src/test/cpp:all
@ulfjack
Copy link
Contributor Author

ulfjack commented Apr 25, 2018

I've changed the patch to use the GCOV variable that's now set by cc_configure (since 9566f67 is in). Ideally, we'd make cc_configure automatically detect that it's configuring clang and which version, and set GCOV accordingly, including on OSX. For now, it's at least possible to use the LLVM coverage implementation.

@ulfjack
Copy link
Contributor Author

ulfjack commented Apr 25, 2018

I've also added usage instructions to the change description.

@hlopko
Copy link
Member

hlopko commented Apr 25, 2018

LGTM.

@hlopko
Copy link
Member

hlopko commented Apr 25, 2018

Testing cc_configure would be awesome. But the best approach I thought of is to have multiple CI slaves, each set up with different combination and versions of gcc/clang/gcov/libstdc++/libc++/full Xcode/command line tools only, and running all of this on ubuntu debian and mac :)

@ulfjack
Copy link
Contributor Author

ulfjack commented Apr 25, 2018 via email

@bazel-io bazel-io closed this in 1441a3a Apr 26, 2018
@ulfjack ulfjack mentioned this pull request May 7, 2018
@ulfjack ulfjack deleted the llvm-coverage branch May 15, 2018 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants