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

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

Closed
alex opened this issue Apr 14, 2021 · 4 comments · Fixed by #84529
Closed

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

alex opened this issue Apr 14, 2021 · 4 comments · Fixed by #84529
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) C-bug Category: This is a bug.

Comments

@alex
Copy link
Member

alex commented Apr 14, 2021

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 pyca/cryptography#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
@alex alex added the C-bug Category: This is a bug. label Apr 14, 2021
@klensy
Copy link
Contributor

klensy commented Apr 14, 2021

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

@alex
Copy link
Member Author

alex commented Apr 14, 2021

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
Copy link
Member Author

alex commented Apr 14, 2021

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
Copy link
Member Author

alex commented Apr 14, 2021

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

@wesleywiser wesleywiser added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label Apr 16, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this issue Apr 28, 2021
Improve coverage spans for chained function calls

Fixes: rust-lang#84180

For chained function calls separated by the `?` try operator, the
function call following the try operator produced a MIR `Call` span that
matched the span of the first call. The `?` try operator started a new
span, so the second call got no span.

It turns out the MIR `Call` terminator has a `func` `Operand`
for the `Constant` representing the function name, and the function
name's Span can be used to reset the starting position of the span.

r? `@tmandry`
cc: `@wesleywiser`
@bors bors closed this as completed in 41667e8 Apr 28, 2021
alex added a commit to pyca/cryptography that referenced this issue May 7, 2021
this was written this way to work around rust-lang/rust#84180, which is fixed
reaperhulk pushed a commit to pyca/cryptography that referenced this issue May 7, 2021
this was written this way to work around rust-lang/rust#84180, which is fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants