rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.96k stars 12.69k forks source link

-Zinstrument-coverage producing coverage report which claims line is never run #84180

Closed alex closed 3 years ago

alex commented 3 years ago

I'm attempting to use -Zinstrument-coverage to measure coverage in a Python extension module (using pyo3). This is mostly working well. Except... there's a line that's getting recorded as not being executed, even though it is.

Exact steps for reproducing this can be seen in https://github.com/pyca/cryptography/pull/5970. Specifically it's claimed that this line isn't covered: https://github.com/pyca/cryptography/blob/main/src/rust/src/asn1.rs#L40

That is line 40, a chained method call.

(I realize this is a somewhat involved example (there's Python! and numerous crates!), but I'm also not sure how to minimize it further. If there's any intermediate outputs I can produce, just ask!))

Looking at the raw llvm-cov show output, I see the following:

  | _RNvNtCslsOXpOCSGQA_17cryptography_rust4asn128___pyo3_raw_parse_tls_feature:
  |   36|      4|#[pyo3::prelude::pyfunction]
  ------------------
   37|      4|fn parse_tls_feature(py: pyo3::Python<'_>, data: &[u8]) -> pyo3::PyResult<pyo3::PyObject> {
   38|      4|    let tls_feature_type_to_enum = py
   39|      4|        .import("cryptography.x509.extensions")?
                                                             ^0
   40|      0|        .getattr("_TLS_FEATURE_TYPE_TO_ENUM")?;
   41|       |
   42|      4|    let features = asn1::parse::<_, PyAsn1Error, _>(data, |p| {
   43|      4|        let features = pyo3::types::PyList::empty(py);
   44|      5|        for el in p.read_element::<asn1::SequenceOf<u64>>()? {
                                ^4                                       ^0
   45|      5|            let feature = el?;
                                          ^0
   46|      5|            let py_feature = tls_feature_type_to_enum.get_item(feature.to_object(py))?;
                                                                                                   ^0
   47|      5|            features.append(py_feature)?;
                                                     ^0
   48|       |        }
   49|      4|        Ok(features)
   50|      4|    })?;
                    ^0
   51|       |
   52|      4|    let x509_module = py.import("cryptography.x509")?;
                                                                  ^0
   53|      4|    x509_module
   54|      4|        .call1("TLSFeature", (features,))
   55|      4|        .map(|o| o.to_object(py))
   56|      4|}
   57|       |

This confirms the same thing, that coverage thinks line 40 isn't being run. But we can tell it is, Line 42 is being run, and we can't get to line 42 without going through line 40 :-)

Meta

rustc --version --verbose:

rustc 1.53.0-nightly (07e0e2ec2 2021-03-24)
binary: rustc
commit-hash: 07e0e2ec268c140e607e1ac7f49f145612d0f597
commit-date: 2021-03-24
host: x86_64-unknown-linux-gnu
release: 1.53.0-nightly
LLVM version: 12.0.0
klensy commented 3 years ago

Try to check with fresh nightly (there was some merges that touch coverage-related things), toolstate was fixed few days ago.

alex commented 3 years ago

Still reproduced at:

rustc 1.53.0-nightly (132b4e5d1 2021-04-13)
binary: rustc
commit-hash: 132b4e5d167b7e622fcc11fa2b67b931105b4de1
commit-date: 2021-04-13
host: x86_64-unknown-linux-gnu
release: 1.53.0-nightly
LLVM version: 12.0.0
alex commented 3 years ago

Ok, I minimized this, which makes me think it's a span issue. Here's the code:

pub struct Foo(u32);

impl Foo {
    pub fn foo(&self) -> Result<&Foo, ()> {
        Ok(self)
    }

    pub fn bar(&self) -> Result<u32, ()> {
        Ok(self.0)
    }
}

pub fn bar(f: &Foo) -> Result<u32, ()> {
    let x = f
        .foo()?
        .bar()?;

    Ok(x + 3)
}

#[cfg(test)]
mod tests {
    use super::{bar, Foo};

    #[test]
    fn test_it() {
        let f = Foo(5);
        let result = bar(&f);
        assert_eq!(result, Ok(8))
    }
}

And here's the coverage results (look to line 16 specifically, though line 29 is also weird to me):

    1|      1|pub struct Foo(u32);pub struct Foo(u32);                                                                                                                                                                             
    2|       |                                                                                                                                                                                                                     
    3|       |impl Foo {                                                                                                                                                                                                           
    4|      1|    pub fn foo(&self) -> Result<&Foo, ()> {                                                                                                                                                                          
    5|      1|        Ok(self)                                                                                                                                                                                                     
    6|      1|    }                                                                                                                                                                                                                
    7|       |                                                                                                                                                                                                                     
    8|      1|    pub fn bar(&self) -> Result<u32, ()> {                                                                                                                                                                           
    9|      1|        Ok(self.0)                                                                                                                                                                                                   
   10|      1|    }                                                                                                                                                                                                                
   11|       |}                                                                                                                                                                                                                    
   12|       |                                                                                                                                                                                                                     
   13|      1|pub fn bar(f: &Foo) -> Result<u32, ()> {                                                                                                                                                                             
   14|      1|    let x = f                                                                                                                                                                                                        
   15|      1|          .foo()?                                                                                                                                                                                                    
   16|      0|          .bar()?;                                                                                                                                                                                                   
   17|       |                                                                                                                                                                                                                     
   18|      1|    Ok(x + 3)                                                                                                                                                                                                        
   19|      1|}                                                                                                                                                                                                                    
   20|       |                                                                                                                                                                                                                     
   21|       |#[cfg(test)]                                                                                                                                                                                                         
   22|       |mod tests {                                                                                                                                                                                                          
   23|       |    use super::{bar, Foo};                                                                                                                                                                                           
   24|       |                                                                                                                                                                                                                     
   25|       |    #[test]                                                                                                                                                                                                          
   26|      1|    fn test_it() {                                                                                                                                                                                                   
  ------------------                                                                                                                                                                                                               
  | _RNCNvNtCsfrdjj0EOOuS_8kangaroo5tests7test_it0B5_:                                                                                                                                                                             
  |   26|      1|    fn test_it() {                                                                                                                                                                                                
  ------------------                                                                                                                                                                                                               
   27|      1|        let f = Foo(5);                                                                                                                                                                                              
   28|      1|        let result = bar(&f);                                                                                                                                                                                        
   29|      0|        assert_eq!(result, Ok(8))                                                                                                                                                                                    
   30|      1|    }                                                                                                                                                                                                                
  ------------------                                                                                                                                                                                                               
  | _RNvNtCsfrdjj0EOOuS_8kangaroo5testss_7test_it:                                                                                                                                                                                 
  |   26|      1|    fn test_it() {
  |   27|      1|        let f = Foo(5);
  |   28|      1|        let result = bar(&f);
  |   29|      0|        assert_eq!(result, Ok(8))
  |   30|      1|    }
  ------------------
   31|       |}

And for convenience, the script I used to drive coverage:

#!/bin/sh -ex

env RUSTFLAGS="-Zinstrument-coverage" cargo +nightly test
llvm-profdata merge -sparse default.profraw -o default.profdata
llvm-cov show ./target/debug/deps/kangaroo-f715d112c2e2f815 -instr-profile=default.profdata --ignore-filename-regex='/.cargo/registry' --ignore-filename-regex='/.rustup/toolchains/'
alex commented 3 years ago

I guess one important note that's not visible in the text version is that the ? on line 15 and 16 are red, which I believe means uncovered. So it seems like llvm-cov has been told that that span is uncovered, but still doesn't believe the rest of the line was ever executed? Here's a screenshot for visibility:

image