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

Code coverage runtime #20539

Closed
wants to merge 168 commits into from
Closed

Code coverage runtime #20539

wants to merge 168 commits into from

Conversation

myrrc
Copy link
Contributor

@myrrc myrrc commented Feb 15, 2021

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Build/Testing/Packaging Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Сode coverage runtime for functional tests (replaces coverage CI check).
Uses clang SanitizerCoverage callbacks to store data on a per-test basis and dump them after testing.
Introduces critical edges coverage metric.

myrrc and others added 6 commits February 15, 2021 02:48
moved coverage to base/common

temp add

Small bugfixes regarding old-style casts and variable renames

Removed old include

Removed old declaration

Added the initial dump callback

Added the definition to hide the sanitizer callbacks
@myrrc

This comment has been minimized.

@myrrc
Copy link
Contributor Author

myrrc commented Feb 16, 2021

Looks like it's the harmful contrib that was causing the bugs, removed it from the coverage builds as we won't need it anyway.

is better for build with and without code coverage
@myrrc

This comment has been minimized.

@robot-ch-test-poll2 robot-ch-test-poll2 added the submodule changed At least one submodule changed in this PR. label Apr 7, 2021
@myrrc
Copy link
Contributor Author

myrrc commented Apr 7, 2021

A word about the IPC between the tester script and the server -- currently we have 3 main options:

  1. Utilizing the tester's --testname option which executes a SELECT testname query before running the test.
    The issue here is that we need to interfere with the query process pipeline -- either write a hook or handle the special coverage case with a macro.

  2. Setting some option like SET coverage_test_name = name; Same issue with the pipeline.

  3. Sending a signal from the tester to the server. This option is "cheaper" in a way, the only issue here is how to attach the test_id to the signal. The server uses the POSIX signal handler with the SA_SIGINFO. The tester preloads libc and executes the sigqueue function.

as clang's sanitizer blacklist options are broken
@myrrc

This comment has been minimized.

@myrrc
Copy link
Contributor Author

myrrc commented Aug 5, 2021

Depends on #27228

@myrrc
Copy link
Contributor Author

myrrc commented Aug 6, 2021

Depends on #27361

@myrrc
Copy link
Contributor Author

myrrc commented Aug 10, 2021

I think this PR won't be done in near future, so I'm closing it.

If you operate entirely on the information provided by the binary itself (with or without PC-table), all you can get is addresses that were hit (or some basic blocks indices that were hit). You can symbolize this data and get the source file and line, but the problem is that, you can't really do anything with this information.

Multiple basic blocks can correspond to some lines (e.g. function template instantiations). Basic blocks can correspond to lines not belonging to functions (e.g. macro expansion like settings traits implementation).
If you instrument the code at critical edges level, you lose the access to basic blocks structure, these addresses (of edges) could as well be generated randomly as there's no profit from using them.
You won't be able to get lines ranges that were hit as C++ is hard to parse and you need a compiler.

If you involve the compiler (.gcno), things don't get better. clang does not enumerate its basic blocks, so there's no way to match the basic block that was hit, for example, in the boolean counters array (the PC-table) with a basic block parsed from a gcno file. You get stuck at the function level. Sorting-transforming basic blocks line ranges still does not solve the problem (moreover, .gcno produces strange results like a line belonging to a basic block that's empty). gcno parsers like lcov use perl scripts containing thousands of lines to turn all this information into a human-readable format.

And if one day you want to try the source-based code coverage, you'll also fail. There's no way to tweak what data are stored and tracked (unless you patch the compiler which obviously is not a good idea here), so you just collect everything (and it takes >= 9 hours currently for a general run without per-test data). All you can do is use builtins provided to write the report to some other location, but it won't make everything substantially faster.

You may wonder: why can't you simply collect addresses that were hit and display them? Well, for CH purposes it's a) useless, as one needs to get the coverage percentage, and b) useless, as you reinvent the sancov wheel.

I doubt there's a good way to resolve this PR.

@alexey-milovidov
Copy link
Member

You may wonder: why can't you simply collect addresses that were hit and display them?

I'm going to do exactly this in #56102

useless, as one needs to get the coverage percentage

The percentage can be calculated as the percentage of all instrumented edges, the percentage of instrumented symbols in the binary, functions, or source files. The percentage of lines can be calculated roughly if you take the assumption that every basic block spans from the line corresponding to its address to the next instrumented line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-build Pull request with build/testing/packaging improvement submodule changed At least one submodule changed in this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants