-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Lookup metadata for inlined frames for stack traces #41099
Changes from all commits
4dc1469
2cd3645
150d98a
1f5fbc9
f296521
f7cb357
a40cdc9
00659e3
617ed96
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately that only retrieves the code inside the Method's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The runtime usually tries hard to not preserve the information you expect to be here (with an exception made that it is preserved only in the system image). The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you clarify what you mean by reliable/correct? In this code, if two possible MethodInstances are found, it falls back on the Method; am I missing another place where it finds just the first when there may be others? |
||
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) 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 | ||
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 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 | ||
linfo = nextline.method | ||
break | ||
BioTurboNick marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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 in 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 in 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 in 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) | ||
argnames = replace(argnames, :var"#unused#" => :var"") | ||
if def.nkw > 0 | ||
|
@@ -247,8 +350,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 | ||
|
||
|
@@ -273,10 +374,18 @@ function Base.parentmodule(frame::StackFrame) | |
linfo = frame.linfo | ||
if linfo isa MethodInstance | ||
def = linfo.def | ||
return def isa Module ? def : parentmodule(def::Method) | ||
if def isa Module | ||
return def | ||
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 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it really necessary to allow
linfo::Module
? From what I can tell, this module information is never used or explicitly tested, so there's no need to store it here. I'm concerned that this heavy-union field definition complicates the code unnecessarily.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible it's out of date, but at least at one point it was needed to look up the module for
macro expansion
frame.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should have implemented test cases for these new features. Do you have any use case for that information? Otherwise we should clean that up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. But I just described the use case. Is this causing a problem somewhere right now? Otherwise something to look at when the underlying problem #50204 is resolved someway and the full functionality can be restored here.