From ab0713aaa8fa385474e2c53c242c91cafe857a4d Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Wed, 29 Aug 2018 04:24:02 -0500 Subject: [PATCH] Check world age bounds on candidate ambiguities. Fixes #28899 --- base/compiler/utilities.jl | 2 +- base/errorshow.jl | 2 ++ base/methodshow.jl | 2 +- base/reflection.jl | 2 +- src/dump.c | 3 ++- src/gf.c | 14 ++++++++------ src/jltypes.c | 10 ++++++---- src/julia.h | 1 + src/method.c | 4 +++- src/typemap.c | 2 +- test/reflection.jl | 12 ++++++++++++ 11 files changed, 38 insertions(+), 16 deletions(-) diff --git a/base/compiler/utilities.jl b/base/compiler/utilities.jl index 0edcec83bd038..89f5c0b05fb52 100644 --- a/base/compiler/utilities.jl +++ b/base/compiler/utilities.jl @@ -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) diff --git a/base/errorshow.jl b/base/errorshow.jl index c6c81a02081ba..f9de780cf0446 100644 --- a/base/errorshow.jl +++ b/base/errorshow.jl @@ -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)) diff --git a/base/methodshow.jl b/base/methodshow.jl index e02101ba533d1..2015a564ac1f8 100644 --- a/base/methodshow.jl +++ b/base/methodshow.jl @@ -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) diff --git a/base/reflection.jl b/base/reflection.jl index 25de43a3270dd..c20a15b959ffa 100644 --- a/base/reflection.jl +++ b/base/reflection.jl @@ -1101,7 +1101,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) diff --git a/src/dump.c b/src/dump.c index f970009de5ea8..359565f9f4b10 100644 --- a/src/dump.c +++ b/src/dump.c @@ -1644,6 +1644,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); @@ -2991,7 +2992,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; diff --git a/src/gf.c b/src/gf.c index 7480524a16872..aa597ecbaa116 100644 --- a/src/gf.c +++ b/src/gf.c @@ -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_PROPAGATES_ROOT, 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); @@ -169,7 +169,7 @@ JL_DLLEXPORT jl_method_instance_t *jl_specializations_get_linfo(jl_method_t *m J 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; @@ -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 @@ -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? @@ -1513,6 +1514,7 @@ JL_DLLEXPORT void jl_method_table_disable(jl_methtable_t *mt, jl_method_t *metho jl_typemap_entry_t *methodentry = do_typemap_search(mt, method); 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? @@ -1537,7 +1539,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 @@ -1995,7 +1997,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; @@ -2009,7 +2011,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; diff --git a/src/jltypes.c b/src/jltypes.c index 6258b3bfb3cdb..889ec95545eb4 100644 --- a/src/jltypes.c +++ b/src/jltypes.c @@ -2063,13 +2063,14 @@ void jl_init_types(void) JL_GC_DISABLED 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", @@ -2083,13 +2084,14 @@ void jl_init_types(void) JL_GC_DISABLED "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, @@ -2103,7 +2105,7 @@ void jl_init_types(void) JL_GC_DISABLED 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, @@ -2215,7 +2217,7 @@ void jl_init_types(void) JL_GC_DISABLED 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); diff --git a/src/julia.h b/src/julia.h index 97951e4ceba92..5b35d52dc41d1 100644 --- a/src/julia.h +++ b/src/julia.h @@ -266,6 +266,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; diff --git a/src/method.c b/src/method.c index 4365def7f3d11..c829c0c45782d 100644 --- a/src/method.c +++ b/src/method.c @@ -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; } @@ -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; } @@ -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; } diff --git a/src/typemap.c b/src/typemap.c index 42e5fb6e82b22..ea1df9fb517ec 100644 --- a/src/typemap.c +++ b/src/typemap.c @@ -744,7 +744,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) diff --git a/test/reflection.jl b/test/reflection.jl index 7c992072fbf79..b1f5fecc9178f 100644 --- a/test/reflection.jl +++ b/test/reflection.jl @@ -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