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

Add Coverage Analysis (à la DMD) #878

Merged
merged 1 commit into from
Apr 8, 2015

Conversation

JohanEngelen
Copy link
Member

This PR adds coverage analysis to LDC2 (cmdline option -cov[=n%]), equal to DMD's coverage analysis (http://dlang.org/code_coverage.html).

Two data structures are added to the module object file for coverage analysis: _d_cover_data and _d_cover_valid. _d_cover_data is an array of the execution count for each line of code (the execution count is atomically incremented at the start of each statement). _d_cover_valid is a bit-array that for each line says whether there is a statement on that line for which the execution count must be printed.
Each module compiled with -cov is registered for coverage output by calling _d_cover_register2() in druntime/rt/cover.d, passing _d_cover_data and _d_cover_valid. Then, druntime will take care of outputting the coverage listing. _d_cover_register2() is called by a shared static constructor.

There is a bug in druntime's interface of _d_cover_register2(). The _d_cover_valid bit array must be passed as a "size_t[] valid", however, the length field of the slice must be the number of bits in the array (number of lines). I will submit a PR to druntime to fix this, but do not know if it will be accepted. The code in this PR is very conservative and creates an oversized array for _d_cover_valid, such that the slice length field is equal to (at least) number of lines.

Tested using MSVC, x64.

@redstar
Copy link
Member

redstar commented Mar 31, 2015

Looks nice. I try to do a review this evening.

@dnadlinger
Copy link
Member

Nice work! Did you try how your implementation fares against the test cases from the DMD test-suite? (There is a couple of them which are currently commented out. Just search for -cov on a REQUIRED_ARGS line.)

@dnadlinger
Copy link
Member

Just post that druntime PR – I suppose the easiest solution would be to turn the slice into a pointer and a separate value for the length in bits?

@JohanEngelen
Copy link
Member Author

@klickverbot I was thinking about another solution for the druntime bug, preserving the function's signature. But I think yours is simpler and better.

@redstar
Copy link
Member

redstar commented Apr 1, 2015

In general, the coding looks good. Could you please check the Travis build? There are compile errors with the different LLVM versions we support.
A PR requires a green Travis build. (At least, a "virtual" green build. There is an out-of-memory problem in the std.algorithm unit test which sometimes kills the build. You can ignore this failure.)


void emitCoverageLinecountInc(Loc &loc);

#endif LDC_GEN_COVERAGE_H
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the LDC_GEN_COVERAGE. The file fits on a screen so this does not add information.

@redstar
Copy link
Member

redstar commented Apr 3, 2015

Looks good known. Could you please rebase your commits into one? Then I merge this PR.

@redstar
Copy link
Member

redstar commented Apr 3, 2015

BTW: Could you please check ldmd2? I think the -cov switch is parsed but I don't know if it passed to ldc2. The help text is currently disabled. You can provide another PR for this.

@JohanEngelen
Copy link
Member Author

@redstar ldmd2 should now correctly handle -cov. I've squashed all commits into a single one.
Thanks!

@JohanEngelen
Copy link
Member Author

@klickverbot See https://issues.dlang.org/show_bug.cgi?id=14417 .

@JohanEngelen
Copy link
Member Author

@redstar "all" Travis builds are green now.
I've also put in a PR to enable the coverage analysis tests: ldc-developers/dmd-testsuite#7

@redstar
Copy link
Member

redstar commented Apr 8, 2015

@JohanEngelen Great work! Thank you!

redstar added a commit that referenced this pull request Apr 8, 2015
Add Coverage Analysis (à la DMD)
@redstar redstar merged commit b1ff093 into ldc-developers:master Apr 8, 2015
@JohanEngelen JohanEngelen deleted the coverage branch April 8, 2015 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants