Skip to content

Commit

Permalink
Check world age bounds on candidate ambiguities. Fixes #28899 (#28916)
Browse files Browse the repository at this point in the history
(cherry picked from commit 513db98)
  • Loading branch information
timholy authored and KristofferC committed Sep 12, 2018
1 parent 447103c commit a2de527
Show file tree
Hide file tree
Showing 11 changed files with 38 additions and 16 deletions.
2 changes: 1 addition & 1 deletion base/compiler/utilities.jl
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ function retrieve_code_info(linfo::MethodInstance)
end

function code_for_method(method::Method, @nospecialize(atypes), sparams::SimpleVector, world::UInt, preexisting::Bool=false)
if world < min_world(method)
if world < min_world(method) || world > max_world(method)
return nothing
end
if isdefined(method, :generator) && !isdispatchtuple(atypes)
Expand Down
2 changes: 2 additions & 0 deletions base/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,8 @@ function show_method_candidates(io::IO, ex::MethodError, @nospecialize kwargs=()
end
if ex.world < min_world(method)
print(iob, " (method too new to be called from this world context.)")
elseif ex.world > max_world(method)
print(iob, " (method deleted before this world age.)")
end
# TODO: indicate if it's in the wrong world
push!(lines, (buf, right_matches))
Expand Down
2 changes: 1 addition & 1 deletion base/methodshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ end

function kwarg_decl(m::Method, kwtype::DataType)
sig = rewrap_unionall(Tuple{kwtype, Any, unwrap_unionall(m.sig).parameters...}, m.sig)
kwli = ccall(:jl_methtable_lookup, Any, (Any, Any, UInt), kwtype.name.mt, sig, typemax(UInt))
kwli = ccall(:jl_methtable_lookup, Any, (Any, Any, UInt), kwtype.name.mt, sig, max_world(m))
if kwli !== nothing
kwli = kwli::Method
src = uncompressed_ast(kwli)
Expand Down
2 changes: 1 addition & 1 deletion base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1103,7 +1103,7 @@ has_bottom_parameter(t::TypeVar) = t.ub == Bottom || has_bottom_parameter(t.ub)
has_bottom_parameter(::Any) = false

min_world(m::Method) = reinterpret(UInt, m.min_world)
max_world(m::Method) = typemax(UInt)
max_world(m::Method) = reinterpret(UInt, m.max_world)
min_world(m::Core.MethodInstance) = reinterpret(UInt, m.min_world)
max_world(m::Core.MethodInstance) = reinterpret(UInt, m.max_world)

Expand Down
3 changes: 2 additions & 1 deletion src/dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -1647,6 +1647,7 @@ static jl_value_t *jl_deserialize_value_method(jl_serializer_state *s, jl_value_
m->file = (jl_sym_t*)jl_deserialize_value(s, NULL);
m->line = read_int32(s->s);
m->min_world = jl_world_counter;
m->max_world = ~(size_t)0;
m->ambig = jl_deserialize_value(s, (jl_value_t**)&m->ambig);
jl_gc_wb(m, m->ambig);
m->called = read_int32(s->s);
Expand Down Expand Up @@ -2992,7 +2993,7 @@ static jl_method_instance_t *jl_recache_method_instance(jl_method_instance_t *li
//assert(ti != jl_bottom_type); (void)ti;
if (ti == jl_bottom_type)
env = jl_emptysvec; // the intersection may fail now if the type system had made an incorrect subtype env in the past
jl_method_instance_t *_new = jl_specializations_get_linfo(m, (jl_value_t*)argtypes, env, jl_world_counter);
jl_method_instance_t *_new = jl_specializations_get_linfo(m, (jl_value_t*)argtypes, env, jl_world_counter < max_world ? jl_world_counter : max_world);
_new->max_world = max_world;
jl_update_backref_list((jl_value_t*)li, (jl_value_t*)_new, start);
return _new;
Expand Down
14 changes: 8 additions & 6 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ static int8_t jl_cachearg_offset(jl_methtable_t *mt)
// get or create the MethodInstance for a specialization
JL_DLLEXPORT jl_method_instance_t *jl_specializations_get_linfo(jl_method_t *m, jl_value_t *type, jl_svec_t *sparams, size_t world)
{
assert(world >= m->min_world && "typemap lookup is corrupted");
assert(world >= m->min_world && world <= m->max_world && "typemap lookup is corrupted");
JL_LOCK(&m->writelock);
jl_typemap_entry_t *sf =
jl_typemap_assoc_by_type(m->specializations, type, NULL, /*subtype*/0, /*offs*/0, world, /*max_world_mask*/0);
Expand All @@ -169,7 +169,7 @@ JL_DLLEXPORT jl_method_instance_t *jl_specializations_get_linfo(jl_method_t *m,
li->min_world = world;
}
if (world == jl_world_counter) {
li->max_world = ~(size_t)0;
li->max_world = m->max_world;
}
else {
li->max_world = world;
Expand Down Expand Up @@ -358,6 +358,7 @@ JL_DLLEXPORT jl_method_instance_t* jl_set_method_inferred(
else {
JL_LOCK(&li->def.method->writelock);
assert(min_world >= li->def.method->min_world);
assert(max_world <= li->def.method->max_world);
int isinferred = jl_is_rettype_inferred(li);
if (!isinferred && li->min_world >= min_world && li->max_world <= max_world) {
// expand the current (uninferred) entry to cover the full inferred range
Expand Down Expand Up @@ -955,7 +956,7 @@ static jl_method_instance_t *cache_method(
jl_tupletype_t *cachett = tt;
jl_svec_t* guardsigs = jl_emptysvec;
size_t min_valid = definition->min_world;
size_t max_valid = ~(size_t)0;
size_t max_valid = definition->max_world;
if (!cache_with_orig) {
// now examine what will happen if we chose to use this sig in the cache
temp = ml_matches(mt->defs, 0, compilationsig, -1, 0, world, &min_valid, &max_valid); // TODO: use MAX_UNSPECIALIZED_CONFLICTS?
Expand Down Expand Up @@ -1501,6 +1502,7 @@ JL_DLLEXPORT void jl_method_table_disable(jl_methtable_t *mt, jl_method_t *metho
jl_error("method not in method table");
JL_LOCK(&mt->writelock);
// Narrow the world age on the method to make it uncallable
method->max_world = jl_world_counter;
methodentry->max_world = jl_world_counter++;
// Recompute ambiguities (deleting a more specific method might reveal ambiguities that it previously resolved)
check_ambiguous_matches(mt->defs, methodentry, check_disabled_ambiguous_visitor); // TODO: decrease repeated work?
Expand All @@ -1525,7 +1527,7 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method
JL_LOCK(&mt->writelock);
jl_typemap_entry_t *newentry = jl_typemap_insert(&mt->defs, (jl_value_t*)mt,
(jl_tupletype_t*)type, simpletype, jl_emptysvec, (jl_value_t*)method, 0, &method_defs,
method->min_world, ~(size_t)0, &oldvalue);
method->min_world, method->max_world, &oldvalue);
if (oldvalue) {
if (oldvalue == (jl_value_t*)method) {
// redundant add of same method; no need to do anything
Expand Down Expand Up @@ -1982,7 +1984,7 @@ JL_DLLEXPORT int jl_is_call_ambiguous(jl_value_t *types, jl_method_t *m)
return 0;
for (size_t i = 0; i < jl_array_len(m->ambig); i++) {
jl_method_t *mambig = (jl_method_t*)jl_array_ptr_ref(m->ambig, i);
if (jl_subtype((jl_value_t*)types, (jl_value_t*)mambig->sig))
if (mambig->min_world <= jl_world_counter && jl_world_counter <= mambig->max_world && jl_subtype((jl_value_t*)types, (jl_value_t*)mambig->sig))
return 1;
}
return 0;
Expand All @@ -1996,7 +1998,7 @@ JL_DLLEXPORT int jl_has_call_ambiguities(jl_value_t *types, jl_method_t *m)
return 0;
for (size_t i = 0; i < jl_array_len(m->ambig); i++) {
jl_method_t *mambig = (jl_method_t*)jl_array_ptr_ref(m->ambig, i);
if (!jl_has_empty_intersection(mambig->sig, types))
if (mambig->min_world <= jl_world_counter && jl_world_counter <= mambig->max_world && !jl_has_empty_intersection(mambig->sig, types))
return 1;
}
return 0;
Expand Down
10 changes: 6 additions & 4 deletions src/jltypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -2059,13 +2059,14 @@ void jl_init_types(void)
jl_method_type =
jl_new_datatype(jl_symbol("Method"), core,
jl_any_type, jl_emptysvec,
jl_perm_symsvec(19,
jl_perm_symsvec(20,
"name",
"module",
"file",
"line",
"sig",
"min_world",
"max_world",
"ambig",
"specializations",
"sparam_syms",
Expand All @@ -2079,13 +2080,14 @@ void jl_init_types(void)
"nospecialize",
"isva",
"pure"),
jl_svec(19,
jl_svec(20,
jl_sym_type,
jl_module_type,
jl_sym_type,
jl_int32_type,
jl_type_type,
jl_long_type,
jl_long_type,
jl_any_type, // Union{Array, Nothing}
jl_any_type, // TypeMap
jl_simplevector_type,
Expand All @@ -2099,7 +2101,7 @@ void jl_init_types(void)
jl_int32_type,
jl_bool_type,
jl_bool_type),
0, 1, 9);
0, 1, 10);

jl_method_instance_type =
jl_new_datatype(jl_symbol("MethodInstance"), core,
Expand Down Expand Up @@ -2211,7 +2213,7 @@ void jl_init_types(void)
jl_svecset(jl_methtable_type->types, 7, jl_int32_type); // DWORD
#endif
jl_svecset(jl_methtable_type->types, 8, jl_int32_type); // uint32_t
jl_svecset(jl_method_type->types, 10, jl_method_instance_type);
jl_svecset(jl_method_type->types, 11, jl_method_instance_type);
jl_svecset(jl_method_instance_type->types, 11, jl_voidpointer_type);
jl_svecset(jl_method_instance_type->types, 12, jl_voidpointer_type);
jl_svecset(jl_method_instance_type->types, 13, jl_voidpointer_type);
Expand Down
1 change: 1 addition & 0 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ typedef struct _jl_method_t {
// method's type signature. redundant with TypeMapEntry->specTypes
jl_value_t *sig;
size_t min_world;
size_t max_world;

// list of potentially-ambiguous methods (nothing = none, Vector{Any} of Methods otherwise)
jl_value_t *ambig;
Expand Down
4 changes: 3 additions & 1 deletion src/method.c
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ jl_method_instance_t *jl_get_specialized(jl_method_t *m, jl_value_t *types, jl_s
new_linfo->specTypes = types;
new_linfo->sparam_vals = sp;
new_linfo->min_world = m->min_world;
new_linfo->max_world = ~(size_t)0;
new_linfo->max_world = m->max_world;
return new_linfo;
}

Expand Down Expand Up @@ -595,6 +595,7 @@ JL_DLLEXPORT jl_method_t *jl_new_method_uninit(jl_module_t *module)
m->nargs = 0;
m->traced = 0;
m->min_world = 1;
m->max_world = ~(size_t)0;
JL_MUTEX_INIT(&m->writelock);
return m;
}
Expand Down Expand Up @@ -639,6 +640,7 @@ static jl_method_t *jl_new_method(

JL_GC_POP();
m->min_world = ++jl_world_counter;
m->max_world = ~(size_t)0;
return m;
}

Expand Down
2 changes: 1 addition & 1 deletion src/typemap.c
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,7 @@ jl_typemap_entry_t *jl_typemap_entry_assoc_exact(jl_typemap_entry_t *ml, jl_valu
{
// some manually-unrolled common special cases
while (ml->simplesig == (void*)jl_nothing && ml->guardsigs == jl_emptysvec && ml->isleafsig) {
// use a tight loop for a long as possible
// use a tight loop for as long as possible
if (world >= ml->min_world && world <= ml->max_world) {
if (n == jl_field_count(ml->sig) && jl_typeof(args[0]) == jl_tparam(ml->sig, 0)) {
if (n == 1)
Expand Down
12 changes: 12 additions & 0 deletions test/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,18 @@ typeparam(::Type{T}, a::AbstractArray{T}) where T = 2
@test typeparam(Float64, rand(2)) == 1
@test typeparam(Int, rand(Int, 2)) == 2

# prior ambiguities (issue #28899)
uambig(::Union{Int,Nothing}) = 1
uambig(::Union{Float64,Nothing}) = 2
@test uambig(1) == 1
@test uambig(1.0) == 2
@test_throws MethodError uambig(nothing)
m = which(uambig, Tuple{Int})
Base.delete_method(m)
@test_throws MethodError uambig(1)
@test uambig(1.0) == 2
@test uambig(nothing) == 2

end

# issue #26267
Expand Down

0 comments on commit a2de527

Please sign in to comment.