Skip to content

Commit

Permalink
jl_insert_method_instances: check method validity
Browse files Browse the repository at this point in the history 
This fixes a separate bug in which #43990 should have checked
that the method had not been deleted. Tests passed formerly
simply because we weren't caching external CodeInstances
that inferred down to a `Const`; fixing that exposed the bug.
This bug has been exposed since merging #43990 for non-`Const`
inference, and would affect Revise etc.
  • Loading branch information
@timholy
timholy committed Jul 13, 2022
1 parent de8b202 commit f591258
Showing 1 changed file with 58 additions and 47 deletions.
105 changes: 58 additions & 47 deletions src/dump.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2313,63 +2313,74 @@ static void jl_insert_method_instances(jl_array_t *list)
size_t world = jl_atomic_load_acquire(&jl_world_counter);
for (i = 0; i < l; i++) {
jl_method_instance_t *mi = (jl_method_instance_t*)jl_array_ptr_ref(list, i);
int valid = 1;
assert(jl_is_method_instance(mi));
if (jl_is_method(mi->def.method)) {
// Is this still the method we'd be calling?
jl_methtable_t *mt = jl_method_table_for(mi->specTypes);
struct jl_typemap_assoc search = {(jl_value_t*)mi->specTypes, world, NULL, 0, ~(size_t)0};
jl_typemap_entry_t *entry = jl_typemap_assoc_by_type(mt->defs, &search, /*offs*/0, /*subtype*/1);
if (entry) {
jl_value_t *mworld = entry->func.value;
if (jl_is_method(mworld) && mi->def.method != (jl_method_t*)mworld && jl_type_morespecific(((jl_method_t*)mworld)->sig, mi->def.method->sig)) {
// There's still a chance this is valid, if any caller made this via `invoke` and the invoke-signature is still valid
assert(mi->backedges); // should not be NULL if it's on `list`
jl_value_t *invokeTypes;
jl_method_instance_t *caller;
size_t jins = 0, j0, j = 0, nbe = jl_array_len(mi->backedges);
while (j < nbe) {
j0 = j;
j = get_next_backedge(mi->backedges, j, &invokeTypes, &caller);
if (invokeTypes) {
struct jl_typemap_assoc search = {invokeTypes, world, NULL, 0, ~(size_t)0};
entry = jl_typemap_assoc_by_type(mt->defs, &search, /*offs*/0, /*subtype*/1);
if (entry) {
jl_value_t *imworld = entry->func.value;
if (jl_is_method(imworld) && mi->def.method == (jl_method_t*)imworld) {
// this one is OK
// in case we deleted some earlier ones, move this earlier
for (; j0 < j; jins++, j0++) {
jl_array_ptr_set(mi->backedges, jins, jl_array_ptr_ref(mi->backedges, j0));
jl_method_t *m = mi->def.method;
if (m->deleted_world != ~(size_t)0) {
// The method we depended on has been deleted, invalidate
valid = 0;
} else {
// Is this still the method we'd be calling?
jl_methtable_t *mt = jl_method_table_for(mi->specTypes);
struct jl_typemap_assoc search = {(jl_value_t*)mi->specTypes, world, NULL, 0, ~(size_t)0};
jl_typemap_entry_t *entry = jl_typemap_assoc_by_type(mt->defs, &search, /*offs*/0, /*subtype*/1);
if (entry) {
jl_value_t *mworld = entry->func.value;
if (jl_is_method(mworld) && mi->def.method != (jl_method_t*)mworld && jl_type_morespecific(((jl_method_t*)mworld)->sig, mi->def.method->sig)) {
// There's still a chance this is valid, if any caller made this via `invoke` and the invoke-signature is still valid
assert(mi->backedges); // should not be NULL if it's on `list`
jl_value_t *invokeTypes;
jl_method_instance_t *caller;
size_t jins = 0, j0, j = 0, nbe = jl_array_len(mi->backedges);
while (j < nbe) {
j0 = j;
j = get_next_backedge(mi->backedges, j, &invokeTypes, &caller);
if (invokeTypes) {
struct jl_typemap_assoc search = {invokeTypes, world, NULL, 0, ~(size_t)0};
entry = jl_typemap_assoc_by_type(mt->defs, &search, /*offs*/0, /*subtype*/1);
if (entry) {
jl_value_t *imworld = entry->func.value;
if (jl_is_method(imworld) && mi->def.method == (jl_method_t*)imworld) {
// this one is OK
// in case we deleted some earlier ones, move this earlier
for (; j0 < j; jins++, j0++) {
jl_array_ptr_set(mi->backedges, jins, jl_array_ptr_ref(mi->backedges, j0));
}
continue;
}
continue;
}
}
invalidate_backedges(&remove_code_instance_from_validation, caller, world, "jl_insert_method_instance");
// The codeinst of this mi haven't yet been removed
jl_code_instance_t *codeinst = caller->cache;
while (codeinst) {
remove_code_instance_from_validation(codeinst);
codeinst = codeinst->next;
}
}
invalidate_backedges(&remove_code_instance_from_validation, caller, world, "jl_insert_method_instance");
// The codeinst of this mi haven't yet been removed
jl_code_instance_t *codeinst = caller->cache;
while (codeinst) {
remove_code_instance_from_validation(codeinst);
codeinst = codeinst->next;
}
}
jl_array_del_end(mi->backedges, j - jins);
if (jins == 0) {
// None of the callers were valid, so invalidate `mi` too
jl_array_uint8_set(valids, i, 0);
invalidate_backedges(&remove_code_instance_from_validation, mi, world, "jl_insert_method_instance");
jl_code_instance_t *codeinst = mi->cache;
while (codeinst) {
remove_code_instance_from_validation(codeinst);
codeinst = codeinst->next;
}
if (_jl_debug_method_invalidation) {
jl_array_ptr_1d_push(_jl_debug_method_invalidation, mworld);
jl_array_ptr_1d_push(_jl_debug_method_invalidation, jl_cstr_to_string("jl_method_table_insert")); // GC disabled
jl_array_del_end(mi->backedges, j - jins);
if (jins == 0) {
m = (jl_method_t*)mworld;
valid = 0;
}
}
}
}
if (!valid) {
// None of the callers were valid, so invalidate `mi` too
jl_array_uint8_set(valids, i, 0);
invalidate_backedges(&remove_code_instance_from_validation, mi, world, "jl_insert_method_instance");
jl_code_instance_t *codeinst = mi->cache;
while (codeinst) {
remove_code_instance_from_validation(codeinst);
codeinst = codeinst->next;
}
if (_jl_debug_method_invalidation) {
jl_array_ptr_1d_push(_jl_debug_method_invalidation, (jl_value_t*)m);
jl_array_ptr_1d_push(_jl_debug_method_invalidation, jl_cstr_to_string("jl_method_table_insert")); // GC disabled
}
}
}
}
// While it's tempting to just remove the invalidated MIs altogether,
Expand Down

0 comments on commit f591258

Please sign in to comment.