Skip to content

Commit

Permalink
Switch debug registry mapping to CodeInstance (#56878)
Browse files Browse the repository at this point in the history
Currently our debugging code maps from pointers to `MethodInstance`.
However, I think it makes more sense to map to `CodeInstance` instead,
since that's what we're actually compiling and as we're starting to make
more use of external owners and custom specializations, we'll want to be
able to annotate that in backtraces. This only adjusts the internal data
structures - any actual printing changes for those sorts of use cases
will have to come separately.
  • Loading branch information
Keno authored Dec 22, 2024
1 parent 5ee67b8 commit cc4122c
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 56 deletions.
12 changes: 5 additions & 7 deletions base/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -915,9 +915,9 @@ function _collapse_repeated_frames(trace)
[3] g(x::Int64) <-- useless
@ Main ./REPL[1]:1
=#
if frame.linfo isa MethodInstance && last_frame.linfo isa MethodInstance &&
frame.linfo.def isa Method && last_frame.linfo.def isa Method
m, last_m = frame.linfo.def::Method, last_frame.linfo.def::Method
m, last_m = StackTraces.frame_method_or_module(frame),
StackTraces.frame_method_or_module(last_frame)
if m isa Method && last_m isa Method
params, last_params = Base.unwrap_unionall(m.sig).parameters, Base.unwrap_unionall(last_m.sig).parameters
if last_m.nkw != 0
pos_sig_params = last_params[(last_m.nkw+2):end]
Expand All @@ -944,7 +944,6 @@ function _collapse_repeated_frames(trace)
return trace[kept_frames]
end


function process_backtrace(t::Vector, limit::Int=typemax(Int); skipC = true)
n = 0
last_frame = StackTraces.UNKNOWN
Expand All @@ -965,9 +964,8 @@ function process_backtrace(t::Vector, limit::Int=typemax(Int); skipC = true)
if (lkup.from_c && skipC)
continue
end
code = lkup.linfo
if code isa MethodInstance
def = code.def
if lkup.linfo isa Union{MethodInstance, CodeInstance}
def = StackTraces.frame_method_or_module(lkup)
if def isa Method && def.name !== :kwcall && def.sig <: Tuple{typeof(Core.kwcall),NamedTuple,Any,Vararg}
# hide kwcall() methods, which are probably internal keyword sorter methods
# (we print the internal method instead, after demangling
Expand Down
71 changes: 53 additions & 18 deletions base/stacktraces.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module StackTraces


import Base: hash, ==, show
import Core: CodeInfo, MethodInstance
import Core: CodeInfo, MethodInstance, CodeInstance
using Base.IRShow: normalize_method_name, append_scopes!, LineInfoNode

export StackTrace, StackFrame, stacktrace
Expand All @@ -21,9 +21,9 @@ Stack information representing execution context, with the following fields:
The name of the function containing the execution context.
- `linfo::Union{Method, Core.MethodInstance, Core.CodeInfo, Nothing}`
- `linfo::Union{Method, Core.MethodInstance, Core.CodeInstance, Core.CodeInfo, Nothing}`
The Method, MethodInstance, or CodeInfo containing the execution context (if it could be found), \
The Method, MethodInstance, CodeInstance, or CodeInfo containing the execution context (if it could be found), \
or nothing (for example, if the inlining was a result of macro expansion).
- `file::Symbol`
Expand Down Expand Up @@ -54,9 +54,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), \
"the CodeInstance or CodeInfo containing the execution context (if it could be found), \
or nothing (for example, if the inlining was a result of macro expansion)."
linfo::Union{MethodInstance, Method, CodeInfo, Nothing}
linfo::Union{Core.MethodInstance, Core.CodeInstance, Method, CodeInfo, Nothing}
"true if the code is from C"
from_c::Bool
"true if the code is from an inlined frame"
Expand Down Expand Up @@ -137,16 +137,26 @@ function lookup(ip::Base.InterpreterIP)
line = meth.line
codeinfo = meth.source
else
func = top_level_scope_sym
file = empty_sym
line = Int32(0)
if code isa Core.CodeInstance
codeinfo = code.inferred::CodeInfo
def = code.def
if isa(def, Core.ABIOverride)
def = def.def
end
if isa(def, MethodInstance) && isa(def.def, Method)
meth = def.def
func = meth.name
file = meth.file
line = meth.line
end
else
codeinfo = code::CodeInfo
end
func = top_level_scope_sym
file = empty_sym
line = Int32(0)
end
def = (code isa MethodInstance ? code : StackTraces) # Module just used as a token for top-level code
def = (code isa CodeInfo ? StackTraces : code) # Module just used as a token for top-level code
pc::Int = max(ip.stmt + 1, 0) # n.b. ip.stmt is 0-indexed
scopes = LineInfoNode[]
append_scopes!(scopes, pc, codeinfo.debuginfo, def)
Expand All @@ -157,7 +167,7 @@ function lookup(ip::Base.InterpreterIP)
scopes = map(scopes) do lno
if inlined
def = lno.method
def isa Union{Method,MethodInstance} || (def = nothing)
def isa Union{Method,Core.CodeInstance,MethodInstance} || (def = nothing)
else
def = codeinfo
end
Expand Down Expand Up @@ -227,6 +237,23 @@ end

