From d28c29443610f627cbafcab1ddf7007a02934b9a Mon Sep 17 00:00:00 2001 From: Adrian Vogelsgesang Date: Tue, 11 Jun 2024 20:44:41 +0200 Subject: [PATCH] feat(coverage): Demangling C++ & Rust function names in coverage results (#398) feat(coverage): Demangling C++ & Rust function names in coverage results Demangle C++ and Rust symbols using `c++filt` and `rustfilt`. We rely on those tools to be installed on the $PATH. I was also considering whether I should compile `c++filt` and / or `rustfilt` to WebAssembly, but decided against it, given that this would make the build much more complicated and it is currently unclear to me whether we will stick with the "traditional" tool chains (npm, webpack, cargo, ...) or if we will pivot to using Bazel. --- .github/workflows/build.yml | 4 +++ README.md | 4 +++ src/bazel/tasks.ts | 2 +- src/test-explorer/index.ts | 4 +-- src/test-explorer/lcov_parser.ts | 49 +++++++++++++++++++++++--------- test/lcov_parser.test.ts | 22 ++++++++------ 6 files changed, 60 insertions(+), 25 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f765eaae..f24f275b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -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' diff --git a/README.md b/README.md index 7742f896..ac86d5f2 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/src/bazel/tasks.ts b/src/bazel/tasks.ts index b64bbed9..2a6c1e32 100644 --- a/src/bazel/tasks.ts +++ b/src/bazel/tasks.ts @@ -184,7 +184,7 @@ async function onTaskProcessEnd(event: vscode.TaskProcessEndEvent) { ); } else { // Show the coverage date - showLcovCoverage( + await showLcovCoverage( description, workspaceInfo.bazelWorkspacePath, covFileStr, diff --git a/src/test-explorer/index.ts b/src/test-explorer/index.ts index f3dfddcf..031a075c 100644 --- a/src/test-explorer/index.ts +++ b/src/test-explorer/index.ts @@ -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, @@ -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(); diff --git a/src/test-explorer/lcov_parser.ts b/src/test-explorer/lcov_parser.ts index 617cd9a7..8c7afa7c 100644 --- a/src/test-explorer/lcov_parser.ts +++ b/src/test-explorer/lcov_parser.ts @@ -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. * @@ -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 { + 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. * @@ -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 { + 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: @@ -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 = + demangleJVMMethodName(funcName) ?? + (await demangleNameUsingFilter(rustFiltPath, funcName)) ?? + (await demangleNameUsingFilter(cxxFiltPath, funcName)) ?? + funcName; info.functionsByLine.set( startLine, new vscode.DeclarationCoverage(demangled, 0, location), diff --git a/test/lcov_parser.test.ts b/test/lcov_parser.test.ts index 2ffba97b..65f45b21 100644 --- a/test/lcov_parser.test.ts +++ b/test/lcov_parser.test.ts @@ -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 { return parseLcov("/base", lcov); } @@ -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); }); @@ -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", () => { @@ -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); });