From 23c9e0f2abf52ceefa9d3dd06dd85f63a938b00b Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Mon, 13 Jul 2020 21:18:14 -0400 Subject: [PATCH 1/2] Rm some dead code This code is dead, but also has GC-rooting issues (it modifies iparams with a newly allocated object, but iparams is not guaranteed to be roots). Delete it and add an assert that should fire if anybody ever changes the surrounding code to make it necessary again (at which point they can find this commit and fix it properly). --- src/jltypes.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/jltypes.c b/src/jltypes.c index ab84096f7b917..988f8bd75e123 100644 --- a/src/jltypes.c +++ b/src/jltypes.c @@ -1189,7 +1189,7 @@ static void check_datatype_parameters(jl_typename_t *tn, jl_value_t **params, si JL_GC_POP(); } -static jl_value_t *extract_wrapper(jl_value_t *t JL_PROPAGATES_ROOT) +static jl_value_t *extract_wrapper(jl_value_t *t JL_PROPAGATES_ROOT) JL_GLOBALLY_ROOTED { t = jl_unwrap_unionall(t); if (jl_is_datatype(t)) @@ -1255,12 +1255,9 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value jl_value_t *tw = extract_wrapper(pi); if (tw && tw != pi && (tn != jl_type_typename || jl_typeof(pi) == jl_typeof(tw)) && jl_types_equal(pi, tw)) { - if (jl_is_vararg_type(iparams[i])) { - tw = jl_wrap_vararg(tw, jl_tparam1(jl_unwrap_unionall(iparams[i]))); - JL_GC_PUSH1(&tw); - tw = jl_rewrap_unionall(tw, iparams[i]); - JL_GC_POP(); - } + // This would require some special handling, but is never used at + // the moment. + assert(!jl_is_vararg_type(iparams[i])); iparams[i] = tw; if (p) jl_gc_wb(p, tw); } From d778a3d4baa17a6a9063aba06ec84510dcc3412a Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Mon, 13 Jul 2020 21:19:34 -0400 Subject: [PATCH 2/2] Move `instance` initialization into jl_uninitialized_datatype Otherwise it's illegal to hold onto this object without having set ->instance, which is just asking for crashes. Fixes #36649. --- src/datatype.c | 3 ++- src/dump.c | 1 - src/jltypes.c | 6 ------ 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/datatype.c b/src/datatype.c index c6544eca16606..73b194468de37 100644 --- a/src/datatype.c +++ b/src/datatype.c @@ -98,6 +98,8 @@ jl_datatype_t *jl_new_uninitialized_datatype(void) t->has_concrete_subtype = 1; t->layout = NULL; t->names = NULL; + t->types = NULL; + t->instance = NULL; return t; } @@ -554,7 +556,6 @@ JL_DLLEXPORT jl_datatype_t *jl_new_datatype( t->abstract = abstract; t->mutabl = mutabl; t->ninitialized = ninitialized; - t->instance = NULL; t->size = 0; t->name = NULL; diff --git a/src/dump.c b/src/dump.c index 7d83ccac3610c..56c8e6129c16e 100644 --- a/src/dump.c +++ b/src/dump.c @@ -1179,7 +1179,6 @@ static jl_value_t *jl_deserialize_datatype(jl_serializer_state *s, int pos, jl_v assert(pos == backref_list.len - 1 && "nothing should have been deserialized since assigning pos"); backref_list.items[pos] = dt; dt->size = size; - dt->instance = NULL; dt->abstract = flags & 1; dt->mutabl = (flags >> 1) & 1; int has_layout = (flags >> 2) & 1; diff --git a/src/jltypes.c b/src/jltypes.c index 988f8bd75e123..94d921cc8de0e 100644 --- a/src/jltypes.c +++ b/src/jltypes.c @@ -1416,7 +1416,6 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value } ndt->mutabl = dt->mutabl; ndt->abstract = dt->abstract; - ndt->instance = NULL; ndt->size = 0; jl_precompute_memoized_dt(ndt, cacheable); if (istuple) @@ -1912,7 +1911,6 @@ void jl_init_types(void) JL_GC_DISABLED jl_any_type, jl_any_type, jl_any_type, jl_any_type, jl_any_type, jl_any_type, jl_any_type, jl_any_type, jl_any_type); - jl_datatype_type->instance = NULL; jl_datatype_type->abstract = 0; // NOTE: types are not actually mutable, but we want to ensure they are heap-allocated with stable addresses jl_datatype_type->mutabl = 1; @@ -1931,7 +1929,6 @@ void jl_init_types(void) JL_GC_DISABLED jl_typename_type->types = jl_svec(9, jl_symbol_type, jl_any_type, jl_simplevector_type, jl_type_type, jl_simplevector_type, jl_simplevector_type, jl_any_type, jl_any_type, jl_any_type); - jl_typename_type->instance = NULL; jl_typename_type->abstract = 0; jl_typename_type->mutabl = 1; jl_typename_type->ninitialized = 2; @@ -1951,7 +1948,6 @@ void jl_init_types(void) JL_GC_DISABLED jl_any_type, jl_any_type/*module*/, jl_any_type/*any vector*/, jl_any_type/*long*/, jl_any_type/*int32*/, jl_any_type/*uint8*/, jl_any_type/*uint8*/); - jl_methtable_type->instance = NULL; jl_methtable_type->abstract = 0; jl_methtable_type->mutabl = 1; jl_methtable_type->ninitialized = 5; @@ -1964,7 +1960,6 @@ void jl_init_types(void) JL_GC_DISABLED jl_symbol_type->parameters = jl_emptysvec; jl_symbol_type->name->names = jl_emptysvec; jl_symbol_type->types = jl_emptysvec; - jl_symbol_type->instance = NULL; jl_symbol_type->size = 0; jl_symbol_type->abstract = 0; jl_symbol_type->mutabl = 1; @@ -1978,7 +1973,6 @@ void jl_init_types(void) JL_GC_DISABLED jl_simplevector_type->parameters = jl_emptysvec; jl_simplevector_type->name->names = jl_emptysvec; jl_simplevector_type->types = jl_emptysvec; - jl_simplevector_type->instance = NULL; jl_simplevector_type->abstract = 0; jl_simplevector_type->mutabl = 1; jl_simplevector_type->ninitialized = 0;