Page MenuHomePhabricator

Create llvm-cov structured export submodule

Authored by ehurtig on Jul 21 2016, 2:39 PM.



This patch enables users to export coverage information into portable JSON for use by analysis tools and storage in document based databases. This will expose the coverage information gathered by the instrumentation in such a fashion that automated tooling will be able to process coverage information.

The export submodule is invoked just like the the report subcommand

$ llvm-cov export -instr-profile /path/to/foo.profdata /path/to/foo

The resulting JSON contains a list of Files and Functions.

Every file object contains a list of segments, expansions, and a summary of the file's region, function, and line coverage.

Every function object contains the function's name and regions

There is also a total summary for the entire object file.

A documentation patch is incoming to update

Diff Detail


Event Timeline

ehurtig updated this revision to Diff 64963.Jul 21 2016, 2:39 PM
ehurtig retitled this revision from to Create llvm-cov structured export submodule.
ehurtig updated this object.
ehurtig added reviewers: llvm-commits, vsk.
ehurtig set the repository for this revision to rL LLVM.
ehurtig changed the visibility from "Public (No Login Required)" to "Custom Policy".
ehurtig added a subscriber: ehurtig.
vsk edited reviewers, added: MaggieYi, bogner; removed: llvm-commits.Jul 21 2016, 3:04 PM
vsk added a subscriber: llvm-commits.
vsk changed the visibility from "Custom Policy" to "Public (No Login Required)".Jul 21 2016, 3:33 PM
vsk edited edge metadata.Jul 21 2016, 6:21 PM

Thank you for working on this! I just have small nitpicks -- functionally, this patch looks like it's in great shape.

Could you add a short (1-2 line) description of the export command to docs/CommandGuide/llvm-cov.rst?

8 ↗(On Diff #64963)

Can you use your shorter filename-matching regex for all filenames? Something like {{[^\"]+}}showHighlightedRanges.cpp in this case.

10 ↗(On Diff #64963)

Here, it would be less brittle / more succinct to say:

CHECK-SAME: {{(\[[0-9]+,[0-9]+,[0-9]+,[0-9]+,[0-9]+\],?)+}}

We should do this for all arrays of regions/segments. If we need to change the format later on, it'll be much easier to do so this way. Plus, it's easier to check that a regex is correct vs. the actual region counts.

35 ↗(On Diff #64963)

Why are there 4 of the same filenames? It sounds like a bug.. maybe fixed by clang/r275924 (remove '..' from filenames and normalize them)?

92 ↗(On Diff #64963)

No reference needed here, since an int64_t is cheaper to copy than to deref+load. Also, value should be capitalized (this applies elsewhere).

199 ↗(On Diff #64963)

Stray semicolon.

202 ↗(On Diff #64963)

Unused parameter. Please drop.

243 ↗(On Diff #64963)

Change renderFile to accept a const CoverageData &, and pass FileCoverage by reference. That's cheaper.

264 ↗(On Diff #64963)

This should be const auto &.

265 ↗(On Diff #64963)

Braces around single-statement loops are usually dropped.

296 ↗(On Diff #64963)

Should be const CoverageSegment &.

345 ↗(On Diff #64963)

Stray semi-colon.

348 ↗(On Diff #64963)

Should be const CountedRegion &.

366 ↗(On Diff #64963)

Should be const FileCoverageSummary &.

ehurtig marked 10 inline comments as done.Jul 22 2016, 3:28 PM
ehurtig added inline comments.
35 ↗(On Diff #64963)

Yeah I thought it was a bug at first, however, I have a feeling it is expected and has to do with nested expansions. The source for showExpansions.cpp:

  do {                      \
  } while (0)
#define ANOTHER_THING() \
  do {                  \
    if (0) {            \
    }                   \
  } while (0)

#define DO_SOMETHING(x)    \
  do {                     \
    if (x)                 \
    else                   \
      ANOTHER_THING();     \
  } while (0)
// CHECK-DAG: Expansion at line [[@LINE-4]], 7 -> 24
// CHECK-DAG: Expansion at line [[@LINE-3]], 7 -> 20

int main(int argc, const char *argv[]) {
  for (int i = 0; i < 100; ++i)
    DO_SOMETHING(i); // CHECK-DAG: Expansion at line [[@LINE]], 5 -> 17
  return 0;

0. FileID for the "Main" or "Source" file.

  1. For the expansion of DO_SOMETHING
  2. For the expansion of ANOTHER_THING
  3. For the expansion of DO_SOMETHING_ELSE

Not saying whether it is right or wrong, just offering what I think is the reason for the duplicate filenames and separate FileIDs

202 ↗(On Diff #64963)

In order to match the rest of the render* functions, and specifically the renderFiles(... list of files ...) function, I have fixed the function to actually use this parameter and actually render the functions given to it.

243 ↗(On Diff #64963)

There were some issues blocking this that are now resolved by some NFCs in r276474 and thus I have made the necessary changes to resolve this as well

ehurtig updated this revision to Diff 65169.Jul 22 2016, 3:29 PM
ehurtig edited edge metadata.
ehurtig marked 2 inline comments as done.

Address review feedback

vsk accepted this revision.Jul 25 2016, 11:12 AM
vsk edited edge metadata.

Thank you again for your work on this. It should make it a lot easier to build reporting/analytics tools that work with coverage data.

This LGTM. Let's leave it up for another day to see if anyone has more feedback. I think the CommandGuide change is still missing, but that's a ~1 line change we can make pre-commit.

This revision is now accepted and ready to land.Jul 25 2016, 11:12 AM
This revision was automatically updated to reflect the committed changes.