From 33afb811f05aa9dc62825fadd76e825f00636d97 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Mon, 3 Oct 2022 15:10:32 -0400 Subject: [PATCH] remove svec(#null) from the language (#46975) This was not supported, and almost never used, so audit all places it could happen and avoid it at the source. Fix #46784 Co-authored-by: Keno Fischer --- base/essentials.jl | 7 +++++-- src/builtins.c | 2 +- src/ccall.cpp | 24 ------------------------ src/gf.c | 1 + src/jl_exported_funcs.inc | 1 - src/jltypes.c | 28 +++++++++++++--------------- src/julia_internal.h | 2 +- src/simplevector.c | 8 +------- src/staticdata.c | 21 +++++++++++++-------- src/subtype.c | 25 +++++++++++++++++++++---- test/core.jl | 2 +- test/show.jl | 6 ------ 12 files changed, 57 insertions(+), 70 deletions(-) diff --git a/base/essentials.jl b/base/essentials.jl index 1d02c3ffdf1b1..daee352b7649d 100644 --- a/base/essentials.jl +++ b/base/essentials.jl @@ -690,7 +690,10 @@ end @eval getindex(v::SimpleVector, i::Int) = Core._svec_ref($(Expr(:boundscheck)), v, i) function length(v::SimpleVector) - return ccall(:jl_svec_len, Int, (Any,), v) + t = @_gc_preserve_begin v + len = unsafe_load(Ptr{Int}(pointer_from_objref(v))) + @_gc_preserve_end t + return len end firstindex(v::SimpleVector) = 1 lastindex(v::SimpleVector) = length(v) @@ -745,7 +748,7 @@ function isassigned end function isassigned(v::SimpleVector, i::Int) @boundscheck 1 <= i <= length(v) || return false - return ccall(:jl_svec_isassigned, Bool, (Any, Int), v, i - 1) + return true end diff --git a/src/builtins.c b/src/builtins.c index 954bff7771e3e..595014e97ee50 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -569,7 +569,7 @@ STATIC_INLINE void _grow_to(jl_value_t **root, jl_value_t ***oldargs, jl_svec_t *n_alloc = newalloc; } -static jl_value_t *do_apply( jl_value_t **args, uint32_t nargs, jl_value_t *iterate) +static jl_value_t *do_apply(jl_value_t **args, uint32_t nargs, jl_value_t *iterate) { jl_function_t *f = args[0]; if (nargs == 2) { diff --git a/src/ccall.cpp b/src/ccall.cpp index 99cf6a1f67f96..fb5799b081537 100644 --- a/src/ccall.cpp +++ b/src/ccall.cpp @@ -28,7 +28,6 @@ TRANSFORMED_CCALL_STAT(jl_set_next_task); TRANSFORMED_CCALL_STAT(jl_sigatomic_begin); TRANSFORMED_CCALL_STAT(jl_sigatomic_end); TRANSFORMED_CCALL_STAT(jl_svec_len); -TRANSFORMED_CCALL_STAT(jl_svec_isassigned); TRANSFORMED_CCALL_STAT(jl_svec_ref); TRANSFORMED_CCALL_STAT(jl_array_isassigned); TRANSFORMED_CCALL_STAT(jl_string_ptr); @@ -1687,28 +1686,6 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs) JL_GC_POP(); return mark_or_box_ccall_result(ctx, len, retboxed, rt, unionall, static_rt); } - else if (is_libjulia_func(jl_svec_isassigned) && - argv[1].typ == (jl_value_t*)jl_long_type) { - ++CCALL_STAT(jl_svec_isassigned); - assert(!isVa && !llvmcall && nccallargs == 2); - const jl_cgval_t &svecv = argv[0]; - const jl_cgval_t &idxv = argv[1]; - Value *idx = emit_unbox(ctx, getSizeTy(ctx.builder.getContext()), idxv, (jl_value_t*)jl_long_type); - idx = ctx.builder.CreateAdd(idx, ConstantInt::get(getSizeTy(ctx.builder.getContext()), 1)); - auto ptr = emit_bitcast(ctx, boxed(ctx, svecv), ctx.types().T_pprjlvalue); - Value *slot_addr = ctx.builder.CreateInBoundsGEP(ctx.types().T_prjlvalue, - decay_derived(ctx, ptr), idx); - LoadInst *load = ctx.builder.CreateAlignedLoad(ctx.types().T_prjlvalue, slot_addr, - Align(sizeof(void*))); - load->setAtomic(AtomicOrdering::Unordered); - // Only mark with TBAA if we are sure about the type. - // This could otherwise be in a dead branch - if (svecv.typ == (jl_value_t*)jl_simplevector_type) - tbaa_decorate(ctx.tbaa().tbaa_const, load); - Value *res = ctx.builder.CreateZExt(ctx.builder.CreateICmpNE(load, Constant::getNullValue(ctx.types().T_prjlvalue)), getInt8Ty(ctx.builder.getContext())); - JL_GC_POP(); - return mark_or_box_ccall_result(ctx, res, retboxed, rt, unionall, static_rt); - } else if (is_libjulia_func(jl_svec_ref) && argv[1].typ == (jl_value_t*)jl_long_type) { ++CCALL_STAT(jl_svec_ref); assert(lrt == ctx.types().T_prjlvalue); @@ -1727,7 +1704,6 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs) // This could otherwise be in a dead branch if (svecv.typ == (jl_value_t*)jl_simplevector_type) tbaa_decorate(ctx.tbaa().tbaa_const, load); - null_pointer_check(ctx, load); JL_GC_POP(); return mark_or_box_ccall_result(ctx, load, retboxed, rt, unionall, static_rt); } diff --git a/src/gf.c b/src/gf.c index d3b1fac6c2b07..138092ab9c93e 100644 --- a/src/gf.c +++ b/src/gf.c @@ -1151,6 +1151,7 @@ static jl_method_instance_t *cache_method( // NULL, jl_emptysvec, /*guard*/NULL, jl_cachearg_offset(mt), other->min_world, other->max_world); } } + assert(guards == jl_svec_len(guardsigs)); } if (!cache_with_orig) { // determined above that there's no ambiguity in also using compilationsig as the cacheablesig diff --git a/src/jl_exported_funcs.inc b/src/jl_exported_funcs.inc index f47e5bcb17592..b17251d4a5af3 100644 --- a/src/jl_exported_funcs.inc +++ b/src/jl_exported_funcs.inc @@ -453,7 +453,6 @@ XX(jl_svec2) \ XX(jl_svec_copy) \ XX(jl_svec_fill) \ - XX(jl_svec_isassigned) \ XX(jl_svec_ref) \ XX(jl_switch) \ XX(jl_switchto) \ diff --git a/src/jltypes.c b/src/jltypes.c index b45c34ed7e5ed..7c527450b4cd3 100644 --- a/src/jltypes.c +++ b/src/jltypes.c @@ -657,9 +657,9 @@ static jl_datatype_t *lookup_type_set(jl_svec_t *cache, jl_value_t **key, size_t size_t iter = 0; do { jl_datatype_t *val = jl_atomic_load_relaxed(&tab[index]); - if (val == NULL) + if ((jl_value_t*)val == jl_nothing) return NULL; - if ((jl_value_t*)val != jl_nothing && val->hash == hv && typekey_eq(val, key, n)) + if (val->hash == hv && typekey_eq(val, key, n)) return val; index = (index + 1) & (sz - 1); iter++; @@ -680,9 +680,9 @@ static jl_datatype_t *lookup_type_setvalue(jl_svec_t *cache, jl_value_t *key1, j size_t iter = 0; do { jl_datatype_t *val = jl_atomic_load_relaxed(&tab[index]); - if (val == NULL) + if ((jl_value_t*)val == jl_nothing) return NULL; - if ((jl_value_t*)val != jl_nothing && val->hash == hv && typekeyvalue_eq(val, key1, key, n, leaf)) + if (val->hash == hv && typekeyvalue_eq(val, key1, key, n, leaf)) return val; index = (index + 1) & (sz - 1); iter++; @@ -702,7 +702,7 @@ static ssize_t lookup_type_idx_linear(jl_svec_t *cache, jl_value_t **key, size_t ssize_t i; for (i = 0; i < cl; i++) { jl_datatype_t *tt = jl_atomic_load_relaxed(&data[i]); - if (tt == NULL) + if ((jl_value_t*)tt == jl_nothing) return ~i; if (typekey_eq(tt, key, n)) return i; @@ -719,7 +719,7 @@ static ssize_t lookup_type_idx_linearvalue(jl_svec_t *cache, jl_value_t *key1, j ssize_t i; for (i = 0; i < cl; i++) { jl_datatype_t *tt = jl_atomic_load_relaxed(&data[i]); - if (tt == NULL) + if ((jl_value_t*)tt == jl_nothing) return ~i; if (typekeyvalue_eq(tt, key1, key, n, 1)) return i; @@ -777,7 +777,7 @@ static int cache_insert_type_set_(jl_svec_t *a, jl_datatype_t *val, uint_t hv, i size_t maxprobe = max_probe(sz); do { jl_value_t *tab_i = jl_atomic_load_relaxed(&tab[index]); - if (tab_i == NULL || tab_i == jl_nothing) { + if (tab_i == jl_nothing) { if (atomic) jl_atomic_store_release(&tab[index], (jl_value_t*)val); else @@ -792,8 +792,6 @@ static int cache_insert_type_set_(jl_svec_t *a, jl_datatype_t *val, uint_t hv, i return 0; } -static jl_svec_t *cache_rehash_set(jl_svec_t *a, size_t newsz); - static void cache_insert_type_set(jl_datatype_t *val, uint_t hv) { jl_svec_t *a = jl_atomic_load_relaxed(&val->name->cache); @@ -820,17 +818,17 @@ static void cache_insert_type_set(jl_datatype_t *val, uint_t hv) } } -static jl_svec_t *cache_rehash_set(jl_svec_t *a, size_t newsz) +jl_svec_t *cache_rehash_set(jl_svec_t *a, size_t newsz) { jl_value_t **ol = jl_svec_data(a); size_t sz = jl_svec_len(a); while (1) { size_t i; - jl_svec_t *newa = jl_alloc_svec(newsz); + jl_svec_t *newa = jl_svec_fill(newsz, jl_nothing); JL_GC_PUSH1(&newa); for (i = 0; i < sz; i += 1) { jl_value_t *val = ol[i]; - if (val != NULL && val != jl_nothing) { + if (val != jl_nothing) { uint_t hv = ((jl_datatype_t*)val)->hash; if (!cache_insert_type_set_(newa, (jl_datatype_t*)val, hv, 0)) { break; @@ -849,15 +847,15 @@ static void cache_insert_type_linear(jl_datatype_t *type, ssize_t insert_at) jl_svec_t *cache = jl_atomic_load_relaxed(&type->name->linearcache); assert(jl_is_svec(cache)); size_t n = jl_svec_len(cache); - if (n == 0 || jl_svecref(cache, n - 1) != NULL) { - jl_svec_t *nc = jl_alloc_svec(n < 8 ? 8 : (n*3)>>1); + if (n == 0 || jl_svecref(cache, n - 1) != jl_nothing) { + jl_svec_t *nc = jl_svec_fill(n < 4 ? 4 : n * 2, jl_nothing); memcpy(jl_svec_data(nc), jl_svec_data(cache), sizeof(void*) * n); jl_atomic_store_release(&type->name->linearcache, nc); jl_gc_wb(type->name, nc); cache = nc; n = jl_svec_len(nc); } - assert(jl_svecref(cache, insert_at) == NULL); + assert(jl_svecref(cache, insert_at) == jl_nothing); jl_svecset(cache, insert_at, (jl_value_t*)type); // todo: make this an atomic-store } diff --git a/src/julia_internal.h b/src/julia_internal.h index b5f6349bee72c..b5fbf9416fcf0 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -685,6 +685,7 @@ jl_vararg_t *jl_wrap_vararg(jl_value_t *t, jl_value_t *n); void jl_reinstantiate_inner_types(jl_datatype_t *t); jl_datatype_t *jl_lookup_cache_type_(jl_datatype_t *type); void jl_cache_type_(jl_datatype_t *type); +jl_svec_t *cache_rehash_set(jl_svec_t *a, size_t newsz); void set_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_t *rhs, int isatomic) JL_NOTSAFEPOINT; jl_value_t *swap_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_t *rhs, int isatomic); jl_value_t *modify_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_t *op, jl_value_t *rhs, int isatomic); @@ -1430,7 +1431,6 @@ int jl_typemap_intersection_visitor(jl_typemap_t *a, int offs, struct typemap_in // For codegen only. JL_DLLEXPORT size_t (jl_svec_len)(jl_svec_t *t) JL_NOTSAFEPOINT; -JL_DLLEXPORT int8_t jl_svec_isassigned(jl_svec_t *t JL_PROPAGATES_ROOT, ssize_t i) JL_NOTSAFEPOINT; JL_DLLEXPORT jl_value_t *jl_svec_ref(jl_svec_t *t JL_PROPAGATES_ROOT, ssize_t i); diff --git a/src/simplevector.c b/src/simplevector.c index 988cf18ccc9b6..cb65646e00936 100644 --- a/src/simplevector.c +++ b/src/simplevector.c @@ -93,15 +93,9 @@ JL_DLLEXPORT size_t (jl_svec_len)(jl_svec_t *t) JL_NOTSAFEPOINT return jl_svec_len(t); } -JL_DLLEXPORT int8_t jl_svec_isassigned(jl_svec_t *t JL_PROPAGATES_ROOT, ssize_t i) JL_NOTSAFEPOINT -{ - return jl_svecref(t, (size_t)i) != NULL; -} - JL_DLLEXPORT jl_value_t *jl_svec_ref(jl_svec_t *t JL_PROPAGATES_ROOT, ssize_t i) { jl_value_t *v = jl_svecref(t, (size_t)i); - if (__unlikely(v == NULL)) - jl_throw(jl_undefref_exception); + assert(v != NULL); return v; } diff --git a/src/staticdata.c b/src/staticdata.c index 38d56a37f3db6..10c1c3cae9e3f 100644 --- a/src/staticdata.c +++ b/src/staticdata.c @@ -1646,7 +1646,7 @@ static void jl_scan_type_cache_gv(jl_serializer_state *s, jl_svec_t *cache) size_t l = jl_svec_len(cache), i; for (i = 0; i < l; i++) { jl_value_t *ti = jl_svecref(cache, i); - if (ti == NULL || ti == jl_nothing) + if (ti == jl_nothing) continue; if (jl_get_llvm_gv(native_functions, ti)) { jl_serialize_value(s, ti); @@ -1660,16 +1660,21 @@ static void jl_scan_type_cache_gv(jl_serializer_state *s, jl_svec_t *cache) } // remove cached types not referenced in the stream -static void jl_prune_type_cache_hash(jl_svec_t *cache) +static jl_svec_t *jl_prune_type_cache_hash(jl_svec_t *cache) JL_GC_DISABLED { size_t l = jl_svec_len(cache), i; for (i = 0; i < l; i++) { jl_value_t *ti = jl_svecref(cache, i); - if (ti == NULL || ti == jl_nothing) + if (ti == jl_nothing) continue; if (ptrhash_get(&backref_table, ti) == HT_NOTFOUND) jl_svecset(cache, i, jl_nothing); } + void *idx = ptrhash_get(&backref_table, cache); + ptrhash_remove(&backref_table, cache); + cache = cache_rehash_set(cache, l); + ptrhash_put(&backref_table, cache, idx); + return cache; } static void jl_prune_type_cache_linear(jl_svec_t *cache) @@ -1677,14 +1682,13 @@ static void jl_prune_type_cache_linear(jl_svec_t *cache) size_t l = jl_svec_len(cache), ins = 0, i; for (i = 0; i < l; i++) { jl_value_t *ti = jl_svecref(cache, i); - if (ti == NULL) + if (ti == jl_nothing) break; if (ptrhash_get(&backref_table, ti) != HT_NOTFOUND) jl_svecset(cache, ins++, ti); } - if (i > ins) { - memset(&jl_svec_data(cache)[ins], 0, (i - ins) * sizeof(jl_value_t*)); - } + while (ins < l) + jl_svecset(cache, ins++, jl_nothing); } static jl_value_t *strip_codeinfo_meta(jl_method_t *m, jl_value_t *ci_, int orig) @@ -1958,7 +1962,8 @@ static void jl_save_system_image_to_stream(ios_t *f) JL_GC_DISABLED // built-in type caches for (i = 0; i < typenames.len; i++) { jl_typename_t *tn = (jl_typename_t*)typenames.items[i]; - jl_prune_type_cache_hash(tn->cache); + tn->cache = jl_prune_type_cache_hash(tn->cache); + jl_gc_wb(tn, tn->cache); jl_prune_type_cache_linear(tn->linearcache); } arraylist_free(&typenames); diff --git a/src/subtype.c b/src/subtype.c index 9509db359b159..55579f2b47305 100644 --- a/src/subtype.c +++ b/src/subtype.c @@ -3547,17 +3547,26 @@ jl_value_t *jl_type_intersection_env_s(jl_value_t *a, jl_value_t *b, jl_svec_t * } } if (sz == 0 && szb > 0) { - while (jl_is_unionall(b)) { - env[i++] = (jl_value_t*)((jl_unionall_t*)b)->var; - b = ((jl_unionall_t*)b)->body; + jl_unionall_t *ub = (jl_unionall_t*)b; + while (jl_is_unionall(ub)) { + env[i++] = (jl_value_t*)ub->var; + ub = (jl_unionall_t*)ub->body; } sz = szb; } if (penv) { jl_svec_t *e = jl_alloc_svec(sz); *penv = e; - for(i=0; i < sz; i++) + for (i = 0; i < sz; i++) jl_svecset(e, i, env[i]); + jl_unionall_t *ub = (jl_unionall_t*)b; + for (i = 0; i < sz; i++) { + assert(jl_is_unionall(ub)); + // TODO: assert(env[i] != NULL); + if (env[i] == NULL) + env[i] = (jl_value_t*)ub->var; + ub = (jl_unionall_t*)ub->body; + } } bot: JL_GC_POP(); @@ -3601,6 +3610,14 @@ int jl_subtype_matching(jl_value_t *a, jl_value_t *b, jl_svec_t **penv) *penv = e; for (i = 0; i < szb; i++) jl_svecset(e, i, env[i]); + jl_unionall_t *ub = (jl_unionall_t*)b; + for (i = 0; i < szb; i++) { + assert(jl_is_unionall(ub)); + // TODO: assert(env[i] != NULL); + if (env[i] == NULL) + env[i] = (jl_value_t*)ub->var; + ub = (jl_unionall_t*)ub->body; + } } JL_GC_POP(); return sub; diff --git a/test/core.jl b/test/core.jl index b84f0ef76bd14..35b029f93da44 100644 --- a/test/core.jl +++ b/test/core.jl @@ -784,7 +784,7 @@ end mutable struct Type11167{T,N} end function count11167() let cache = Type11167.body.body.name.cache - return sum(i -> isassigned(cache, i), 0:length(cache)) + return count(!isnothing, cache) end end @test count11167() == 0 diff --git a/test/show.jl b/test/show.jl index 6ea52489c3d9a..30c06817bff54 100644 --- a/test/show.jl +++ b/test/show.jl @@ -1275,12 +1275,6 @@ end let repr = sprint(dump, Core.svec()) @test repr == "empty SimpleVector\n" end -let sv = Core.svec(:a, :b, :c) - # unsafe replacement of :c with #undef to test handling of incomplete SimpleVectors - unsafe_store!(convert(Ptr{Ptr{Cvoid}}, Base.pointer_from_objref(sv)) + 3 * sizeof(Ptr), C_NULL) - repr = sprint(dump, sv) - @test repr == "SimpleVector\n 1: Symbol a\n 2: Symbol b\n 3: #undef\n" -end let repr = sprint(dump, sin) @test repr == "sin (function of type typeof(sin))\n" end