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

Coverage of #[no_mangle] functions in cdylib is missed #365

Open
pl4nf0ur opened this issue Apr 23, 2024 · 5 comments
Open

Coverage of #[no_mangle] functions in cdylib is missed #365

pl4nf0ur opened this issue Apr 23, 2024 · 5 comments
Labels
C-question Category: A question C-upstream-bug Category: This is a bug of compiler or dependencies (the fix may require action in the upstream)

Comments

@pl4nf0ur
Copy link

Is it possible to get coverage report for dynamic libraries? My use case is that I'm building a sqlite extension which I compile into a shared library using:

# Cargo.toml
[lib]
crate-type = ["cdylib"]

I already have a few tests where I test the library from within sqlite by making it load the compiled artifact as an extension. I just tried running cargo llvm-cov on those tests but the resulting coverage is completely empty.

@taiki-e
Copy link
Owner

taiki-e commented Apr 23, 2024

cdylib itself should be supported since cargo-llvm-cov works with PyO3 and proc macros (#304 #307)

If it does not work, it is generally the case that the required environment variables are not propagated properly (#320 (comment)), or you are exiting in an unsupported way (#235), etc. That said, if the problem occurs only on Windows in particular, it may be a bug on the cargo-llvm-cov side regarding Windows-specific path handling.

In any case, this issue needs a reproduction. It is always difficult to investigate an issue unless I can reproduce it on my end.

@taiki-e taiki-e added C-question Category: A question needs-mcve Call for participation: This issue needs a Minimal Complete and Verifiable Example S-needs-repro Status: This issue has no reproduction and needs a reproduction to make progress. and removed needs-mcve Call for participation: This issue needs a Minimal Complete and Verifiable Example C-question Category: A question labels Apr 23, 2024
@pl4nf0ur
Copy link
Author

This is the smallest example I could create to reproduce the issue.

Steps:

  1. create a new library crate;

  2. set its Cargo.toml to:

[package]
name = "foo"
version = "0.1.0"
edition = "2021"

[lib]
crate-type = ["cdylib"]

[dependencies]
sqlite_nostd = { git = "https://github.com/vlcn-io/sqlite-rs-embedded/", rev = "7a0dd3a", features = ["loadable_extension"] }

[dev-dependencies]
miniserde = "0.1"
rusqlite = { version = "0.31", features = ["load_extension"] }
  1. set its lib.rs to:
#![allow(clippy::missing_safety_doc)]

use core::ffi::{c_char, c_int};

use sqlite_nostd::bindings::*;
use sqlite_nostd::*;

#[no_mangle]
pub unsafe fn sqlite3_foo_init(
    db: *mut sqlite3,
    _err_msg: *mut *mut c_char,
    api: *mut api_routines,
) -> c_int {
    EXTENSION_INIT2(api);

    db.create_function_v2(
        "foo",
        0,
        SQLITE_UTF8 | SQLITE_DIRECTONLY,
        None,
        Some(foo),
        None,
        None,
        None,
    )
    .err()
    .unwrap_or(ResultCode::OK) as c_int
}

#[no_mangle]
pub unsafe extern "C" fn foo(ctx: *mut context, _: c_int, _: *mut *mut value) {
    ctx.result_text_transient("foo");
}

#[cfg(test)]
mod tests {
    use std::env;
    use std::path::PathBuf;
    use std::process::Command;

    use miniserde::json;
    use rusqlite::Connection;

    fn target_dir() -> PathBuf {
        let output = Command::new("cargo")
            .arg("metadata")
            .arg("--format-version=1")
            .output()
            .expect("Failed to execute `cargo metadata`");

        let metadata_str = std::str::from_utf8(&output.stdout).unwrap();

        let metadata = json::from_str::<json::Object>(metadata_str).unwrap();

        match metadata.get("target_directory") {
            Some(json::Value::String(target_dir)) => PathBuf::from(target_dir),
            _ => unreachable!(),
        }
    }

    fn library_name() -> String {
        format!(
            "{prefix}{crate_name}{suffix}",
            prefix = env::consts::DLL_PREFIX,
            crate_name = env!("CARGO_CRATE_NAME"),
            suffix = env::consts::DLL_SUFFIX,
        )
    }

    fn library_path() -> PathBuf {
        target_dir().join("debug").join(library_name())
    }

    fn connection() -> Connection {
        let connection = Connection::open_in_memory().unwrap();
        unsafe { connection.load_extension_enable() }.unwrap();
        unsafe { connection.load_extension(library_path(), None) }.unwrap();
        connection.load_extension_disable().unwrap();
        connection
    }

    #[test]
    fn foo() {
        let connection = connection();

        let result = connection
            .query_row("SELECT foo()", [], |row| row.get::<_, String>(0))
            .unwrap();

        assert_eq!(result, "foo");
    }
}
  1. cargo b;

  2. cargo llvm-cov --html;

I would expect the coverage to include both sqlite3_foo_init and foo, but they're both reported as "uncovered".

Screenshot 2024-04-24 at 16 38 55

@pl4nf0ur
Copy link
Author

pl4nf0ur commented May 1, 2024

Were you able to reproduce the issue?

@taiki-e
Copy link
Owner

taiki-e commented May 1, 2024

Thanks for the repro.

4. cargo b;

The first problems is that the libfoo.so built in this step is not instrumented. (In other words, the environment variables necessary to make cdylib instrumentable for coverage are not applied.)

The commands needed to get coverage correctly in complex cases involving multiple cargo commands are documented here and in your case would be:

source <(cargo llvm-cov show-env --export-prefix)
cargo llvm-cov clean --workspace
cargo build
cargo test
cargo llvm-cov report --html 

The second problem is that your function uses #[no_mangle]. It seems that due to some kind of rustc bug, coverage of #[no_mangle] is not being measured (this problem does not exist when calling #[no_mangle] functions directly within the same crate).

Separate the main processes from the #[no_mangle] function and we can see them being executed:

 #[no_mangle]
 pub unsafe fn sqlite3_foo_init(
+    db: *mut sqlite3,
+    err_msg: *mut *mut c_char,
+    api: *mut api_routines,
+) -> c_int {
+    _sqlite3_foo_init(db, err_msg, api)
+}
+
+unsafe fn _sqlite3_foo_init(
     db: *mut sqlite3,
     _err_msg: *mut *mut c_char,
after

@taiki-e taiki-e added C-upstream-bug Category: This is a bug of compiler or dependencies (the fix may require action in the upstream) and removed S-needs-repro Status: This issue has no reproduction and needs a reproduction to make progress. labels May 1, 2024
@taiki-e taiki-e changed the title Support for cdylib crates Coverage of #[no_mangle] functions in cdylib is missed May 1, 2024
@pl4nf0ur
Copy link
Author

pl4nf0ur commented May 1, 2024

Oh sorry, I read that section of the README but didn't realize it was relevant for my case.

I just run those commands and it's now working properly, however I noticed that:

  1. I get a warning when running cargo-llvm-cov:
$ cargo llvm-cov report --html
warning: 1 functions have mismatched data

I'm not sure what that means.

  1. In the html report, there are several instances of Unexecuted instantiation, like the following:
Screenshot 2024-05-01 at 20 39 48

Again, I'm not sure how to interpret this since foo_inner is reported as covered.

Lastly, is there a way to also get coverage if a test directly runs sqlite from the cli, like so?

use std::process::Command;

#[test] 
fn test_compiled_library_from_sqlite() {
    Command::new("sqlite3")
    //  ... execute functions from the `foo` extension by loading `libfoo.so`
}

I tried doing this but the coverage is blank, which I'm assuming is due to sqlite not being compiled by cargo, even though libfoo.so is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-question Category: A question C-upstream-bug Category: This is a bug of compiler or dependencies (the fix may require action in the upstream)
Projects
None yet
Development

No branches or pull requests

2 participants