Skip to content

Commit

Permalink
fix some issues with toplevel coverage missing after inlining
Browse files Browse the repository at this point in the history
  • Loading branch information
vtjnash committed Feb 23, 2024
1 parent eaa980b commit e4c1486
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 47 deletions.
21 changes: 11 additions & 10 deletions base/compiler/inferencestate.jl
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ mutable struct InferenceState
exc_bestguess = Bottom
ipo_effects = EFFECTS_TOTAL

insert_coverage = should_insert_coverage(mod, src)
insert_coverage = should_insert_coverage(mod, src.debuginfo)
if insert_coverage
ipo_effects = Effects(ipo_effects; effect_free = ALWAYS_FALSE)
end
Expand Down Expand Up @@ -474,20 +474,21 @@ function compute_trycatch(code::Vector{Any}, ip::BitSet)
end

# check if coverage mode is enabled
function should_insert_coverage(mod::Module, src::CodeInfo)
function should_insert_coverage(mod::Module, debuginfo::DebugInfo)
coverage_enabled(mod) && return true
JLOptions().code_coverage == 3 || return false
# path-specific coverage mode: if any line falls in a tracked file enable coverage for all
return should_insert_coverage(src.debuginfo)
return _should_insert_coverage(debuginfo)
end
should_insert_coverage(mod::Symbol) = is_file_tracked(mod)
should_insert_coverage(mod::Method) = should_insert_coverage(mod.file)
should_insert_coverage(mod::MethodInstance) = should_insert_coverage(mod.def)
should_insert_coverage(mod::Module) = false
function should_insert_coverage(info::DebugInfo)

_should_insert_coverage(mod::Symbol) = is_file_tracked(mod)
_should_insert_coverage(mod::Method) = _should_insert_coverage(mod.file)
_should_insert_coverage(mod::MethodInstance) = _should_insert_coverage(mod.def)
_should_insert_coverage(mod::Module) = false
function _should_insert_coverage(info::DebugInfo)
linetable = info.linetable
linetable === nothing || (should_insert_coverage(linetable) && return true)
should_insert_coverage(info.def) && return true
linetable === nothing || (_should_insert_coverage(linetable) && return true)
_should_insert_coverage(info.def) && return true
return false
end

Expand Down
7 changes: 3 additions & 4 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,6 @@ end

