Skip to content

Commit

Permalink
Validate CodeInstances with no external edges
Browse files Browse the repository at this point in the history
In #38983 and #41872, it was discovered that only CodeInstances with
external backedges got validated after deserialization.
This stores new CodeInstances in a list, and if they have neither been
validated nor invalidated, it validates them.

Closes #41872
  • Loading branch information
timholy committed Aug 23, 2021
1 parent 9cad1e0 commit 6c2b718
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 7 deletions.
38 changes: 31 additions & 7 deletions src/dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ static jl_value_t *deser_symbols[256];
static htable_t backref_table;
static int backref_table_numel;
static arraylist_t backref_list;
static arraylist_t new_code_instances;

// list of (jl_value_t **loc, size_t pos) entries
// for anything that was flagged by the deserializer for later
Expand Down Expand Up @@ -1587,8 +1588,11 @@ static jl_value_t *jl_deserialize_value_method_instance(jl_serializer_state *s,
if (mi->callbacks)
jl_gc_wb(mi, mi->callbacks);
mi->cache = (jl_code_instance_t*)jl_deserialize_value(s, (jl_value_t**)&mi->cache);
if (mi->cache)
if (mi->cache) {
jl_gc_wb(mi, mi->cache);
if (internal)
arraylist_push(&new_code_instances, mi->cache);
}
return (jl_value_t*)mi;
}

Expand Down Expand Up @@ -2036,6 +2040,7 @@ static void jl_insert_backedges(jl_array_t *list, jl_array_t *targets)
int32_t idx = idxs[j];
valid = jl_array_uint8_ref(valids, idx);
}
size_t ci_world;
if (valid) {
// if this callee is still valid, add all the backedges
for (j = 0; j < jl_array_len(idxs_array); j++) {
Expand All @@ -2051,24 +2056,38 @@ static void jl_insert_backedges(jl_array_t *list, jl_array_t *targets)
}
}
// then enable it
jl_code_instance_t *codeinst = caller->cache;
while (codeinst) {
if (codeinst->min_world > 0)
codeinst->max_world = ~(size_t)0;
codeinst = jl_atomic_load_relaxed(&codeinst->next);
}
ci_world = ~(size_t)0;
}
else {
ci_world = jl_world_counter > 1 ? jl_world_counter - 1 : 1;
if (_jl_debug_method_invalidation) {
jl_array_ptr_1d_push(_jl_debug_method_invalidation, (jl_value_t*)caller);
loctag = jl_cstr_to_string("insert_backedges");
jl_array_ptr_1d_push(_jl_debug_method_invalidation, loctag);
}
}
jl_code_instance_t *codeinst = caller->cache;
while (codeinst) {
if (codeinst->min_world > 0)
codeinst->max_world = ci_world;
codeinst = jl_atomic_load_relaxed(&codeinst->next);
}
}
JL_GC_POP();
}

static void validate_new_code_instances(void)
{
size_t i;
for (i = 0; i < new_code_instances.len; i++) {
jl_code_instance_t *codeinst = (jl_code_instance_t*)new_code_instances.items[i];
while (codeinst) {
if (codeinst->min_world > 0 && codeinst->max_world == 0) // lacks external edges
codeinst->max_world = ~(size_t)0;
codeinst = jl_atomic_load_relaxed(&codeinst->next);
}
}
}

static jl_value_t *read_verify_mod_list(ios_t *s, jl_array_t *mod_list)
{
Expand Down Expand Up @@ -2636,6 +2655,7 @@ static jl_value_t *_jl_restore_incremental(ios_t *f, jl_array_t *mod_array)
arraylist_new(&backref_list, 4000);
arraylist_push(&backref_list, jl_main_module);
arraylist_new(&flagref_list, 0);
arraylist_new(&new_code_instances, 0);
arraylist_new(&ccallable_list, 0);
htable_new(&uniquing_table, 0);

Expand Down Expand Up @@ -2671,7 +2691,11 @@ static jl_value_t *_jl_restore_incremental(ios_t *f, jl_array_t *mod_array)

jl_insert_backedges((jl_array_t*)external_backedges, (jl_array_t*)external_edges); // restore external backedges (needs to be last)

// check new CodeInstances and validate any that lack external backedges
validate_new_code_instances();

serializer_worklist = NULL;
arraylist_free(&new_code_instances);
arraylist_free(&flagref_list);
arraylist_free(&backref_list);
ios_close(f);
Expand Down
28 changes: 28 additions & 0 deletions test/precompile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,34 @@ precompile_test_harness("Renamed Imports") do load_path
@test (@eval (using RenameImports; RenameImports.test())) isa Module
end

# issue #41872 (example from #38983)
precompile_test_harness("No external edges") do load_path
write(joinpath(load_path, "NoExternalEdges.jl"),
"""
module NoExternalEdges
bar(x::Int) = hcat(rand())
@inline bar() = hcat(rand())
bar(x::Float64) = bar()
foo1() = bar(1)
foo2() = bar(1.0)
foo3() = bar()
foo4() = hcat(rand())
precompile(foo1, ())
precompile(foo2, ())
precompile(foo3, ())
precompile(foo4, ())
end
""")
Base.compilecache(Base.PkgId("NoExternalEdges"))
@eval begin
using NoExternalEdges
@test only(methods(NoExternalEdges.foo1)).specializations[1].cache.max_world != 0
@test only(methods(NoExternalEdges.foo2)).specializations[1].cache.max_world != 0
@test only(methods(NoExternalEdges.foo3)).specializations[1].cache.max_world != 0
@test only(methods(NoExternalEdges.foo4)).specializations[1].cache.max_world != 0
end
end

@testset "issue 38149" begin
M = Module()
@eval M begin
Expand Down

0 comments on commit 6c2b718

Please sign in to comment.