Merge pull request #34254 from JuliaLang/jn/codecov-accuracy

fix codecov accuracy for optimized and dead statements

base/compiler/abstractinterpretation.jl

@@ -1222,7 +1222,8 @@ function typeinf_local(frame::InferenceState)
                     if isa(fname, Slot)
                         changes = StateUpdate(fname, VarState(Any, false), changes)
                     end
-                elseif hd === :inbounds || hd === :meta || hd === :loopinfo
+                elseif hd === :inbounds || hd === :meta || hd === :loopinfo || hd == :code_coverage_effect
+                    # these do not generate code
                 else
                     t = abstract_eval(stmt, changes, frame)
                     t === Bottom && break

base/compiler/optimize.jl

@@ -198,7 +198,7 @@ function optimize(opt::OptimizationState, @nospecialize(result))
             opt.src.pure = true
         end
 
-        if proven_pure && !coverage_enabled()
+        if proven_pure
             # use constant calling convention
             # Do not emit `jl_fptr_const_return` if coverage is enabled
             # so that we don't need to add coverage support
@@ -408,14 +408,19 @@ function renumber_ir_elements!(body::Vector{Any}, ssachangemap::Vector{Int}, lab
             ssachangemap[i] += ssachangemap[i - 1]
         end
     end
-    (labelchangemap[end] != 0 && ssachangemap[end] != 0) || return
+    if labelchangemap[end] == 0 && ssachangemap[end] == 0
+        return
+    end
     for i = 1:length(body)
         el = body[i]
         if isa(el, GotoNode)
             body[i] = GotoNode(el.label + labelchangemap[el.label])
         elseif isa(el, SSAValue)
             body[i] = SSAValue(el.id + ssachangemap[el.id])
         elseif isa(el, Expr)
+            if el.head === :(=) && el.args[2] isa Expr
+                el = el.args[2]::Expr
+            end
             if el.head === :gotoifnot
                 cond = el.args[1]
                 if isa(cond, SSAValue)
@@ -427,9 +432,6 @@ function renumber_ir_elements!(body::Vector{Any}, ssachangemap::Vector{Int}, lab
                 tgt = el.args[1]::Int
                 el.args[1] = tgt + labelchangemap[tgt]
             elseif !is_meta_expr_head(el.head)
-                if el.head === :(=) && el.args[2] isa Expr && !is_meta_expr_head(el.args[2].head)
-                    el = el.args[2]::Expr
-                end
                 args = el.args
                 for i = 1:length(args)
                     el = args[i]

base/compiler/ssair/driver.jl

@@ -49,28 +49,45 @@ function normalize(@nospecialize(stmt), meta::Vector{Any})
     return stmt
 end
 
-function just_construct_ssa(ci::CodeInfo, code::Vector{Any}, nargs::Int, sv::OptimizationState)
+function convert_to_ircode(ci::CodeInfo, code::Vector{Any}, coverage::Bool, nargs::Int, sv::OptimizationState)
     # Go through and add an unreachable node after every
     # Union{} call. Then reindex labels.
     idx = 1
     oldidx = 1
     changemap = fill(0, length(code))
+    labelmap = coverage ? fill(0, length(code)) : changemap
+    prevloc = zero(eltype(ci.codelocs))
     while idx <= length(code)
+        codeloc = ci.codelocs[idx]
+        if coverage && codeloc != prevloc && codeloc != 0
+            # insert a side-effect instruction before the current instruction in the same basic block
+            insert!(code, idx, Expr(:code_coverage_marker))
+            insert!(ci.codelocs, idx, codeloc)
+            insert!(ci.ssavaluetypes, idx, Nothing)
+            changemap[oldidx] += 1
+            if oldidx < length(labelmap)
+                labelmap[oldidx + 1] += 1
+            end
+            idx += 1
+            prevloc = codeloc
+        end
         if code[idx] isa Expr && ci.ssavaluetypes[idx] === Union{}
-            if !(idx < length(code) && isexpr(code[idx+1], :unreachable))
+            if !(idx < length(code) && isexpr(code[idx + 1], :unreachable))
+                # insert unreachable in the same basic block after the current instruction (splitting it)
                 insert!(code, idx + 1, ReturnNode())
                 insert!(ci.codelocs, idx + 1, ci.codelocs[idx])
                 insert!(ci.ssavaluetypes, idx + 1, Union{})
                 if oldidx < length(changemap)
-                    changemap[oldidx + 1] = 1
+                    changemap[oldidx + 1] += 1
+                    coverage && (labelmap[oldidx + 1] += 1)
                 end
                 idx += 1
             end
         end
         idx += 1
         oldidx += 1
     end
-    renumber_ir_elements!(code, changemap)
+    renumber_ir_elements!(code, changemap, labelmap)
 
     inbounds_depth = 0 # Number of stacked inbounds
     meta = Any[]
@@ -99,17 +116,22 @@ function just_construct_ssa(ci::CodeInfo, code::Vector{Any}, nargs::Int, sv::Opt
     end
     strip_trailing_junk!(ci, code, flags)
     cfg = compute_basic_blocks(code)
-    defuse_insts = scan_slot_def_use(nargs, ci, code)
-    @timeit "domtree 1" domtree = construct_domtree(cfg)
-    ir = let code = Any[nothing for _ = 1:length(code)]
-            IRCode(code, Any[], ci.codelocs, flags, cfg, collect(LineInfoNode, ci.linetable), sv.slottypes, meta, sv.sptypes)
-        end
-    @timeit "construct_ssa" ir = construct_ssa!(ci, code, ir, domtree, defuse_insts, nargs, sv.sptypes, sv.slottypes)
+    ir = IRCode(code, Any[], ci.codelocs, flags, cfg, collect(LineInfoNode, ci.linetable), sv.slottypes, meta, sv.sptypes)
+    return ir
+end
+
+function slot2reg(ir::IRCode, ci::CodeInfo, nargs::Int, sv::OptimizationState)
+    # need `ci` for the slot metadata, IR for the code
+    @timeit "domtree 1" domtree = construct_domtree(ir.cfg)
+    defuse_insts = scan_slot_def_use(nargs, ci, ir.stmts)
+    @timeit "construct_ssa" ir = construct_ssa!(ci, ir, domtree, defuse_insts, nargs, sv.sptypes, sv.slottypes) # consumes `ir`
     return ir
 end
 
 function run_passes(ci::CodeInfo, nargs::Int, sv::OptimizationState)
-    ir = just_construct_ssa(ci, copy_exprargs(ci.code), nargs, sv)
+    preserve_coverage = coverage_enabled(sv.mod)
+    ir = convert_to_ircode(ci, copy_exprargs(ci.code), preserve_coverage, nargs, sv)
+    ir = slot2reg(ir, ci, nargs, sv)
     #@Base.show ("after_construct", ir)
     # TODO: Domsorting can produce an updated domtree - no need to recompute here
     @timeit "compact 1" ir = compact!(ir)

base/compiler/ssair/slot2ssa.jl

@@ -569,8 +569,9 @@ function recompute_type(node::Union{PhiNode, PhiCNode}, ci::CodeInfo, ir::IRCode
     return new_typ
 end
 
-function construct_ssa!(ci::CodeInfo, code::Vector{Any}, ir::IRCode, domtree::DomTree, defuse, nargs::Int, sptypes::Vector{Any},
+function construct_ssa!(ci::CodeInfo, ir::IRCode, domtree::DomTree, defuse, nargs::Int, sptypes::Vector{Any},
                         slottypes::Vector{Any})
+    code = ir.stmts
     cfg = ir.cfg
     left = Int[]
     catch_entry_blocks = Tuple{Int, Int}[]

base/compiler/utilities.jl

@@ -227,8 +227,22 @@ end
 # options #
 ###########
 
+is_root_module(m::Module) = false
+
 inlining_enabled() = (JLOptions().can_inline == 1)
-coverage_enabled() = (JLOptions().code_coverage != 0)
+function coverage_enabled(m::Module)
+    ccall(:jl_generating_output, Cint, ()) == 0 || return false # don't alter caches
+    cov = JLOptions().code_coverage
+    if cov == 1
+        m = moduleroot(m)
+        m === Core && return false
+        isdefined(Main, :Base) && m === Main.Base && return false
+        return true
+    elseif cov == 2
+        return true
+    end
+    return false
+end
 function inbounds_option()
     opt_check_bounds = JLOptions().check_bounds
     opt_check_bounds == 0 && return :default

base/compiler/validation.jl

@@ -26,6 +26,7 @@ const VALID_EXPR_HEADS = IdDict{Any,Any}(
     :foreigncall => 5:typemax(Int), # name, RT, AT, nreq, cconv, args..., roots...
     :cfunction => 5:5,
     :isdefined => 1:1,
+    :code_coverage_effect => 0:0,
     :loopinfo => 0:typemax(Int),
     :gc_preserve_begin => 0:typemax(Int),
     :gc_preserve_end => 0:typemax(Int),
@@ -144,7 +145,7 @@ function validate_code!(errors::Vector{>:InvalidCodeError}, c::CodeInfo, is_top_
                 head === :const || head === :enter || head === :leave || head === :pop_exception ||
                 head === :method || head === :global || head === :static_parameter ||
                 head === :new || head === :splatnew || head === :thunk || head === :loopinfo ||
-                head === :throw_undef_if_not || head === :unreachable
+                head === :throw_undef_if_not || head === :unreachable || head === :code_coverage_effect
                 validate_val!(x)
             else
                 # TODO: nothing is actually in statement position anymore

base/reflection.jl

@@ -42,7 +42,7 @@ function moduleroot(m::Module)
     while true
         is_root_module(m) && return m
         p = parentmodule(m)
-        p == m && return m
+        p === m && return m
         m = p
     end
 end

src/ast.c

@@ -60,7 +60,7 @@ jl_sym_t *nospecialize_sym; jl_sym_t *macrocall_sym;
 jl_sym_t *colon_sym; jl_sym_t *hygienicscope_sym;
 jl_sym_t *throw_undef_if_not_sym; jl_sym_t *getfield_undefref_sym;
 jl_sym_t *gc_preserve_begin_sym; jl_sym_t *gc_preserve_end_sym;
-jl_sym_t *escape_sym;
+jl_sym_t *coverageeffect_sym; jl_sym_t *escape_sym;
 jl_sym_t *aliasscope_sym; jl_sym_t *popaliasscope_sym;
 
 static uint8_t flisp_system_image[] = {
@@ -365,6 +365,7 @@ void jl_init_frontend(void)
     throw_undef_if_not_sym = jl_symbol("throw_undef_if_not");
     getfield_undefref_sym = jl_symbol("##getfield##");
     do_sym = jl_symbol("do");
+    coverageeffect_sym = jl_symbol("code_coverage_marker");
     aliasscope_sym = jl_symbol("aliasscope");
     popaliasscope_sym = jl_symbol("popaliasscope");
 }

src/codegen.cpp

@@ -1750,7 +1750,7 @@ const int logdata_blocksize = 32; // target getting nearby lines in the same gen
 typedef uint64_t logdata_block[logdata_blocksize];
 typedef StringMap< std::vector<logdata_block*> > logdata_t;
 
-static void visitLine(jl_codectx_t &ctx, std::vector<logdata_block*> &vec, int line, Value *addend, const char* name)
+static uint64_t *allocLine(std::vector<logdata_block*> &vec, int line)
 {
     unsigned block = line / logdata_blocksize;
     line = line % logdata_blocksize;
@@ -1762,8 +1762,14 @@ static void visitLine(jl_codectx_t &ctx, std::vector<logdata_block*> &vec, int l
     logdata_block &data = *vec[block];
     if (data[line] == 0)
         data[line] = 1;
+    return &data[line];
+}
+
+static void visitLine(jl_codectx_t &ctx, std::vector<logdata_block*> &vec, int line, Value *addend, const char* name)
+{
+    uint64_t *ptr = allocLine(vec, line);
     Value *pv = ConstantExpr::getIntToPtr(
-        ConstantInt::get(T_size, (uintptr_t)&data[line]),
+        ConstantInt::get(T_size, (uintptr_t)ptr),
         T_pint64);
     Value *v = ctx.builder.CreateLoad(pv, true, name);
     v = ctx.builder.CreateAdd(v, addend);
@@ -1783,6 +1789,14 @@ static void coverageVisitLine(jl_codectx_t &ctx, StringRef filename, int line)
     visitLine(ctx, coverageData[filename], line, ConstantInt::get(T_int64, 1), "lcnt");
 }
 
+static void coverageAllocLine(StringRef filename, int line)
+{
+    assert(!imaging_mode);
+    if (filename == "" || filename == "none" || filename == "no file" || filename == "<missing>" || line < 0)
+        return;
+    allocLine(coverageData[filename], line);
+}
+
 // Memory allocation log (malloc_log)
 
 static logdata_t mallocData;
@@ -4012,7 +4026,8 @@ static void emit_stmtpos(jl_codectx_t &ctx, jl_value_t *expr, int ssaval_result)
     jl_expr_t *ex = (jl_expr_t*)expr;
     jl_value_t **args = (jl_value_t**)jl_array_data(ex->args);
     jl_sym_t *head = ex->head;
-    if (head == meta_sym || head == inbounds_sym || head == aliasscope_sym || head == popaliasscope_sym) {
+    if (head == meta_sym || head == inbounds_sym || head == coverageeffect_sym
+            || head == aliasscope_sym || head == popaliasscope_sym) {
         // some expression types are metadata and can be ignored
         // in statement position
         return;
@@ -4293,26 +4308,10 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
         I->setMetadata("julia.loopinfo", MD);
         return jl_cgval_t();
     }
-    else if (head == goto_ifnot_sym) {
-        jl_error("Expr(:goto_ifnot) in value position");
-    }
-    else if (head == leave_sym) {
-        jl_error("Expr(:leave) in value position");
-    }
-    else if (head == pop_exception_sym) {
-        jl_error("Expr(:pop_exception) in value position");
-    }
-    else if (head == enter_sym) {
-        jl_error("Expr(:enter) in value position");
-    }
-    else if (head == inbounds_sym) {
-        jl_error("Expr(:inbounds) in value position");
-    }
-    else if (head == aliasscope_sym) {
-        jl_error("Expr(:aliasscope) in value position");
-    }
-    else if (head == popaliasscope_sym) {
-        jl_error("Expr(:popaliasscope) in value position");
+    else if (head == goto_ifnot_sym || head == leave_sym || head == coverageeffect_sym
+            || head == pop_exception_sym || head == enter_sym || head == inbounds_sym
+            || head == aliasscope_sym || head == popaliasscope_sym) {
+        jl_errorf("Expr(:%s) in value position", jl_symbol_name(head));
     }
     else if (head == boundscheck_sym) {
         return mark_julia_const(bounds_check_enabled(ctx, jl_true) ? jl_true : jl_false);
@@ -6416,6 +6415,12 @@ static std::unique_ptr<Module> emit_function(
             dbg = linetable.at(dbg).inlined_at;
         mallocVisitLine(ctx, ctx.file, linetable.at(dbg).line);
     };
+    if (coverage_mode != JL_LOG_NONE) {
+        // record all lines that could be covered
+        for (const auto &info : linetable)
+            if (do_coverage(info.is_user_code))
+                coverageAllocLine(info.file, info.line);
+    }
 
     come_from_bb[0] = ctx.builder.GetInsertBlock();
 
@@ -6470,8 +6475,15 @@ static std::unique_ptr<Module> emit_function(
         BB[label] = bb;
     }
 
-    if (do_coverage(mod_is_user_mod))
+    if (do_coverage(mod_is_user_mod)) {
         coverageVisitLine(ctx, ctx.file, toplineno);
+        if (linetable.size() >= 1) {
+            // avoid double-counting the entry line
+            const auto &info = linetable.at(1);
+            if (info.file == ctx.file && info.line == toplineno && info.is_user_code == mod_is_user_mod)
+                current_lineinfo.push_back(1);
+        }
+    }
     if (do_malloc_log(mod_is_user_mod))
         mallocVisitLine(ctx, ctx.file, toplineno);
     find_next_stmt(0);

src/interpreter.c

@@ -543,7 +543,7 @@ static jl_value_t *eval_value(jl_value_t *e, interpreter_state *s)
     else if (head == boundscheck_sym) {
         return jl_true;
     }
-    else if (head == meta_sym || head == inbounds_sym || head == loopinfo_sym) {
+    else if (head == meta_sym || head == coverageeffect_sym || head == inbounds_sym || head == loopinfo_sym) {
         return jl_nothing;
     }
     else if (head == gc_preserve_begin_sym || head == gc_preserve_end_sym) {

src/julia_internal.h

@@ -1111,7 +1111,7 @@ extern jl_sym_t *colon_sym; extern jl_sym_t *hygienicscope_sym;
 extern jl_sym_t *throw_undef_if_not_sym; extern jl_sym_t *getfield_undefref_sym;
 extern jl_sym_t *gc_preserve_begin_sym; extern jl_sym_t *gc_preserve_end_sym;
 extern jl_sym_t *failed_sym; extern jl_sym_t *done_sym; extern jl_sym_t *runnable_sym;
-extern jl_sym_t *escape_sym;
+extern jl_sym_t *coverageeffect_sym; extern jl_sym_t *escape_sym;
 
 struct _jl_sysimg_fptrs_t;
 

src/method.c

@@ -36,7 +36,8 @@ static jl_value_t *resolve_globals(jl_value_t *expr, jl_module_t *module, jl_sve
             jl_toplevel_eval_flex(module, expr, 0, 1);
             expr = jl_nothing;
         }
-        if (jl_is_toplevel_only_expr(expr) || e->head == const_sym || e->head == copyast_sym ||
+        if (jl_is_toplevel_only_expr(expr) || e->head == const_sym ||
+            e->head == coverageeffect_sym || e->head == copyast_sym ||
             e->head == quote_sym || e->head == inert_sym ||
             e->head == meta_sym || e->head == inbounds_sym ||
             e->head == boundscheck_sym || e->head == loopinfo_sym ||

test/cmdlineargs.jl

@@ -232,7 +232,7 @@ let exename = `$(Base.julia_cmd()) --startup-file=no`
             "<FILENAME>" => realpath(inputfile))
         covfile = replace(joinpath(dir, "coverage.info"), "%" => "%%")
         @test !isfile(covfile)
-        defaultcov = readchomp(`$exename -E "Bool(Base.JLOptions().code_coverage)" -L $inputfile`)
+        defaultcov = readchomp(`$exename -E "Base.JLOptions().code_coverage != 0" -L $inputfile`)
         opts = Base.JLOptions()
         coverage_file = (opts.output_code_coverage != C_NULL) ?  unsafe_string(opts.output_code_coverage) : ""
         @test !isfile(covfile)
@@ -244,30 +244,29 @@ let exename = `$(Base.julia_cmd()) --startup-file=no`
             --code-coverage=$covfile --code-coverage`) == "1"
         @test isfile(covfile)
         got = read(covfile, String)
-        @test occursin(expected, got) || got
         rm(covfile)
+        @test occursin(expected, got) || (expected, got)
         @test readchomp(`$exename -E "Base.JLOptions().code_coverage" -L $inputfile
             --code-coverage=$covfile --code-coverage=user`) == "1"
         @test isfile(covfile)
         got = read(covfile, String)
-        @test occursin(expected, got) || got
         rm(covfile)
+        @test occursin(expected, got) || (expected, got)
         @test readchomp(`$exename -E "Base.JLOptions().code_coverage" -L $inputfile
             --code-coverage=$covfile --code-coverage=all`) == "2"
         @test isfile(covfile)
         got = read(covfile, String)
-        @test occursin(expected, got) || got
         rm(covfile)
+        @test occursin(expected, got) || (expected, got)
     end
 
     # --track-allocation
-    @test readchomp(`$exename -E "Bool(Base.JLOptions().malloc_log)"`) == "false"
-    @test readchomp(`$exename -E "Bool(Base.JLOptions().malloc_log)"
-        --track-allocation=none`) == "false"
+    @test readchomp(`$exename -E "Base.JLOptions().malloc_log != 0"`) == "false"
+    @test readchomp(`$exename -E "Base.JLOptions().malloc_log != 0" --track-allocation=none`) == "false"
 
-    @test readchomp(`$exename -E "Bool(Base.JLOptions().malloc_log)"
+    @test readchomp(`$exename -E "Base.JLOptions().malloc_log != 0"
         --track-allocation`) == "true"
-    @test readchomp(`$exename -E "Bool(Base.JLOptions().malloc_log)"
+    @test readchomp(`$exename -E "Base.JLOptions().malloc_log != 0"
         --track-allocation=user`) == "true"
 
     # --optimize