From 4dc1469965b273317886ae1e68c4c7638b8211f0 Mon Sep 17 00:00:00 2001 From: Nicholas Bauer Date: Fri, 21 Oct 2022 17:04:42 -0400 Subject: [PATCH 1/3] Implement storage of MethodInstance in linetables and stacktrace lookup fallbacks --- base/compiler/ssair/inlining.jl | 33 +++++--- base/compiler/ssair/passes.jl | 2 +- base/compiler/typeinfer.jl | 2 +- base/stacktraces.jl | 129 +++++++++++++++++++++++++++++--- src/debuginfo.cpp | 2 +- src/julia.h | 4 +- src/method.c | 11 +-- test/stacktraces.jl | 5 +- 8 files changed, 155 insertions(+), 33 deletions(-) diff --git a/base/compiler/ssair/inlining.jl b/base/compiler/ssair/inlining.jl index 56afbe7423dd8..87466f4f648a8 100644 --- a/base/compiler/ssair/inlining.jl +++ b/base/compiler/ssair/inlining.jl @@ -303,18 +303,32 @@ function finish_cfg_inline!(state::CFGInliningState) end end +# duplicated from IRShow +normalize_method_name(m::Method) = m.name +normalize_method_name(m::MethodInstance) = (m.def::Method).name +normalize_method_name(m::Symbol) = m +normalize_method_name(m) = Symbol("") +@noinline method_name(m::LineInfoNode) = normalize_method_name(m.method) + +inline_node_is_duplicate(topline::LineInfoNode, line::LineInfoNode) = + topline.module === line.module && + method_name(topline) === method_name(line) && + topline.file === line.file && + topline.line === line.line + function ir_inline_linetable!(linetable::Vector{LineInfoNode}, inlinee_ir::IRCode, - inlinee::Method, + inlinee::MethodInstance, inlined_at::Int32) - coverage = coverage_enabled(inlinee.module) + inlinee_def = inlinee.def::Method + coverage = coverage_enabled(inlinee_def.module) linetable_offset::Int32 = length(linetable) # Append the linetable of the inlined function to our line table topline::Int32 = linetable_offset + Int32(1) coverage_by_path = JLOptions().code_coverage == 3 - push!(linetable, LineInfoNode(inlinee.module, inlinee.name, inlinee.file, inlinee.line, inlined_at)) + push!(linetable, LineInfoNode(inlinee_def.module, inlinee, inlinee_def.file, inlinee_def.line, inlined_at)) oldlinetable = inlinee_ir.linetable extra_coverage_line = zero(Int32) - for oldline in 1:length(oldlinetable) + for oldline in eachindex(oldlinetable) entry = oldlinetable[oldline] if !coverage && coverage_by_path && is_file_tracked(entry.file) # include topline coverage entry if in path-specific coverage mode, and any file falls under path @@ -324,7 +338,7 @@ function ir_inline_linetable!(linetable::Vector{LineInfoNode}, inlinee_ir::IRCod (entry.inlined_at > 0 ? entry.inlined_at + linetable_offset + (oldline == 1) : inlined_at)) if oldline == 1 # check for a duplicate on the first iteration (likely true) - if newentry === linetable[topline] + if inline_node_is_duplicate(linetable[topline], newentry) continue else linetable_offset += 1 @@ -340,9 +354,10 @@ end function ir_prepare_inlining!(insert_node!::Inserter, inline_target::Union{IRCode, IncrementalCompact}, linetable::Vector{LineInfoNode}, ir′::IRCode, sparam_vals::SimpleVector, - def::Method, inlined_at::Int32, argexprs::Vector{Any}) + mi::MethodInstance, inlined_at::Int32, argexprs::Vector{Any}) + def = mi.def::Method topline::Int32 = length(linetable) + Int32(1) - linetable_offset, extra_coverage_line = ir_inline_linetable!(linetable, ir′, def, inlined_at) + linetable_offset, extra_coverage_line = ir_inline_linetable!(linetable, ir′, mi, inlined_at) if extra_coverage_line != 0 insert_node!(NewInstruction(Expr(:code_coverage_effect), Nothing, extra_coverage_line)) end @@ -372,11 +387,10 @@ function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector boundscheck::Symbol, todo_bbs::Vector{Tuple{Int, Int}}) # Ok, do the inlining here sparam_vals = item.mi.sparam_vals - def = item.mi.def::Method inlined_at = compact.result[idx][:line] ((sp_ssa, argexprs), linetable_offset) = ir_prepare_inlining!(InsertHere(compact), - compact, linetable, item.ir, sparam_vals, def, inlined_at, argexprs) + compact, linetable, item.ir, sparam_vals, item.mi, inlined_at, argexprs) if boundscheck === :default || boundscheck === :propagate if (compact.result[idx][:flag] & IR_FLAG_INBOUNDS) != 0 @@ -386,6 +400,7 @@ function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector # If the iterator already moved on to the next basic block, # temporarily re-open in again. local return_value + def = item.mi.def::Method sig = def.sig # Special case inlining that maintains the current basic block if there's only one BB in the target new_new_offset = length(compact.new_new_nodes) diff --git a/base/compiler/ssair/passes.jl b/base/compiler/ssair/passes.jl index 8b9251f9e369b..5e9cfd99cc1d1 100644 --- a/base/compiler/ssair/passes.jl +++ b/base/compiler/ssair/passes.jl @@ -1090,7 +1090,7 @@ function try_inline_finalizer!(ir::IRCode, argexprs::Vector{Any}, idx::Int, # TOOD: Should there be a special line number node for inlined finalizers? inlined_at = ir[SSAValue(idx)][:line] ((sp_ssa, argexprs), linetable_offset) = ir_prepare_inlining!(InsertBefore(ir, SSAValue(idx)), ir, - ir.linetable, src, mi.sparam_vals, mi.def, inlined_at, argexprs) + ir.linetable, src, mi.sparam_vals, mi, inlined_at, argexprs) # TODO: Use the actual inliner here rather than open coding this special purpose inliner. spvals = mi.sparam_vals diff --git a/base/compiler/typeinfer.jl b/base/compiler/typeinfer.jl index 82183cb594444..7384a6b147a89 100644 --- a/base/compiler/typeinfer.jl +++ b/base/compiler/typeinfer.jl @@ -1040,7 +1040,7 @@ function typeinf_ext(interp::AbstractInterpreter, mi::MethodInstance) tree.slotflags = fill(IR_FLAG_NULL, nargs) tree.ssavaluetypes = 1 tree.codelocs = Int32[1] - tree.linetable = [LineInfoNode(method.module, method.name, method.file, method.line, Int32(0))] + tree.linetable = [LineInfoNode(method.module, mi, method.file, method.line, Int32(0))] tree.inferred = true tree.ssaflags = UInt8[0] tree.pure = true diff --git a/base/stacktraces.jl b/base/stacktraces.jl index 3cb81d82bd3f7..829bb50f91c42 100644 --- a/base/stacktraces.jl +++ b/base/stacktraces.jl @@ -52,8 +52,9 @@ struct StackFrame # this type should be kept platform-agnostic so that profiles file::Symbol "the line number in the file containing the execution context" line::Int - "the MethodInstance or CodeInfo containing the execution context (if it could be found)" - linfo::Union{MethodInstance, CodeInfo, Nothing} + "the MethodInstance or CodeInfo containing the execution context (if it could be found), \ + or Module (for macro expansions)" + linfo::Union{MethodInstance, Method, Module, CodeInfo, Nothing} "true if the code is from C" from_c::Bool "true if the code is from an inlined frame" @@ -95,6 +96,86 @@ function hash(frame::StackFrame, h::UInt) return h end +get_inlinetable(::Any) = nothing +function get_inlinetable(mi::MethodInstance) + isdefined(mi, :def) && mi.def isa Method && isdefined(mi, :cache) && isdefined(mi.cache, :inferred) && + mi.cache.inferred !== nothing || return nothing + linetable = ccall(:jl_uncompress_ir, Any, (Any, Any, Any), mi.def, mi.cache, mi.cache.inferred).linetable + return filter!(x -> x.inlined_at > 0, linetable) +end + +get_method_instance_roots(::Any) = nothing +function get_method_instance_roots(mi::Union{Method, MethodInstance}) + m = mi isa MethodInstance ? mi.def : mi + m isa Method && isdefined(m, :roots) || return nothing + return filter(x -> x isa MethodInstance, m.roots) +end + +function lookup_inline_frame_info(func::Symbol, file::Symbol, linenum::Int, inlinetable::Vector{Core.LineInfoNode}) + #REPL frames and some base files lack this prefix while others have it; should fix? + filestripped = Symbol(lstrip(string(file), ('.', '\\', '/'))) + linfo = nothing + #= + Some matching entries contain the MethodInstance directly. + Other matching entries contain only a Method or Symbol (function name); such entries + are located after the entry with the MethodInstance, so backtracking is required. + If backtracking fails, the Method or Module is stored for return, but we continue + the search in case a MethodInstance is found later. + TODO: If a backtrack has failed, do we need to backtrack again later if another Method + or Symbol match is found? Or can a limit on the subsequent backtracks be placed? + =# + for (i, line) ∈ enumerate(inlinetable) + Base.IRShow.method_name(line) == func && line.file ∈ (file, filestripped) && line.line == linenum || continue + if line.method isa MethodInstance + linfo = line.method + break + elseif line.method isa Method || line.method isa Symbol + linfo = line.method isa Method ? line.method : line.module + # backtrack to find the matching MethodInstance, if possible + for j ∈ (i - 1):-1:1 + nextline = inlinetable[j] + nextline.inlined_at == line.inlined_at && Base.IRShow.method_name(line) == Base.IRShow.method_name(nextline) && line.file == nextline.file || break + if nextline.method isa MethodInstance + linfo = nextline.method + break + end + end + end + end + return linfo +end + +function lookup_inline_frame_info(func::Symbol, file::Symbol, miroots::Vector{Any}) + # REPL frames and some base files lack this prefix while others have it; should fix? + filestripped = Symbol(lstrip(string(file), ('.', '\\', '/'))) + matches = filter(miroots) do x + x.def isa Method || return false + m = x.def::Method + return m.name == func && m.file ∈ (file, filestripped) + end + if length(matches) > 1 + # ambiguous, check if method is same and return that instead + all_matched = true + for m ∈ matches + all_matched = m.def.line == matches[1].def.line && + m.def.module == matches[1].def.module + all_matched || break + end + if all_matched + return matches[1].def + end + # all else fails, return module if they match, or give up + all_matched = true + for m ∈ matches + all_matched = m.def.module == matches[1].def.module + all_matched || break + end + return all_matched ? matches[1].def.module : nothing + elseif length(matches) == 1 + return matches[1] + end + return nothing +end """ lookup(pointer::Ptr{Cvoid}) -> Vector{StackFrame} @@ -107,11 +188,26 @@ Base.@constprop :none function lookup(pointer::Ptr{Cvoid}) infos = ccall(:jl_lookup_code_address, Any, (Ptr{Cvoid}, Cint), pointer, false)::Core.SimpleVector pointer = convert(UInt64, pointer) isempty(infos) && return [StackFrame(empty_sym, empty_sym, -1, nothing, true, false, pointer)] # this is equal to UNKNOWN + parent_linfo = infos[end][4] + inlinetable = get_inlinetable(parent_linfo) + miroots = inlinetable === nothing ? get_method_instance_roots(parent_linfo) : nothing # fallback if linetable missing res = Vector{StackFrame}(undef, length(infos)) - for i in 1:length(infos) + for i ∈ reverse(1:length(infos)) info = infos[i]::Core.SimpleVector @assert(length(info) == 6) - res[i] = StackFrame(info[1]::Symbol, info[2]::Symbol, info[3]::Int, info[4], info[5]::Bool, info[6]::Bool, pointer) + func = info[1]::Symbol + file = info[2]::Symbol + linenum = info[3]::Int + linfo = info[4] + if i < length(infos) + if inlinetable !== nothing + linfo = lookup_inline_frame_info(func, file, linenum, inlinetable) + elseif miroots !== nothing + linfo = lookup_inline_frame_info(func, file, miroots) + end + linfo = linfo === nothing ? parentmodule(res[i + 1]) : linfo # e.g. `macro expansion` + end + res[i] = StackFrame(func, file, linenum, linfo, info[5]::Bool, info[6]::Bool, pointer) end return res end @@ -219,10 +315,17 @@ function show_spec_linfo(io::IO, frame::StackFrame) else Base.print_within_stacktrace(io, Base.demangle_function_name(string(frame.func)), bold=true) end - elseif linfo isa MethodInstance - def = linfo.def - if isa(def, Method) - sig = linfo.specTypes + elseif linfo isa CodeInfo + print(io, "top-level scope") + elseif linfo isa Module + Base.print_within_stacktrace(io, Base.demangle_function_name(string(frame.func)), bold=true) + else + def, sig = if linfo isa MethodInstance + linfo.def, linfo.specTypes + else + linfo, linfo.sig + end + if def isa Method argnames = Base.method_argnames(def) if def.nkw > 0 # rearrange call kw_impl(kw_args..., func, pos_args...) to func(pos_args...) @@ -246,8 +349,6 @@ function show_spec_linfo(io::IO, frame::StackFrame) else Base.show_mi(io, linfo, true) end - elseif linfo isa CodeInfo - print(io, "top-level scope") end end @@ -277,9 +378,13 @@ function Base.parentmodule(frame::StackFrame) else return (def::Method).module end + elseif linfo isa Method + return linfo.module + elseif linfo isa Module + return linfo else - # The module is not always available (common reasons include inlined - # frames and frames arising from the interpreter) + # The module is not always available (common reasons include + # frames arising from the interpreter) nothing end end diff --git a/src/debuginfo.cpp b/src/debuginfo.cpp index fe5614100f9e3..1464edd02a568 100644 --- a/src/debuginfo.cpp +++ b/src/debuginfo.cpp @@ -495,7 +495,7 @@ static int lookup_pointer( std::size_t semi_pos = func_name.find(';'); if (semi_pos != std::string::npos) { func_name = func_name.substr(0, semi_pos); - frame->linfo = NULL; // TODO: if (new_frames[n_frames - 1].linfo) frame->linfo = lookup(func_name in linfo)? + frame->linfo = NULL; // Looked up on Julia side } } } diff --git a/src/julia.h b/src/julia.h index 8bb3d4deac839..7c910e290a4d6 100644 --- a/src/julia.h +++ b/src/julia.h @@ -228,7 +228,7 @@ typedef struct _jl_method_instance_t jl_method_instance_t; typedef struct _jl_line_info_node_t { struct _jl_module_t *module; - jl_value_t *method; + jl_value_t *method; // may contain a jl_symbol, jl_method_t, or jl_method_instance_t jl_sym_t *file; int32_t line; int32_t inlined_at; @@ -393,7 +393,7 @@ typedef struct _jl_code_instance_t { // inference state cache jl_value_t *rettype; // return type for fptr jl_value_t *rettype_const; // inferred constant return value, or null - _Atomic(jl_value_t *) inferred; // inferred jl_code_info_t, or jl_nothing, or null + _Atomic(jl_value_t *) inferred; // inferred jl_code_info_t (may be compressed), or jl_nothing, or null //TODO: jl_array_t *edges; // stored information about edges from this object //TODO: uint8_t absolute_max; // whether true max world is unknown diff --git a/src/method.c b/src/method.c index ec49fdf32a193..057f9272004b9 100644 --- a/src/method.c +++ b/src/method.c @@ -491,8 +491,9 @@ jl_code_info_t *jl_new_code_info_from_ir(jl_expr_t *ir) return src; } -void jl_add_function_name_to_lineinfo(jl_code_info_t *ci, jl_value_t *name) +void jl_add_function_to_lineinfo(jl_code_info_t *ci, jl_value_t *func) { + // func may contain jl_symbol (function name), jl_method_t, or jl_method_instance_t jl_array_t *li = (jl_array_t*)ci->linetable; size_t i, n = jl_array_len(li); jl_value_t *rt = NULL, *lno = NULL, *inl = NULL; @@ -504,8 +505,8 @@ void jl_add_function_name_to_lineinfo(jl_code_info_t *ci, jl_value_t *name) jl_value_t *file = jl_fieldref_noalloc(ln, 2); lno = jl_fieldref(ln, 3); inl = jl_fieldref(ln, 4); - jl_value_t *ln_name = (jl_is_int32(inl) && jl_unbox_int32(inl) == 0) ? name : jl_fieldref_noalloc(ln, 1); - rt = jl_new_struct(jl_lineinfonode_type, mod, ln_name, file, lno, inl); + jl_value_t *ln_func = (jl_is_int32(inl) && jl_unbox_int32(inl) == 0) ? func : jl_fieldref_noalloc(ln, 1); + rt = jl_new_struct(jl_lineinfonode_type, mod, ln_func, file, lno, inl); jl_array_ptr_set(li, i, rt); } JL_GC_POP(); @@ -611,7 +612,7 @@ JL_DLLEXPORT jl_code_info_t *jl_code_for_staged(jl_method_instance_t *linfo) ct->ptls->in_pure_callback = last_in; jl_lineno = last_lineno; ct->world_age = last_age; - jl_add_function_name_to_lineinfo(func, (jl_value_t*)def->name); + jl_add_function_to_lineinfo(func, (jl_value_t*)def->name); } JL_CATCH { ct->ptls->in_pure_callback = last_in; @@ -666,7 +667,7 @@ static void jl_method_set_source(jl_method_t *m, jl_code_info_t *src) m->pure = src->pure; m->constprop = src->constprop; m->purity.bits = src->purity.bits; - jl_add_function_name_to_lineinfo(src, (jl_value_t*)m->name); + jl_add_function_to_lineinfo(src, (jl_value_t*)m->name); jl_array_t *copy = NULL; jl_svec_t *sparam_vars = jl_outer_unionall_vars(m->sig); diff --git a/test/stacktraces.jl b/test/stacktraces.jl index cbb07a60e456b..5cad3e1500e8c 100644 --- a/test/stacktraces.jl +++ b/test/stacktraces.jl @@ -91,8 +91,9 @@ trace = (try; f(3); catch; stacktrace(catch_backtrace()); end)[1:3] can_inline = Bool(Base.JLOptions().can_inline) for (frame, func, inlined) in zip(trace, [g,h,f], (can_inline, can_inline, false)) @test frame.func === typeof(func).name.mt.name - #@test get(frame.linfo).def === which(func, (Any,)).func - #@test get(frame.linfo).specTypes === Tuple{typeof(func), Int} + @test frame.linfo.def.module === which(func, (Any,)).module + @test frame.linfo.def === which(func, (Any,)) + @test frame.linfo.specTypes === Tuple{typeof(func), Int} # line @test frame.file === Symbol(@__FILE__) @test !frame.from_c From f2965215edee4779060cd24417f3d154753bc909 Mon Sep 17 00:00:00 2001 From: Nicholas Bauer Date: Wed, 18 Jan 2023 21:38:18 -0500 Subject: [PATCH 2/3] Switch to `in` for loops for style --- base/stacktraces.jl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/base/stacktraces.jl b/base/stacktraces.jl index 2f95f9447e034..8c2072902c57a 100644 --- a/base/stacktraces.jl +++ b/base/stacktraces.jl @@ -124,7 +124,7 @@ function lookup_inline_frame_info(func::Symbol, file::Symbol, linenum::Int, inli TODO: If a backtrack has failed, do we need to backtrack again later if another Method or Symbol match is found? Or can a limit on the subsequent backtracks be placed? =# - for (i, line) ∈ enumerate(inlinetable) + for (i, line) in enumerate(inlinetable) Base.IRShow.method_name(line) == func && line.file ∈ (file, filestripped) && line.line == linenum || continue if line.method isa MethodInstance linfo = line.method @@ -132,7 +132,7 @@ function lookup_inline_frame_info(func::Symbol, file::Symbol, linenum::Int, inli elseif line.method isa Method || line.method isa Symbol linfo = line.method isa Method ? line.method : line.module # backtrack to find the matching MethodInstance, if possible - for j ∈ (i - 1):-1:1 + for j in (i - 1):-1:1 nextline = inlinetable[j] nextline.inlined_at == line.inlined_at && Base.IRShow.method_name(line) == Base.IRShow.method_name(nextline) && line.file == nextline.file || break if nextline.method isa MethodInstance @@ -156,7 +156,7 @@ function lookup_inline_frame_info(func::Symbol, file::Symbol, miroots::Vector{An if length(matches) > 1 # ambiguous, check if method is same and return that instead all_matched = true - for m ∈ matches + for m in matches all_matched = m.def.line == matches[1].def.line && m.def.module == matches[1].def.module all_matched || break @@ -166,7 +166,7 @@ function lookup_inline_frame_info(func::Symbol, file::Symbol, miroots::Vector{An end # all else fails, return module if they match, or give up all_matched = true - for m ∈ matches + for m in matches all_matched = m.def.module == matches[1].def.module all_matched || break end @@ -192,7 +192,7 @@ Base.@constprop :none function lookup(pointer::Ptr{Cvoid}) inlinetable = get_inlinetable(parent_linfo) miroots = inlinetable === nothing ? get_method_instance_roots(parent_linfo) : nothing # fallback if linetable missing res = Vector{StackFrame}(undef, length(infos)) - for i ∈ reverse(1:length(infos)) + for i in reverse(1:length(infos)) info = infos[i]::Core.SimpleVector @assert(length(info) == 6) func = info[1]::Symbol From f7cb357e563d84833720fc36376cfb052b51a887 Mon Sep 17 00:00:00 2001 From: Nicholas Bauer Date: Wed, 18 Jan 2023 21:39:16 -0500 Subject: [PATCH 3/3] Remove space --- src/debuginfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/debuginfo.cpp b/src/debuginfo.cpp index 9498bc13997c2..a6737fdbda5bb 100644 --- a/src/debuginfo.cpp +++ b/src/debuginfo.cpp @@ -503,7 +503,7 @@ static int lookup_pointer( std::size_t semi_pos = func_name.find(';'); if (semi_pos != std::string::npos) { func_name = func_name.substr(0, semi_pos); - frame->linfo = NULL; // Looked up on Julia side + frame->linfo = NULL; // Looked up on Julia side } } }