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

feat(coverage): Demangling C++ & Rust function names in coverage results #398

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ jobs:
- name: Lint
run: npm run check-lint

# Required for the test cases
- name: Install system dependencies
run: sudo apt install -y binutils rustfilt

- run: xvfb-run -a npm test
if: runner.os == 'Linux'

Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ In case you are using the code coverage integration with any other language
bazelbuild/vscode-bazel#367. Please share both positive and negative experiences
you might have.

For C++ and Rust, make sure to have `c++filt` / `rustfilt` installed and
available through the `$PATH`. Otherwise, only mangled, hard-to-decipher
function names will be displayed. For Java, no additional steps are required.

## Contributing

If you would like to contribute to the Bazel Visual Studio extension, please
Expand Down
2 changes: 1 addition & 1 deletion src/bazel/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ async function onTaskProcessEnd(event: vscode.TaskProcessEndEvent) {
);
} else {
// Show the coverage date
showLcovCoverage(
await showLcovCoverage(
description,
workspaceInfo.bazelWorkspacePath,
covFileStr,
Expand Down
4 changes: 2 additions & 2 deletions src/test-explorer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export function activateTesting(): vscode.Disposable[] {
/**
* Display coverage information from a `.lcov` file.
*/
export function showLcovCoverage(
export async function showLcovCoverage(
description: string,
baseFolder: string,
lcov: string,
Expand All @@ -42,7 +42,7 @@ export function showLcovCoverage(
false,
);
run.appendOutput(description.replaceAll("\n", "\r\n"));
for (const c of parseLcov(baseFolder, lcov)) {
for (const c of await parseLcov(baseFolder, lcov)) {
run.addCoverage(c);
}
run.end();
Expand Down
49 changes: 36 additions & 13 deletions src/test-explorer/lcov_parser.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import * as vscode from "vscode";
import * as path from "path";
import * as child_process from "child_process";
import * as which from "which";
import * as util from "util";
import { assert } from "../assert";

const execFile = util.promisify(child_process.execFile);

/**
* Demangle JVM method names.
*
Expand Down Expand Up @@ -109,6 +114,20 @@ function demangleJVMMethodName(mangled: string): string | undefined {
return `${returnType} ${shortClassName}::${functionName}(${argListStr})`;
}

/**
* Demangle a name by calling a filter binary (like c++filt or rustfilt)
*/
async function demangleNameUsingFilter(
execPath: string | null,
mangled: string,
): Promise<string | undefined> {
if (execPath === null) return undefined;
const unmangled = (await execFile(execPath, [mangled])).stdout.trim();
// If unmangling failed, return undefined, so we can fallback to another demangler.
if (!unmangled || unmangled === mangled) return undefined;
return unmangled;
}

/**
* Coverage data from a Bazel run.
*
Expand Down Expand Up @@ -136,10 +155,12 @@ export class BazelFileCoverage extends vscode.FileCoverage {
/**
* Parses the LCOV coverage info into VS Code's representation
*/
export function parseLcov(
export async function parseLcov(
baseFolder: string,
lcov: string,
): BazelFileCoverage[] {
): Promise<BazelFileCoverage[]> {
const cxxFiltPath = await which("c++filt", { nothrow: true });
const rustFiltPath = await which("rustfilt", { nothrow: true });
lcov = lcov.replaceAll("\r\n", "\n");

// Documentation of the lcov format:
Expand Down Expand Up @@ -218,17 +239,19 @@ export function parseLcov(
location = new vscode.Position(startLine, 0);
}
if (!info.functionsByLine.has(startLine)) {
// TODO: Also add demangling for C++ and Rust.
// https://internals.rust-lang.org/t/symbol-mangling-of-rust-vs-c/7222
// https://github.com/rust-lang/rustc-demangle
//
// Tested with:
// * Go -> no function names, only line coverage
// * C++ -> mangled names
// * Java -> mangled names
// * Rust -> mangled names
// Not tested with Python, Swift, Kotlin etc.
const demangled = demangleJVMMethodName(funcName) ?? funcName;
// Demangle the name.
// We must first try rustfilt before trying c++filt.
// The Rust name mangling scheme is intentionally compatible with
// C++ mangling. Hence, c++filt will be succesful on Rust's mangled
// names. But rustfilt provides more readable demanglings, and hence
// we prefer rustfilt over c++filt. For C++ mangled names, rustfilt
// will fail and we will fallback to c++filt.
// See https://internals.rust-lang.org/t/symbol-mangling-of-rust-vs-c/7222
const demangled =
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's a shame that we don't know what language the lcov was generated from here. Is there no reasonable way of getting this info out of Bazel?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The LCOV file from Bazel might even be merged across multiple different languages. E.g., if I run ./bazel coverage //..., Bazel will merge coverage between Java, C++ and Go test cases.

We could use some heuristic by looking at the file extension, but then again: What would we gain by doing so?

What is your underlying concern with trying multiple decoders and using the first one that can interpret the name?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was just concerned that a decoder could incorrectly demangle a symbol from another language. However, I'm not sure how likely this is and if this isn't possible to easily determine the language then I'm happy with this approach.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we should be good here in almost all cases. As soon as we run into it, we will find a better solution 🙂

demangleJVMMethodName(funcName) ??
(await demangleNameUsingFilter(rustFiltPath, funcName)) ??
(await demangleNameUsingFilter(cxxFiltPath, funcName)) ??
funcName;
info.functionsByLine.set(
startLine,
new vscode.DeclarationCoverage(demangled, 0, location),
Expand Down
22 changes: 13 additions & 9 deletions test/lcov_parser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { DeclarationCoverage, StatementCoverage } from "vscode";

const testDir = path.join(__dirname, "../..", "test");

function parseTestLcov(lcov: string): BazelFileCoverage[] {
function parseTestLcov(lcov: string): Promise<BazelFileCoverage[]> {
return parseLcov("/base", lcov);
}

Expand Down Expand Up @@ -50,19 +50,23 @@ function getLineCoverageForLine(
}

describe("The lcov parser", () => {
it("accepts an empty string", () => {
assert.deepEqual(parseTestLcov(""), []);
it("accepts an empty string", async () => {
assert.deepEqual(await parseTestLcov(""), []);
});

it("accepts Linux end-of-lines", () => {
const coveredFiles = parseTestLcov("SF:a.cpp\nFN:1,abc\nend_of_record\n");
it("accepts Linux end-of-lines", async () => {
const coveredFiles = await parseTestLcov(
"SF:a.cpp\nFN:1,abc\nend_of_record\n",
);
assert.equal(coveredFiles.length, 1);
assert.equal(coveredFiles[0].declarationCoverage.total, 1);
});

it("accepts Windows end-of-lines", () => {
it("accepts Windows end-of-lines", async () => {
// \r\n and no final end of line
const coveredFiles = parseTestLcov("SF:a.cpp\r\nFN:1,abc\r\nend_of_record");
const coveredFiles = await parseTestLcov(
"SF:a.cpp\r\nFN:1,abc\r\nend_of_record",
);
assert.equal(coveredFiles.length, 1);
assert.equal(coveredFiles[0].declarationCoverage.total, 1);
});
Expand Down Expand Up @@ -142,7 +146,7 @@ describe("The lcov parser", () => {
it("function coverage details", () => {
const initFunc = getFunctionByLine(fileCov, 71);
assert(initFunc !== undefined);
assert.equal(initFunc.name, "_ZN5blaze10RcFileTest5SetUpEv");
assert.equal(initFunc.name, "blaze::RcFileTest::SetUp()");
assert.equal(initFunc.executed, 34);
});
it("line coverage details", () => {
Expand Down Expand Up @@ -187,7 +191,7 @@ describe("The lcov parser", () => {
assert(consumeFunc !== undefined);
assert.equal(
consumeFunc.name,
"_RNCNvCscQvVXOS7Ja3_5label20consume_package_name0B3_",
"label::consume_package_name::{closure#0}",
);
assert.equal(consumeFunc.executed, 2);
});
Expand Down