From 6448ad84072e656b790826570e48815e6277b70f Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Tue, 7 Jan 2025 23:16:15 +0000 Subject: [PATCH] Add edge kind for access to non-explicit partitioned bindings Our binding partion invalidation code scans the original source for any GlobalRefs that need to be invalidated. However, this is not the only source of access to binding partitions. Various intrinsics (in particular the `*global` ones) also operate on bindings. Since these are not manifest in the source, we instead use the existing `edges` mechanism to give them forward edges. This PR only includes the basic info type, and validation in the replacement case. There's a bit more work to do there, but I'm waiting on #56499 for that part, precompilation in particular. --- Compiler/src/abstractinterpretation.jl | 86 ++++++++++++-------------- Compiler/src/stmtinfo.jl | 14 +++++ Compiler/src/typeinfer.jl | 3 + base/invalidation.jl | 41 ++++++++---- src/module.c | 8 +-- src/staticdata_utils.c | 8 +++ stdlib/REPL/src/REPLCompletions.jl | 6 +- sysimage.mk | 1 + test/rebinding.jl | 9 ++- 9 files changed, 110 insertions(+), 66 deletions(-) diff --git a/Compiler/src/abstractinterpretation.jl b/Compiler/src/abstractinterpretation.jl index 0c340a89d9878..3766907905aef 100644 --- a/Compiler/src/abstractinterpretation.jl +++ b/Compiler/src/abstractinterpretation.jl @@ -2395,7 +2395,8 @@ function abstract_eval_getglobal(interp::AbstractInterpreter, sv::AbsIntState, s if M isa Const && s isa Const M, s = M.val, s.val if M isa Module && s isa Symbol - return CallMeta(abstract_eval_globalref(interp, GlobalRef(M, s), saw_latestworld, sv), NoCallInfo()) + (ret, bpart) = abstract_eval_globalref(interp, GlobalRef(M, s), saw_latestworld, sv) + return CallMeta(ret, bpart === nothing ? NoCallInfo() : GlobalAccessInfo(bpart)) end return CallMeta(Union{}, TypeError, EFFECTS_THROWS, NoCallInfo()) elseif !hasintersect(widenconst(M), Module) || !hasintersect(widenconst(s), Symbol) @@ -2473,8 +2474,8 @@ function abstract_eval_setglobal!(interp::AbstractInterpreter, sv::AbsIntState, if isa(M, Const) && isa(s, Const) M, s = M.val, s.val if M isa Module && s isa Symbol - rt, exct = global_assignment_rt_exct(interp, sv, saw_latestworld, GlobalRef(M, s), v) - return CallMeta(rt, exct, Effects(setglobal!_effects, nothrow=exct===Bottom), NoCallInfo()) + (rt, exct), partition = global_assignment_rt_exct(interp, sv, saw_latestworld, GlobalRef(M, s), v) + return CallMeta(rt, exct, Effects(setglobal!_effects, nothrow=exct===Bottom), GlobalAccessInfo(partition)) end return CallMeta(Union{}, TypeError, EFFECTS_THROWS, NoCallInfo()) end @@ -2512,7 +2513,7 @@ function abstract_eval_swapglobal!(interp::AbstractInterpreter, sv::AbsIntState, scm = abstract_eval_setglobal!(interp, sv, saw_latestworld, M, s, v) scm.rt === Bottom && return scm gcm = abstract_eval_getglobal(interp, sv, saw_latestworld, M, s) - return CallMeta(gcm.rt, Union{scm.exct,gcm.exct}, merge_effects(scm.effects, gcm.effects), NoCallInfo()) + return CallMeta(gcm.rt, Union{scm.exct,gcm.exct}, merge_effects(scm.effects, gcm.effects), scm.info) end function abstract_eval_swapglobal!(interp::AbstractInterpreter, sv::AbsIntState, saw_latestworld::Bool, @@ -2520,7 +2521,7 @@ function abstract_eval_swapglobal!(interp::AbstractInterpreter, sv::AbsIntState, scm = abstract_eval_setglobal!(interp, sv, saw_latestworld, M, s, v, order) scm.rt === Bottom && return scm gcm = abstract_eval_getglobal(interp, sv, saw_latestworld, M, s, order) - return CallMeta(gcm.rt, Union{scm.exct,gcm.exct}, merge_effects(scm.effects, gcm.effects), NoCallInfo()) + return CallMeta(gcm.rt, Union{scm.exct,gcm.exct}, merge_effects(scm.effects, gcm.effects), scm.info) end function abstract_eval_swapglobal!(interp::AbstractInterpreter, sv::AbsIntState, saw_latestworld::Bool, argtypes::Vector{Any}) @@ -2569,7 +2570,7 @@ function abstract_eval_replaceglobal!(interp::AbstractInterpreter, sv::AbsIntSta end exct = Union{rte.exct, global_assignment_binding_rt_exct(interp, partition, v)[2]} effects = merge_effects(rte.effects, Effects(setglobal!_effects, nothrow=exct===Bottom)) - sg = CallMeta(Any, exct, effects, NoCallInfo()) + sg = CallMeta(Any, exct, effects, GlobalAccessInfo(partition)) else sg = abstract_eval_setglobal!(interp, sv, saw_latestworld, M, s, v) end @@ -2937,7 +2938,8 @@ function abstract_eval_special_value(interp::AbstractInterpreter, @nospecialize( return RTEffects(sv.ir.argtypes[e.n], Union{}, EFFECTS_TOTAL) # TODO frame_argtypes(sv)[e.n] and remove the assertion end elseif isa(e, GlobalRef) - return abstract_eval_globalref(interp, e, sstate.saw_latestworld, sv) + # No need for an edge since an explicit GlobalRef will be picked up by the source scan + return abstract_eval_globalref(interp, e, sstate.saw_latestworld, sv)[1] end if isa(e, QuoteNode) e = e.value @@ -3193,14 +3195,31 @@ function abstract_eval_isdefined_expr(interp::AbstractInterpreter, e::Expr, ssta end return RTEffects(rt, Union{}, EFFECTS_TOTAL) end - return abstract_eval_isdefined(interp, sym, sstate.saw_latestworld, sv) + rt = Bool + effects = EFFECTS_TOTAL + exct = Union{} + if isexpr(sym, :static_parameter) + n = sym.args[1]::Int + if 1 <= n <= length(sv.sptypes) + sp = sv.sptypes[n] + if !sp.undef + rt = Const(true) + elseif sp.typ === Bottom + rt = Const(false) + end + end + else + effects = EFFECTS_UNKNOWN + exct = Any + end + return RTEffects(rt, exct, effects) end const generic_isdefinedglobal_effects = Effects(EFFECTS_TOTAL, consistent=ALWAYS_FALSE, nothrow=false) function abstract_eval_isdefinedglobal(interp::AbstractInterpreter, mod::Module, sym::Symbol, allow_import::Union{Bool, Nothing}, saw_latestworld::Bool, sv::AbsIntState) rt = Bool if saw_latestworld - return RTEffects(rt, Union{}, Effects(generic_isdefinedglobal_effects, nothrow=true)) + return CallMeta(RTEffects(rt, Union{}, Effects(generic_isdefinedglobal_effects, nothrow=true)), NoCallInfo()) end effects = EFFECTS_TOTAL @@ -3222,7 +3241,7 @@ function abstract_eval_isdefinedglobal(interp::AbstractInterpreter, mod::Module, effects = Effects(generic_isdefinedglobal_effects, nothrow=true) end end - return RTEffects(rt, Union{}, effects) + return CallMeta(RTEffects(rt, Union{}, effects), GlobalAccessInfo(partition)) end function abstract_eval_isdefinedglobal(interp::AbstractInterpreter, @nospecialize(M), @nospecialize(s), @nospecialize(allow_import_arg), @nospecialize(order_arg), saw_latestworld::Bool, sv::AbsIntState) @@ -3247,7 +3266,7 @@ function abstract_eval_isdefinedglobal(interp::AbstractInterpreter, @nospecializ if M isa Const && s isa Const M, s = M.val, s.val if M isa Module && s isa Symbol - return merge_exct(CallMeta(abstract_eval_isdefinedglobal(interp, M, s, allow_import, saw_latestworld, sv), NoCallInfo()), exct) + return merge_exct(abstract_eval_isdefinedglobal(interp, M, s, allow_import, saw_latestworld, sv), exct) end return CallMeta(Union{}, TypeError, EFFECTS_THROWS, NoCallInfo()) elseif !hasintersect(widenconst(M), Module) || !hasintersect(widenconst(s), Symbol) @@ -3258,26 +3277,6 @@ function abstract_eval_isdefinedglobal(interp::AbstractInterpreter, @nospecializ return CallMeta(Bool, Union{exct, TypeError, UndefVarError}, generic_isdefinedglobal_effects, NoCallInfo()) end -function abstract_eval_isdefined(interp::AbstractInterpreter, @nospecialize(sym), saw_latestworld::Bool, sv::AbsIntState) - rt = Bool - effects = EFFECTS_TOTAL - exct = Union{} - if isexpr(sym, :static_parameter) - n = sym.args[1]::Int - if 1 <= n <= length(sv.sptypes) - sp = sv.sptypes[n] - if !sp.undef - rt = Const(true) - elseif sp.typ === Bottom - rt = Const(false) - end - end - else - effects = EFFECTS_UNKNOWN - end - return RTEffects(rt, exct, effects) -end - function abstract_eval_throw_undef_if_not(interp::AbstractInterpreter, e::Expr, sstate::StatementState, sv::AbsIntState) condt = abstract_eval_value(interp, e.args[2], sstate, sv) condval = maybe_extract_const_bool(condt) @@ -3533,26 +3532,29 @@ end function abstract_eval_globalref(interp::AbstractInterpreter, g::GlobalRef, saw_latestworld::Bool, sv::AbsIntState) if saw_latestworld - return RTEffects(Any, Any, generic_getglobal_effects) + return Pair{RTEffects, Union{Nothing, Core.BindingPartition}}(RTEffects(Any, Any, generic_getglobal_effects), nothing) end partition = abstract_eval_binding_partition!(interp, g, sv) ret = abstract_eval_partition_load(interp, partition) if ret.rt !== Union{} && ret.exct === UndefVarError && InferenceParams(interp).assume_bindings_static if isdefined(g, :binding) && isdefined(g.binding, :value) - return RTEffects(ret.rt, Union{}, Effects(generic_getglobal_effects, nothrow=true)) + ret = RTEffects(ret.rt, Union{}, Effects(generic_getglobal_effects, nothrow=true)) end # We do not assume in general that assigned global bindings remain assigned. # The existence of pkgimages allows them to revert in practice. end - return ret + return Pair{RTEffects, Union{Nothing, Core.BindingPartition}}(ret, partition) end function global_assignment_rt_exct(interp::AbstractInterpreter, sv::AbsIntState, saw_latestworld::Bool, g::GlobalRef, @nospecialize(newty)) if saw_latestworld - return Pair{Any,Any}(newty, Union{ErrorException, TypeError}) + return Pair{Pair{Any,Any}, Union{Core.BindingPartition, Nothing}}( + Pair{Any,Any}(newty, Union{ErrorException, TypeError}), nothing) end partition = abstract_eval_binding_partition!(interp, g, sv) - return global_assignment_binding_rt_exct(interp, partition, newty) + return Pair{Pair{Any,Any}, Union{Core.BindingPartition, Nothing}}( + global_assignment_binding_rt_exct(interp, partition, newty), + partition) end function global_assignment_binding_rt_exct(interp::AbstractInterpreter, partition::Core.BindingPartition, @nospecialize(newty)) @@ -3573,18 +3575,6 @@ function global_assignment_binding_rt_exct(interp::AbstractInterpreter, partitio return Pair{Any,Any}(newty, Bottom) end -function handle_global_assignment!(interp::AbstractInterpreter, frame::InferenceState, saw_latestworld::Bool, lhs::GlobalRef, @nospecialize(newty)) - effect_free = ALWAYS_FALSE - nothrow = global_assignment_rt_exct(interp, frame, saw_latestworld, lhs, ignorelimited(newty))[2] === Union{} - inaccessiblememonly = ALWAYS_FALSE - if !nothrow - sub_curr_ssaflag!(frame, IR_FLAG_NOTHROW) - end - sub_curr_ssaflag!(frame, IR_FLAG_EFFECT_FREE) - merge_effects!(interp, frame, Effects(EFFECTS_TOTAL; effect_free, nothrow, inaccessiblememonly)) - return nothing -end - abstract_eval_ssavalue(s::SSAValue, sv::InferenceState) = abstract_eval_ssavalue(s, sv.ssavaluetypes) function abstract_eval_ssavalue(s::SSAValue, ssavaluetypes::Vector{Any}) diff --git a/Compiler/src/stmtinfo.jl b/Compiler/src/stmtinfo.jl index a42a9e47b328e..e3f8e2f56c86b 100644 --- a/Compiler/src/stmtinfo.jl +++ b/Compiler/src/stmtinfo.jl @@ -481,4 +481,18 @@ end add_edges_impl(edges::Vector{Any}, info::VirtualMethodMatchInfo) = _add_edges_impl(edges, info.info, #=mi_edge=#true) +""" + info::GlobalAccessInfo <: CallInfo + +Represents access to a global through runtime reflection, rather than as a manifest +`GlobalRef` in the source code. Used for builtins (getglobal/setglobal/etc.) that +perform such accesses. +""" +struct GlobalAccessInfo <: CallInfo + bpart::Core.BindingPartition +end +GlobalAccessInfo(::Nothing) = NoCallInfo() +add_edges_impl(edges::Vector{Any}, info::GlobalAccessInfo) = + push!(edges, info.bpart) + @specialize diff --git a/Compiler/src/typeinfer.jl b/Compiler/src/typeinfer.jl index 49cc711dc72df..ec1e62817106d 100644 --- a/Compiler/src/typeinfer.jl +++ b/Compiler/src/typeinfer.jl @@ -544,6 +544,9 @@ function store_backedges(caller::CodeInstance, edges::SimpleVector) # ignore `Method`-edges (from e.g. failed `abstract_call_method`) i += 1 continue + elseif isa(item, Core.BindingPartition) + i += 1 + continue end if isa(item, CodeInstance) item = item.def diff --git a/base/invalidation.jl b/base/invalidation.jl index 40a010ab7361c..5abb0b74ad884 100644 --- a/base/invalidation.jl +++ b/base/invalidation.jl @@ -9,7 +9,7 @@ globalrefs(mod::Module) = GlobalRefIterator(mod) function iterate(gri::GlobalRefIterator, i = 1) m = gri.mod table = ccall(:jl_module_get_bindings, Ref{SimpleVector}, (Any,), m) - i == length(table) && return nothing + i > length(table) && return nothing b = table[i] b === nothing && return iterate(gri, i+1) return ((b::Core.Binding).globalref, i+1) @@ -85,18 +85,33 @@ function should_invalidate_code_for_globalref(gr::GlobalRef, src::CodeInfo) return found_any end -function invalidate_code_for_globalref!(gr::GlobalRef, new_max_world::UInt) - valid_in_valuepos = false - foreach_reachable_mtable(new_max_world) do mt::Core.MethodTable - for method in MethodList(mt) - if isdefined(method, :source) - src = _uncompressed_ir(method) - old_stmts = src.code - if should_invalidate_code_for_globalref(gr, src) +function scan_edge_list(ci::Core.CodeInstance, bpart::Core.BindingPartition) + isdefined(ci, :edges) || return false + edges = ci.edges + i = 1 + while i <= length(edges) + if isassigned(edges, i) && edges[i] === bpart + return true + end + i += 1 + end + return false +end + +function invalidate_code_for_globalref!(gr::GlobalRef, invalidated_bpart::Core.BindingPartition, new_max_world::UInt) + try + valid_in_valuepos = false + foreach_reachable_mtable(new_max_world) do mt::Core.MethodTable + for method in MethodList(mt) + if isdefined(method, :source) + src = _uncompressed_ir(method) + old_stmts = src.code + invalidate_all = should_invalidate_code_for_globalref(gr, src) for mi in specializations(method) + isdefined(mi, :cache) || continue ci = mi.cache while true - if ci.max_world > new_max_world + if ci.max_world > new_max_world && (invalidate_all || scan_edge_list(ci, invalidated_bpart)) ccall(:jl_invalidate_code_instance, Cvoid, (Any, UInt), ci, new_max_world) end isdefined(ci, :next) || break @@ -105,7 +120,11 @@ function invalidate_code_for_globalref!(gr::GlobalRef, new_max_world::UInt) end end end + return true end - return true + catch err + bt = catch_backtrace() + invokelatest(Base.println, "Internal Error during invalidation:") + invokelatest(Base.display_error, err, bt) end end diff --git a/src/module.c b/src/module.c index 004371b9144b2..66049031f8790 100644 --- a/src/module.c +++ b/src/module.c @@ -1032,7 +1032,7 @@ JL_DLLEXPORT void jl_set_const(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *var jl_gc_wb(bpart, val); } -void jl_invalidate_binding_refs(jl_globalref_t *ref, size_t new_world) +void jl_invalidate_binding_refs(jl_globalref_t *ref, jl_binding_partition_t *invalidated_bpart, size_t new_world) { static jl_value_t *invalidate_code_for_globalref = NULL; if (invalidate_code_for_globalref == NULL && jl_base_module != NULL) @@ -1043,7 +1043,7 @@ void jl_invalidate_binding_refs(jl_globalref_t *ref, size_t new_world) jl_error("Binding invalidation is not permitted during image generation."); jl_value_t *boxed_world = jl_box_ulong(new_world); JL_GC_PUSH1(&boxed_world); - jl_call2((jl_function_t*)invalidate_code_for_globalref, (jl_value_t*)ref, boxed_world); + jl_call3((jl_function_t*)invalidate_code_for_globalref, (jl_value_t*)ref, (jl_value_t*)invalidated_bpart, boxed_world); JL_GC_POP(); } @@ -1063,10 +1063,10 @@ JL_DLLEXPORT void jl_disable_binding(jl_globalref_t *gr) jl_task_t *ct = jl_current_task; size_t last_world = ct->world_age; size_t new_max_world = jl_atomic_load_acquire(&jl_world_counter); + jl_atomic_store_release(&bpart->max_world, new_max_world); ct->world_age = jl_typeinf_world; - jl_invalidate_binding_refs(gr, new_max_world); + jl_invalidate_binding_refs(gr, bpart, new_max_world); ct->world_age = last_world; - jl_atomic_store_release(&bpart->max_world, new_max_world); jl_atomic_store_release(&jl_world_counter, new_max_world + 1); JL_UNLOCK(&world_counter_lock); } diff --git a/src/staticdata_utils.c b/src/staticdata_utils.c index 5de85a8d6ec77..79d2909970d99 100644 --- a/src/staticdata_utils.c +++ b/src/staticdata_utils.c @@ -878,6 +878,10 @@ static int jl_verify_method(jl_code_instance_t *codeinst, size_t *minworld, size size_t min_valid2; size_t max_valid2; assert(!jl_is_method(edge)); // `Method`-edge isn't allowed for the optimized one-edge format + if (jl_is_binding_partition(edge)) { + j += 1; + continue; + } if (jl_is_code_instance(edge)) edge = (jl_value_t*)jl_get_ci_mi((jl_code_instance_t*)edge); if (jl_is_method_instance(edge)) { @@ -1051,6 +1055,10 @@ static void jl_insert_backedges(jl_array_t *edges, jl_array_t *ext_ci_list) j += 1; continue; } + else if (jl_is_binding_partition(edge)) { + j += 1; + continue; + } if (jl_is_code_instance(edge)) edge = (jl_value_t*)((jl_code_instance_t*)edge)->def; if (jl_is_method_instance(edge)) { diff --git a/stdlib/REPL/src/REPLCompletions.jl b/stdlib/REPL/src/REPLCompletions.jl index 2c03c6d563c1e..723a008b10f28 100644 --- a/stdlib/REPL/src/REPLCompletions.jl +++ b/stdlib/REPL/src/REPLCompletions.jl @@ -646,9 +646,11 @@ function CC.abstract_eval_globalref(interp::REPLInterpreter, g::GlobalRef, baile sv::CC.InferenceState) if (interp.limit_aggressive_inference ? is_repl_frame(sv) : is_call_graph_uncached(sv)) if isdefined_globalref(g) - return CC.RTEffects(Const(ccall(:jl_get_globalref_value, Any, (Any,), g)), Union{}, CC.EFFECTS_TOTAL) + return Pair{CC.RTEffects, Union{Nothing, Core.BindingPartition}}( + CC.RTEffects(Const(ccall(:jl_get_globalref_value, Any, (Any,), g)), Union{}, CC.EFFECTS_TOTAL), nothing) end - return CC.RTEffects(Union{}, UndefVarError, CC.EFFECTS_THROWS) + return Pair{CC.RTEffects, Union{Nothing, Core.BindingPartition}}( + CC.RTEffects(Union{}, UndefVarError, CC.EFFECTS_THROWS), nothing) end return @invoke CC.abstract_eval_globalref(interp::CC.AbstractInterpreter, g::GlobalRef, bailed::Bool, sv::CC.InferenceState) diff --git a/sysimage.mk b/sysimage.mk index a74aace4dd11c..571e2da003346 100644 --- a/sysimage.mk +++ b/sysimage.mk @@ -43,6 +43,7 @@ COMPILER_SRCS := $(addprefix $(JULIAHOME)/, \ base/int.jl \ base/indices.jl \ base/iterators.jl \ + base/invalidation.jl \ base/namedtuple.jl \ base/number.jl \ base/operators.jl \ diff --git a/test/rebinding.jl b/test/rebinding.jl index ad0ad1fc1643d..10da27ce3ad8f 100644 --- a/test/rebinding.jl +++ b/test/rebinding.jl @@ -31,7 +31,7 @@ module Rebinding # Tests for @world syntax @test Base.@world(Foo, defined_world_age) == typeof(x) - @test Base.@world(Rebinding.Foo, defined_world_age) == typeof(x) + nameof(@__MODULE__) === :Rebinding && @test Base.@world(Rebinding.Foo, defined_world_age) == typeof(x) @test Base.@world((@__MODULE__).Foo, defined_world_age) == typeof(x) # Test invalidation (const -> undefined) @@ -40,4 +40,11 @@ module Rebinding @test f_return_delete_me() == 1 Base.delete_binding(@__MODULE__, :delete_me) @test_throws UndefVarError f_return_delete_me() + + ## + via indirect access + const delete_me = 2 + f_return_delete_me_indirect() = getglobal(@__MODULE__, :delete_me) + @test f_return_delete_me_indirect() == 2 + Base.delete_binding(@__MODULE__, :delete_me) + @test_throws UndefVarError f_return_delete_me_indirect() end