Skip to content

Commit c6805e2

Browse files
authored
staticdata: Set min validation world to require world (JuliaLang#57381)
When we have implicit binding edges from literal GlobalRefs, these edges imply a implicit minimum world age of jl_require_age (which is what all bindings loaded from pkgimages get their definition age set to). In JuliaLang#57318, the `cpuid_llvm` minimum world age got set to `1` rather than `jl_require_world`, so codegen would refuse to perform the llvmcall special codegen, since it couldn't guarantee that the binding would actually resolve to `llvmcall` in all worlds. Fix that by adjusting staticdata.jl to set the appropriate minworld. That said, there are a few complicating factors here: 1. In general, most of our code doesn't handle world ranges with more than one partition. But for example, codegen could have checked the current world age and implicitly partitioned the binding at codegen time (in practice just adding an appropraite error check). 2. The included test case uses a cached inference. However, in the original issue it appeared that inlining was also creating new references to this replaced binding, which should not have been permitted. I have not fully investigated this behavior yet and this might be another bug. 3. In the original issue, the specialization had its max_world terminated a few ages past jl_require_world. I do not understand this behavior yet. Still, fixes JuliaLang#57318.
1 parent 90046a0 commit c6805e2

File tree

3 files changed

+38
-5
lines changed

3 files changed

+38
-5
lines changed

base/loading.jl

+2-2
Original file line numberDiff line numberDiff line change
@@ -3157,11 +3157,11 @@ This can be used to reduce package load times. Cache files are stored in
31573157
`DEPOT_PATH[1]/compiled`. See [Module initialization and precompilation](@ref)
31583158
for important notes.
31593159
"""
3160-
function compilecache(pkg::PkgId, internal_stderr::IO = stderr, internal_stdout::IO = stdout; flags::Cmd=``, reasons::Union{Dict{String,Int},Nothing}=Dict{String,Int}(), loadable_exts::Union{Vector{PkgId},Nothing}=nothing)
3160+
function compilecache(pkg::PkgId, internal_stderr::IO = stderr, internal_stdout::IO = stdout; flags::Cmd=``, cacheflags::CacheFlags=CacheFlags(), reasons::Union{Dict{String,Int},Nothing}=Dict{String,Int}(), loadable_exts::Union{Vector{PkgId},Nothing}=nothing)
31613161
@nospecialize internal_stderr internal_stdout
31623162
path = locate_package(pkg)
31633163
path === nothing && throw(ArgumentError("$(repr("text/plain", pkg)) not found during precompilation"))
3164-
return compilecache(pkg, path, internal_stderr, internal_stdout; flags, reasons, loadable_exts)
3164+
return compilecache(pkg, path, internal_stderr, internal_stdout; flags, cacheflags, reasons, loadable_exts)
31653165
end
31663166

31673167
const MAX_NUM_PRECOMPILE_FILES = Ref(10)

base/staticdata.jl

+11-3
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ function verify_method_graph(codeinst::CodeInstance, stack::Vector{CodeInstance}
7070
nothing
7171
end
7272

73+
get_require_world() = unsafe_load(cglobal(:jl_require_world, UInt))
74+
7375
# Test all edges relevant to a method:
7476
# - Visit the entire call graph, starting from edges[idx] to determine if that method is valid
7577
# - Implements Tarjan's SCC (strongly connected components) algorithm, simplified to remove the count variable
@@ -81,7 +83,14 @@ function verify_method(codeinst::CodeInstance, stack::Vector{CodeInstance}, visi
8183
return 0, world, max_valid2
8284
end
8385
end
84-
local minworld::UInt, maxworld::UInt = 1, validation_world
86+
# Implicitly referenced bindings in the current module do not get explicit edges.
87+
# If they were invalidated, they'll be in `mwis`. If they weren't, they imply a minworld
88+
# of `get_require_world`. In principle, this is only required for methods that do reference
89+
# an implicit globalref. However, we already don't perform this validation for methods that
90+
# don't have any (implicit or explicit) edges at all. The remaining corner case (some explicit,
91+
# but no implicit edges) is rare and there would be little benefit to lower the minworld for it
92+
# in any case, so we just always use `get_require_world` here.
93+
local minworld::UInt, maxworld::UInt = get_require_world(), validation_world
8594
def = get_ci_mi(codeinst).def
8695
@assert def isa Method
8796
if haskey(visiting, codeinst)
@@ -103,9 +112,8 @@ function verify_method(codeinst::CodeInstance, stack::Vector{CodeInstance}, visi
103112
# verify current edges
104113
if isempty(callees)
105114
# quick return: no edges to verify (though we probably shouldn't have gotten here from WORLD_AGE_REVALIDATION_SENTINEL)
106-
elseif maxworld == unsafe_load(cglobal(:jl_require_world, UInt))
115+
elseif maxworld == get_require_world()
107116
# if no new worlds were allocated since serializing the base module, then no new validation is worth doing right now either
108-
minworld = maxworld
109117
else
110118
j = 1
111119
while j length(callees)

test/precompile.jl

+25
Original file line numberDiff line numberDiff line change
@@ -2285,4 +2285,29 @@ precompile_test_harness("Constprop CodeInstance invalidation") do load_path
22852285
end
22862286
end
22872287

2288+
precompile_test_harness("llvmcall validation") do load_path
2289+
write(joinpath(load_path, "LLVMCall.jl"),
2290+
"""
2291+
module LLVMCall
2292+
using Base: llvmcall
2293+
@noinline do_llvmcall() = llvmcall("ret i32 0", UInt32, Tuple{})
2294+
do_llvmcall2() = do_llvmcall()
2295+
do_llvmcall2()
2296+
end
2297+
""")
2298+
# Also test with --pkgimages=no
2299+
testcode = """
2300+
insert!(LOAD_PATH, 1, $(repr(load_path)))
2301+
insert!(DEPOT_PATH, 1, $(repr(load_path)))
2302+
using LLVMCall
2303+
LLVMCall.do_llvmcall2()
2304+
"""
2305+
@test readchomp(`$(Base.julia_cmd()) --pkgimages=no -E $(testcode)`) == repr(UInt32(0))
2306+
# Now the regular way
2307+
@eval using LLVMCall
2308+
invokelatest() do
2309+
@test LLVMCall.do_llvmcall2() == UInt32(0)
2310+
end
2311+
end
2312+
22882313
finish_precompile_test!()

0 commit comments

Comments
 (0)