Skip to content

Commit 58fb3ad

Browse files
committed
Add missing root for method instances used in opaque closure
Usually the rooting path for method instances is something like this: Module -> DataType -> TypeName -> MethodTable -> Method -> MethodInstance so these MethodInstances are effectively globally rooted. However, the Method(instances) inside opaque closures do not have this rooting path and can be deleted. Unfortunately, if we codegen'ed these opaque closures, we do have a reference to the method instance in our global debuginfo tables. This was causing crashes in cases where the oc was gc'ed before a stack trace could be printed. Fix that by adding any method instance that needs to be rooted to the global roots table.
1 parent bd8350b commit 58fb3ad

File tree

4 files changed

+46
-5
lines changed

4 files changed

+46
-5
lines changed

src/debug-registry.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ class JITDebugInfoRegistry
137137
jl_method_instance_t *lookupLinfo(size_t pointer) JL_NOTSAFEPOINT;
138138
void registerJITObject(const llvm::object::ObjectFile &Object,
139139
std::function<uint64_t(const llvm::StringRef &)> getLoadAddress,
140-
std::function<void*(void*)> lookupWriteAddress) JL_NOTSAFEPOINT;
140+
std::function<void*(void*)> lookupWriteAddress);
141141
objectmap_t& getObjectMap() JL_NOTSAFEPOINT;
142142
void add_image_info(image_info_t info) JL_NOTSAFEPOINT;
143143
bool get_image_info(uint64_t base, image_info_t *info) const JL_NOTSAFEPOINT;

src/debuginfo.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -372,9 +372,18 @@ void JITDebugInfoRegistry::registerJITObject(const object::ObjectFile &Object,
372372
codeinst_in_flight.erase(codeinst_it);
373373
}
374374
}
375+
jl_method_instance_t *mi = NULL;
376+
if (codeinst) {
377+
mi = codeinst->def;
378+
// Non-opaque-closure MethodInstances are considered globally rooted
379+
// through their methods, but for OC, we need to create a global root
380+
// here.
381+
if (jl_is_method(mi->def.value) && mi->def.method->is_for_opaque_closure)
382+
mi = (jl_method_instance_t*)jl_as_global_root((jl_value_t*)mi);
383+
}
375384
jl_profile_atomic([&]() JL_NOTSAFEPOINT {
376-
if (codeinst)
377-
linfomap[Addr] = std::make_pair(Size, codeinst->def);
385+
if (mi)
386+
linfomap[Addr] = std::make_pair(Size, mi);
378387
if (first) {
379388
objectmap[SectionLoadAddr] = {&Object,
380389
(size_t)SectionSize,
@@ -390,7 +399,7 @@ void JITDebugInfoRegistry::registerJITObject(const object::ObjectFile &Object,
390399

391400
void jl_register_jit_object(const object::ObjectFile &Object,
392401
std::function<uint64_t(const StringRef &)> getLoadAddress,
393-
std::function<void *(void *)> lookupWriteAddress) JL_NOTSAFEPOINT
402+
std::function<void *(void *)> lookupWriteAddress)
394403
{
395404
getJITDebugRegistry().registerJITObject(Object, getLoadAddress, lookupWriteAddress);
396405
}

src/jitlayers.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,7 @@ void JuliaOJIT::OptSelLayerT::emit(std::unique_ptr<orc::MaterializationResponsib
690690

691691
void jl_register_jit_object(const object::ObjectFile &debugObj,
692692
std::function<uint64_t(const StringRef &)> getLoadAddress,
693-
std::function<void *(void *)> lookupWriteAddress) JL_NOTSAFEPOINT;
693+
std::function<void *(void *)> lookupWriteAddress);
694694

695695
namespace {
696696

test/opaque_closure.jl

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,3 +297,35 @@ for f in (const_int, const_int_barrier)
297297
@test_throws TypeError eval_oc_spec(oc_mismatch)
298298
end
299299
end
300+
301+
302+
# Attempting to construct an opaque closure backtrace after the oc is GC'ed
303+
f_oc_throws() = error("oops")
304+
@noinline function make_oc_and_collect_bt()
305+
did_gc = Ref{Bool}(false)
306+
bt = let ir = first(only(Base.code_ircode(f_oc_throws, ())))
307+
sentinel = Ref{Any}(nothing)
308+
oc = OpaqueClosure(ir, sentinel)
309+
finalizer(sentinel) do x
310+
did_gc[] = true
311+
end
312+
try
313+
oc()
314+
@test false
315+
catch e
316+
bt = catch_backtrace()
317+
@test isa(e, ErrorException)
318+
bt
319+
end
320+
end
321+
return bt, did_gc
322+
end
323+
let (bt, did_gc) = make_oc_and_collect_bt()
324+
GC.gc(true); GC.gc(true); GC.gc(true);
325+
@test did_gc[]
326+
@test any(stacktrace(bt)) do frame
327+
isa(frame.linfo, Core.MethodInstance) || return false
328+
isa(frame.linfo.def, Method) || return false
329+
return frame.linfo.def.is_for_opaque_closure
330+
end
331+
end

0 commit comments

Comments
 (0)