JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.73k stars 5.48k forks source link

Requesting code coverage for a specific path misses lines #44593

Closed IanButterworth closed 2 years ago

IanButterworth commented 2 years ago

The option to track code coverage for files in a given path (https://github.com/JuliaLang/julia/pull/44359) appears to miss a significant number of lines compared to --code-coverage=user.

As seen in this Octavian CI run on julia nightly https://github.com/JuliaLinearAlgebra/Octavian.jl/pull/143

Screen Shot 2022-03-12 at 6 31 44 PM Screen Shot 2022-03-12 at 6 36 37 PM
IanButterworth commented 2 years ago

I'm failing to find anywhere that the code introduced in https://github.com/JuliaLang/julia/pull/44359 differs for user vs. path mode in a way that would cause this bug.

Things I can confirm from studying the coverage tests, where path mode does miss some lines:

  1. The missing lines are not missing because they're being skipped in coverageVisitLine. I don't see any skipped there https://github.com/JuliaLang/julia/blob/98b4b069c0560c9a358f30a43737345e67562e69/src/codegen.cpp#L2048

  2. The 3 missing lines aren't being skipped by any of the do_coverage checks

It seems like the skipping/missing is happening earlier


Another datapoint. All of the missing lines went through this branch in user mode https://github.com/JuliaLang/julia/blob/98b4b069c0560c9a358f30a43737345e67562e69/src/codegen.cpp#L7159


Another: the first nlocs value is 3 less here in path mode https://github.com/JuliaLang/julia/blob/98b4b069c0560c9a358f30a43737345e67562e69/src/codegen.cpp#L6989 and the same values further up via jl_array_len(src->linetable) https://github.com/JuliaLang/julia/blob/98b4b069c0560c9a358f30a43737345e67562e69/src/codegen.cpp#L6344

user path
23 20
1 1
1 1
2 2
1 1
35 35
1 1
2 2
73 73
21 21
7 7
12 12
IanButterworth commented 2 years ago

@c42f I wondered whether you have any advice here given it looks like you've fixed similar issues before?

My debugging is based on https://github.com/IanButterworth/julia/tree/ib/cov_debug and running Base.runtests("cmdlineargs") which gives the following (annotated)

julia> Base.runtests("cmdlineargs")
Test    (Worker) | Time (s) | GC (s) | GC % | Alloc (MB) | RSS (MB)
cmdlineargs  (1) |        started at 2022-03-13T14:04:25.446
[ Info: ================ user ================
## other file visits omitted
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:3 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:4 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:5 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:7 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:8 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:9 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:10 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:11 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:12 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:14 0 0
## other file visits omitted
VISITED1 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:3 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:4 0 0
## other file visits omitted
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:5 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:7 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:8 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:9 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:10 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:11 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:12 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:10 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:17 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:18 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:1234 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:19 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:22 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:19 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:20 0 0
VISITED1 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:17 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:19 0 0 # missing below
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:22 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:19 0 0 # missing below
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:20 0 0 # missing below
[ Info: ================ dir ================
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:3 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:4 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:5 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:7 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:8 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:9 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:10 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:11 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:12 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:14 1 1
VISITED1 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:3 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:4 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:5 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:7 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:8 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:9 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:10 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:12 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:10 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:17 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:18 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:1234 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:19 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:22 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:19 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:20 1 1
VISITED1 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:17 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:22 1 1
cmdlineargs  (1) |         failed at 2022-03-13T14:04:27.582
IanButterworth commented 2 years ago

My current theory is that https://github.com/JuliaLang/julia/pull/44359 didn't introduce the bug, but that the bug existed for user etc. given that the coverage tests have been against "bad" files, and the good references are @test_broken's https://github.com/JuliaLang/julia/blob/98b4b069c0560c9a358f30a43737345e67562e69/test/cmdlineargs.jl#L294

Perhaps it's something like an indexing issue, and because there's fewer lines visited here, more of the lines of interest are missed.

c42f commented 2 years ago

@c42f I wondered whether you have any advice here given it looks like you've fixed similar issues before?

Hmm, I can't remember facing this. Do you have a particular prior issue in mind?

IanButterworth commented 2 years ago

@c42f I thought that https://github.com/JuliaLang/julia/pull/35138 was relevant, but perhaps not

IanButterworth commented 2 years ago

AFAICT none of the code in codegen.cpp modifies src.linetable yet selecting different codecov options is causing different linetable sizes. See the last section in https://github.com/JuliaLang/julia/issues/44593#issuecomment-1066036600

I can't find anywhere outside of codegen.cpp where the jl_options.code_coverage influences anything, so I'm stuck.

@vtjnash does anything jump out to you here?

vtjnash commented 2 years ago

inference calls coverage_enabled(m::Module) to decide if it should emit coverage information for a statement

IanButterworth commented 2 years ago

Ah! yes, that's it https://github.com/JuliaLang/julia/pull/44625 Thanks!