function ir_inline_linetable!(debuginfo::DebugInfoStream, inlinee_debuginfo::DebugInfo, inlinee::MethodInstance)
# Append the linetable of the inlined function to our edges table
extra_coverage_line = false
linetable_offset = 1
while true
if linetable_offset > length(debuginfo.edges)
Expand All @@ -313,17 +312,17 @@ function ir_inline_linetable!(debuginfo::DebugInfoStream, inlinee_debuginfo::Deb
end
linetable_offset += 1
end
return Int32(linetable_offset), extra_coverage_line
return Int32(linetable_offset)
end

function ir_prepare_inlining!(insert_node!::Inserter, inline_target::Union{IRCode, IncrementalCompact},
ir::IRCode, di::DebugInfo, mi::MethodInstance, inlined_at::Int32, argexprs::Vector{Any})
def = mi.def::Method
debuginfo = inline_target isa IRCode ? inline_target.debuginfo : inline_target.ir.debuginfo

linetable_offset, extra_coverage_line = ir_inline_linetable!(debuginfo, di, mi)
linetable_offset = ir_inline_linetable!(debuginfo, di, mi)
topline = (inlined_at, linetable_offset, Int32(0))
if extra_coverage_line
if should_insert_coverage(def.module, di)
insert_node!(NewInstruction(Expr(:code_coverage_effect), Nothing, topline))
end
spvals_ssa = nothing
Expand Down
4 changes: 2 additions & 2 deletions base/compiler/ssair/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -370,9 +370,9 @@ function append_scopes!(scopes::Vector{LineInfoNode}, pc::Int, debuginfo, @nospe
codeloc = getdebugidx(debuginfo, pc)
line::Int = codeloc[1]
inl_to::Int = codeloc[2]
doupdate &= line != 0 || inl_to != 0 # disabled debug info--no update
if debuginfo.linetable === nothing || pc <= 0 || line < 0
line < 0 && (line = 0) # broken debug info
doupdate &= line != 0 || inl_to != 0 # disabled debug info--no update
line < 0 && (doupdate = false; line = 0) # broken debug info
push!(scopes, LineInfoNode(def, debuginfo_file1(debuginfo), Int32(line)))
else
doupdate = append_scopes!(scopes, line, debuginfo.linetable::Core.DebugInfo, def) && doupdate
Expand Down
3 changes: 1 addition & 2 deletions doc/src/manual/modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,9 @@ julia> nice(::Cat) = "nice 😸"
ERROR: invalid method definition in Main: function NiceStuff.nice must be explicitly imported to be extended
Stacktrace:
[1] top-level scope
@ :0
@ none:0
[2] top-level scope
@ none:1
```

This error prevents accidentally adding methods to functions in other modules that you only intended to use.
Expand Down
43 changes: 22 additions & 21 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8657,6 +8657,7 @@ static jl_llvm_functions_t
DebugLoc loc;
StringRef file;
ssize_t line;
ssize_t line0; // if this represents pc=1, then also cover the entry to the function (pc=0)
bool is_user_code;
int32_t edgeid;
bool sameframe(const DebugLineTable &other) const {
Expand All @@ -8667,12 +8668,12 @@ static jl_llvm_functions_t
DebugLineTable topinfo;
topinfo.file = ctx.file;
topinfo.line = toplineno;
topinfo.line0 = 0;
topinfo.is_user_code = mod_is_user_mod;
topinfo.loc = topdebugloc;
topinfo.edgeid = 0;
std::map<std::tuple<StringRef, StringRef>, DISubprogram*> subprograms;
SmallVector<DebugLineTable, 0> prev_lineinfo, new_lineinfo;
new_lineinfo.push_back(topinfo);
auto update_lineinfo = [&] (size_t pc) {
std::function<bool(jl_debuginfo_t*, jl_value_t*, size_t, size_t)> append_lineinfo =
[&] (jl_debuginfo_t *debuginfo, jl_value_t *func, size_t to, size_t pc) -> bool {
Expand All @@ -8681,16 +8682,16 @@ static jl_llvm_functions_t
func = debuginfo->def; // this is inlined
struct jl_codeloc_t lineidx = jl_uncompress1_codeloc(debuginfo->codelocs, pc);
size_t i = lineidx.line;
if (pc > 0 && i >= 0 && (jl_value_t*)debuginfo->linetable != jl_nothing) {
if (i < 0) // pc out of range: broken debuginfo?
return false;
if (i == 0 && lineidx.to == 0) // no update
return false;
if (pc > 0 && (jl_value_t*)debuginfo->linetable != jl_nothing) {
// indirection node
if (!append_lineinfo(debuginfo->linetable, func, to, i))
return false; // no update
}
else {
if (i < 0) // pc out of range: broken debuginfo?
return false;
if (i == 0 && lineidx.to == 0) // no update
return false;
// actual node
DebugLineTable info;
info.edgeid = to;
Expand All @@ -8699,6 +8700,13 @@ static jl_llvm_functions_t
modu = ctx.module;
info.file = jl_debuginfo_file1(debuginfo);
info.line = i;
info.line0 = 0;
if (pc == 1) {
struct jl_codeloc_t lineidx = jl_uncompress1_codeloc(debuginfo->codelocs, 0);
assert(lineidx.to == 0 && lineidx.pc == 0);
if (lineidx.line > 0 && info.line != lineidx.line)
info.line0 = lineidx.line;
}
if (info.file.empty())
info.file = "<missing>";
if (modu == ctx.module)
Expand Down Expand Up @@ -8867,8 +8875,11 @@ static jl_llvm_functions_t
for (; dbg < new_lineinfo.size(); dbg++) {
const auto &newdbg = new_lineinfo[dbg];
bool is_tracked = in_tracked_path(newdbg.file);
if (do_coverage(newdbg.is_user_code, is_tracked))
if (do_coverage(newdbg.is_user_code, is_tracked)) {
if (newdbg.line0 != 0 && (dbg >= prev_lineinfo.size() || newdbg.edgeid != prev_lineinfo[dbg].edgeid || newdbg.line0 != prev_lineinfo[dbg].line))
coverageVisitLine(ctx, newdbg.file, newdbg.line0);
coverageVisitLine(ctx, newdbg.file, newdbg.line);
}
}
};
auto mallocVisitStmt = [&] (Value *sync, bool have_dbg_update) {
Expand Down Expand Up @@ -8908,18 +8919,8 @@ static jl_llvm_functions_t
struct jl_codeloc_t lineidx = jl_uncompress1_codeloc(debuginfo->codelocs, pc);
if (lineidx.line == -1)
break;
jl_debuginfo_t *linetable = debuginfo->linetable;
while (lineidx.line >= 0 && (jl_value_t*)linetable != jl_nothing) {
if (lineidx.line == 0) {
lineidx = jl_uncompress1_codeloc(linetable->codelocs, lineidx.line);
break;
}
lineidx = jl_uncompress1_codeloc(linetable->codelocs, lineidx.line);
linetable = linetable->linetable;
}
if (lineidx.line > 0) {
if (lineidx.line > 0)
jl_coverage_alloc_line(file, lineidx.line);
}
}
}
};
Expand Down Expand Up @@ -8975,12 +8976,12 @@ static jl_llvm_functions_t
BB[label] = bb;
}

new_lineinfo.push_back(topinfo);
Value *sync_bytes = nullptr;
if (do_malloc_log(true, mod_is_tracked))
sync_bytes = ctx.builder.CreateCall(prepare_call(diff_gc_total_bytes_func), {});
// coverage for the function definition line number
if (do_coverage(topinfo.is_user_code, mod_is_tracked))
coverageVisitLine(ctx, topinfo.file, topinfo.line);
// coverage for the function definition line number (topinfo)
coverageVisitStmt();

find_next_stmt(0);
while (cursor != -1) {
Expand Down
2 changes: 1 addition & 1 deletion src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -2446,7 +2446,7 @@ jl_code_instance_t *jl_compile_method_internal(jl_method_instance_t *mi, size_t
jl_code_instance_t *codeinst = jl_get_method_inferred(
mi, codeinst2->rettype,
jl_atomic_load_relaxed(&codeinst2->min_world), jl_atomic_load_relaxed(&codeinst2->max_world),
codeinst2->debuginfo);
jl_atomic_load_relaxed(&codeinst2->debuginfo));
if (jl_atomic_load_relaxed(&codeinst->invoke) == NULL) {
codeinst->rettype_const = codeinst2->rettype_const;
jl_gc_wb(codeinst, codeinst->rettype_const);
Expand Down
5 changes: 2 additions & 3 deletions test/cmdlineargs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -560,15 +560,14 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
end
do_test()
"""), """
DA:1,1
DA:2,1
DA:3,1
DA:5,1
DA:6,0
DA:9,1
DA:10,1
LH:6
LF:7
LH:5
LF:6
""")
@test contains(coverage_info_for("""
function cov_bug()
Expand Down
1 change: 0 additions & 1 deletion test/staged.jl
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,6 @@ let world = Base.get_world_counter()
match = Base._which(Tuple{typeof(sin), Int}; world)
mi = Core.Compiler.specialize_method(match)
lwr = Core.Compiler.retrieve_code_info(mi, world)
#lwr = Core.DebugInfo(mi, lwr.debuginfo, Core.svec(), "") # TODO: @ccall jl_compress_debuginfo(UInt32[(ip, 0, 0)... for ip in 1:length(lwr.code)))::Any
nstmts = length(lwr.code)
di = Core.DebugInfo(Core.Compiler.DebugInfoStream(mi, lwr.debuginfo, nstmts), nstmts)
lwr.debuginfo = di
Expand Down
5 changes: 3 additions & 2 deletions test/testhelpers/coverage_file.info
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ DA:11,1
DA:12,1
DA:14,0
DA:17,1
DA:18,1
DA:19,1
DA:20,1
DA:22,1
LH:12
LF:14
LH:13
LF:15
end_of_record
2 changes: 1 addition & 1 deletion test/testhelpers/coverage_file.jl
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ end

success = code_coverage_test() == [1, 2, 3] &&
short_form_func_coverage_test(2) == 4
exit(success ? 0 : 1)
exit(success ? 0 : 1)

# end of file

0 comments on commit e4c1486

Please sign in to comment.