is_top_level_frame(f::StackFrame) = f.linfo isa CodeInfo || (f.linfo === nothing && f.func === top_level_scope_sym)

function frame_method_or_module(lkup::StackFrame)
code = lkup.linfo
code isa Method && return code
code isa Module && return code
mi = frame_mi(lkup)
mi isa MethodInstance || return nothing
return mi.def
end

function frame_mi(lkup::StackFrame)
code = lkup.linfo
code isa Core.CodeInstance && (code = code.def)
code isa Core.ABIOverride && (code = code.def)
code isa MethodInstance || return nothing
return code
end

function show_spec_linfo(io::IO, frame::StackFrame)
linfo = frame.linfo
if linfo === nothing
Expand All @@ -241,16 +268,18 @@ function show_spec_linfo(io::IO, frame::StackFrame)
print(io, "top-level scope")
elseif linfo isa Module
Base.print_within_stacktrace(io, Base.demangle_function_name(string(frame.func)), bold=true)
elseif linfo isa MethodInstance
def = linfo.def
if def isa Module
Base.show_mi(io, linfo, #=from_stackframe=#true)
else
if linfo isa Union{MethodInstance, CodeInstance}
def = frame_method_or_module(frame)
if def isa Module
Base.show_mi(io, linfo, #=from_stackframe=#true)
else
show_spec_sig(io, def, frame_mi(frame).specTypes)
end
else
show_spec_sig(io, def, linfo.specTypes)
m = linfo::Method
show_spec_sig(io, m, m.sig)
end
else
m = linfo::Method
show_spec_sig(io, m, m.sig)
end
end

Expand Down Expand Up @@ -302,6 +331,12 @@ end

function Base.parentmodule(frame::StackFrame)
linfo = frame.linfo
if linfo isa CodeInstance
linfo = linfo.def
if isa(linfo, Core.ABIOverride)
linfo = linfo.def
end
end
if linfo isa MethodInstance
def = linfo.def
if def isa Module
Expand Down
8 changes: 4 additions & 4 deletions src/debug-registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ typedef struct {
int64_t slide;
} objfileentry_t;

// Central registry for resolving function addresses to `jl_method_instance_t`s and
// Central registry for resolving function addresses to `jl_code_instance_t`s and
// originating `ObjectFile`s (for the DWARF debug info).
//
// A global singleton instance is notified by the JIT whenever a new object is emitted,
Expand Down Expand Up @@ -82,7 +82,7 @@ class JITDebugInfoRegistry
struct image_info_t {
uint64_t base;
jl_image_fptrs_t fptrs;
jl_method_instance_t **fvars_linfo;
jl_code_instance_t **fvars_cinst;
size_t fvars_n;
};

Expand Down Expand Up @@ -124,7 +124,7 @@ class JITDebugInfoRegistry
typedef rev_map<uint64_t, objfileentry_t> objfilemap_t;

objectmap_t objectmap{};
rev_map<size_t, std::pair<size_t, jl_method_instance_t *>> linfomap{};
rev_map<size_t, std::pair<size_t, jl_code_instance_t *>> cimap{};

// Maintain a mapping of unrealized function names -> linfo objects
// so that when we see it get emitted, we can add a link back to the linfo
Expand All @@ -145,7 +145,7 @@ class JITDebugInfoRegistry
libc_frames_t libc_frames{};

void add_code_in_flight(llvm::StringRef name, jl_code_instance_t *codeinst, const llvm::DataLayout &DL) JL_NOTSAFEPOINT;
jl_method_instance_t *lookupLinfo(size_t pointer) JL_NOTSAFEPOINT;
jl_code_instance_t *lookupCodeInstance(size_t pointer) JL_NOTSAFEPOINT;
void registerJITObject(const llvm::object::ObjectFile &Object,
std::function<uint64_t(const llvm::StringRef &)> getLoadAddress) JL_NOTSAFEPOINT;
objectmap_t& getObjectMap() JL_NOTSAFEPOINT;
Expand Down
33 changes: 14 additions & 19 deletions src/debuginfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,12 @@ void JITDebugInfoRegistry::add_code_in_flight(StringRef name, jl_code_instance_t
(**codeinst_in_flight)[mangle(name, DL)] = codeinst;
}

jl_method_instance_t *JITDebugInfoRegistry::lookupLinfo(size_t pointer)
jl_code_instance_t *JITDebugInfoRegistry::lookupCodeInstance(size_t pointer)
{
jl_lock_profile();
auto region = linfomap.lower_bound(pointer);
jl_method_instance_t *linfo = NULL;
if (region != linfomap.end() && pointer < region->first + region->second.first)
auto region = cimap.lower_bound(pointer);
jl_code_instance_t *linfo = NULL;
if (region != cimap.end() && pointer < region->first + region->second.first)
linfo = region->second.second;
jl_unlock_profile();
return linfo;
Expand Down Expand Up @@ -371,14 +371,9 @@ void JITDebugInfoRegistry::registerJITObject(const object::ObjectFile &Object,
codeinst_in_flight.erase(codeinst_it);
}
}
jl_method_instance_t *mi = NULL;
if (codeinst) {
JL_GC_PROMISE_ROOTED(codeinst);
mi = jl_get_ci_mi(codeinst);
}
jl_profile_atomic([&]() JL_NOTSAFEPOINT {
if (mi)
linfomap[Addr] = std::make_pair(Size, mi);
if (codeinst)
cimap[Addr] = std::make_pair(Size, codeinst);
hassection = true;
objectmap.insert(std::pair{SectionLoadAddr, SectionInfo{
ObjectCopy,
Expand Down Expand Up @@ -506,7 +501,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->ci = NULL; // Looked up on Julia side
}
}
}
Expand Down Expand Up @@ -692,9 +687,9 @@ openDebugInfo(StringRef debuginfopath, const debug_link_info &info) JL_NOTSAFEPO
}
extern "C" JL_DLLEXPORT_CODEGEN
void jl_register_fptrs_impl(uint64_t image_base, const jl_image_fptrs_t *fptrs,
jl_method_instance_t **linfos, size_t n)
jl_code_instance_t **cinfos, size_t n)
{
getJITDebugRegistry().add_image_info({(uintptr_t) image_base, *fptrs, linfos, n});
getJITDebugRegistry().add_image_info({(uintptr_t) image_base, *fptrs, cinfos, n});
}

template<typename T>
Expand Down Expand Up @@ -1177,13 +1172,13 @@ static int jl_getDylibFunctionInfo(jl_frame_t **frames, size_t pointer, int skip
if (saddr == image.fptrs.clone_ptrs[i]) {
uint32_t idx = image.fptrs.clone_idxs[i] & jl_sysimg_val_mask;
if (idx < image.fvars_n) // items after this were cloned but not referenced directly by a method (such as our ccall PLT thunks)
frame0->linfo = image.fvars_linfo[idx];
frame0->ci = image.fvars_cinst[idx];
break;
}
}
for (size_t i = 0; i < image.fvars_n; i++) {
if (saddr == image.fptrs.ptrs[i]) {
frame0->linfo = image.fvars_linfo[i];
frame0->ci = image.fvars_cinst[i];
break;
}
}
Expand Down Expand Up @@ -1255,16 +1250,16 @@ extern "C" JL_DLLEXPORT_CODEGEN int jl_getFunctionInfo_impl(jl_frame_t **frames_
int64_t slide;
uint64_t symsize;
if (jl_DI_for_fptr(pointer, &symsize, &slide, &Section, &context)) {
frames[0].linfo = getJITDebugRegistry().lookupLinfo(pointer);
frames[0].ci = getJITDebugRegistry().lookupCodeInstance(pointer);
int nf = lookup_pointer(Section, context, frames_out, pointer, slide, true, noInline);
return nf;
}
return jl_getDylibFunctionInfo(frames_out, pointer, skipC, noInline);
}

extern "C" jl_method_instance_t *jl_gdblookuplinfo(void *p) JL_NOTSAFEPOINT
extern "C" jl_code_instance_t *jl_gdblookupci(void *p) JL_NOTSAFEPOINT
{
return getJITDebugRegistry().lookupLinfo((size_t)p);
return getJITDebugRegistry().lookupCodeInstance((size_t)p);
}

#if defined(_OS_DARWIN_) && defined(LLVM_SHLIB)
Expand Down
2 changes: 1 addition & 1 deletion src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -1349,7 +1349,7 @@ typedef struct {
char *func_name;
char *file_name;
int line;
jl_method_instance_t *linfo;
jl_code_instance_t *ci;
int fromC;
int inlined;
} jl_frame_t;
Expand Down
2 changes: 1 addition & 1 deletion src/stackwalk.c
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ JL_DLLEXPORT jl_value_t *jl_lookup_code_address(void *ip, int skipC)
jl_svecset(r, 1, jl_empty_sym);
free(frame.file_name);
jl_svecset(r, 2, jl_box_long(frame.line));
jl_svecset(r, 3, frame.linfo != NULL ? (jl_value_t*)frame.linfo : jl_nothing);
jl_svecset(r, 3, frame.ci != NULL ? (jl_value_t*)frame.ci : jl_nothing);
jl_svecset(r, 4, jl_box_bool(frame.fromC));
jl_svecset(r, 5, jl_box_bool(frame.inlined));
}
Expand Down
9 changes: 6 additions & 3 deletions test/opaque_closure.jl
Original file line number Diff line number Diff line change
Expand Up @@ -347,9 +347,12 @@ let (bt, did_gc) = make_oc_and_collect_bt()
GC.gc(true); GC.gc(true); GC.gc(true);
@test did_gc[]
@test any(stacktrace(bt)) do frame
isa(frame.linfo, Core.MethodInstance) || return false
isa(frame.linfo.def, Method) || return false
return frame.linfo.def.is_for_opaque_closure
li = frame.linfo
isa(li, Core.CodeInstance) && (li = li.def)
isa(li, Core.ABIOverride) && (li = li.def)
isa(li, Core.MethodInstance) || return false
isa(li.def, Method) || return false
return li.def.is_for_opaque_closure
end
end

Expand Down
7 changes: 4 additions & 3 deletions test/stacktraces.jl
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,10 @@ 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
# broken until #50082 can be addressed
@test frame.linfo.def.module === which(func, (Any,)).module broken=inlined
@test frame.linfo.def === which(func, (Any,)) broken=inlined
@test frame.linfo.specTypes === Tuple{typeof(func), Int} broken=inlined
mi = isa(frame.linfo, Core.CodeInstance) ? frame.linfo.def : frame.linfo
@test mi.def.module === which(func, (Any,)).module broken=inlined
@test mi.def === which(func, (Any,)) broken=inlined
@test mi.specTypes === Tuple{typeof(func), Int} broken=inlined
# line
@test frame.file === Symbol(@__FILE__)
@test !frame.from_c
Expand Down

0 comments on commit cc4122c

Please sign in to comment.