`mandatory_only_cme` should not be in `def`

`def` (`rb_method_definition_t`) is shared by multiple callable
method entries (cme, `rb_callable_method_entry_t`).

There are two issues:

* old -> young reference: `cme1->def->mandatory_only_cme = monly_cme`
  if `cme1` is young and `monly_cme` is young, there is no problem.
  Howevr, another old `cme2` can refer `def`, in this case, old `cme2`
  points young `monly_cme` and it violates gengc assumption.
* cme can have different `defined_class` but `monly_cme` only has
  one `defined_class`. It does not make sense and `monly_cme`
  should be created for a cme (not `def`).

To solve these issues, this patch allocates `monly_cme` per `cme`.
`cme` does not have another room to store a pointer to the `monly_cme`,
so this patch introduces `overloaded_cme_table`, which is weak key map
`[cme] -> [monly_cme]`.

`def::body::iseqptr::monly_cme` is deleted.

The first issue is reported by Alan Wu.
This commit is contained in:
Koichi Sasada 2021-12-21 06:03:51 +09:00
Родитель 711342d935
Коммит df48db987d
6 изменённых файлов: 166 добавлений и 36 удалений

17
gc.c
Просмотреть файл

@ -6363,6 +6363,8 @@ rb_mark_hash(st_table *tbl)
mark_st(&rb_objspace, tbl);
}
const rb_callable_method_entry_t *rb_vm_lookup_overloaded_cme(const rb_callable_method_entry_t *cme);
static void
mark_method_entry(rb_objspace_t *objspace, const rb_method_entry_t *me)
{
@ -6376,7 +6378,13 @@ mark_method_entry(rb_objspace_t *objspace, const rb_method_entry_t *me)
case VM_METHOD_TYPE_ISEQ:
if (def->body.iseq.iseqptr) gc_mark(objspace, (VALUE)def->body.iseq.iseqptr);
gc_mark(objspace, (VALUE)def->body.iseq.cref);
if (def->body.iseq.mandatory_only_cme) gc_mark(objspace, (VALUE)def->body.iseq.mandatory_only_cme);
if (def->iseq_overload && me->defined_class) { // cme
const rb_callable_method_entry_t *monly_cme = rb_vm_lookup_overloaded_cme((const rb_callable_method_entry_t *)me);
if (monly_cme) {
gc_mark(objspace, (VALUE)monly_cme);
gc_mark_and_pin(objspace, (VALUE)me);
}
}
break;
case VM_METHOD_TYPE_ATTRSET:
case VM_METHOD_TYPE_IVAR:
@ -9599,9 +9607,6 @@ gc_ref_update_method_entry(rb_objspace_t *objspace, rb_method_entry_t *me)
TYPED_UPDATE_IF_MOVED(objspace, rb_iseq_t *, def->body.iseq.iseqptr);
}
TYPED_UPDATE_IF_MOVED(objspace, rb_cref_t *, def->body.iseq.cref);
if (def->body.iseq.mandatory_only_cme) {
TYPED_UPDATE_IF_MOVED(objspace, rb_callable_method_entry_t *, def->body.iseq.mandatory_only_cme);
}
break;
case VM_METHOD_TYPE_ATTRSET:
case VM_METHOD_TYPE_IVAR:
@ -10108,6 +10113,9 @@ gc_ref_update(void *vstart, void *vend, size_t stride, rb_objspace_t * objspace,
extern rb_symbols_t ruby_global_symbols;
#define global_symbols ruby_global_symbols
st_table *rb_vm_overloaded_cme_table(void);
static void
gc_update_references(rb_objspace_t *objspace)
{
@ -10143,6 +10151,7 @@ gc_update_references(rb_objspace_t *objspace)
gc_update_table_refs(objspace, objspace->id_to_obj_tbl);
gc_update_table_refs(objspace, global_symbols.str_sym);
gc_update_table_refs(objspace, finalizer_table);
gc_update_table_refs(objspace, rb_vm_overloaded_cme_table());
}
static VALUE

Просмотреть файл

@ -134,7 +134,6 @@ typedef struct rb_iseq_struct rb_iseq_t;
typedef struct rb_method_iseq_struct {
const rb_iseq_t * iseqptr; /*!< iseq pointer, should be separated from iseqval */
rb_cref_t * cref; /*!< class reference, should be marked */
const rb_callable_method_entry_t *mandatory_only_cme;
} rb_method_iseq_t; /* check rb_add_method_iseq() when modify the fields */
typedef struct rb_method_cfunc_struct {

Просмотреть файл

@ -725,4 +725,21 @@ class TestISeq < Test::Unit::TestCase
assert_equal at0, Time.public_send(:at, 0, 0)
RUBY
end
def test_mandatory_only_redef
assert_separately ['-W0'], <<~RUBY
r = Ractor.new{
Float(10)
module Kernel
undef Float
def Float(n)
:new
end
end
GC.start
Float(30)
}
assert_equal :new, r.take
RUBY
end
end

Просмотреть файл

@ -449,6 +449,10 @@ struct rb_class_cc_entries {
};
#if VM_CHECK_MODE > 0
const rb_callable_method_entry_t *rb_vm_lookup_overloaded_cme(const rb_callable_method_entry_t *cme);
void rb_vm_dump_overloaded_cme_table(void);
static inline bool
vm_ccs_p(const struct rb_class_cc_entries *ccs)
{
@ -459,15 +463,17 @@ static inline bool
vm_cc_check_cme(const struct rb_callcache *cc, const rb_callable_method_entry_t *cme)
{
if (vm_cc_cme(cc) == cme ||
(cme->def->iseq_overload && vm_cc_cme(cc) == cme->def->body.iseq.mandatory_only_cme)) {
(cme->def->iseq_overload && vm_cc_cme(cc) == rb_vm_lookup_overloaded_cme(cme))) {
return true;
}
else {
#if 1
fprintf(stderr, "iseq_overload:%d mandatory_only_cme:%p eq:%d\n",
(int)cme->def->iseq_overload,
(void *)cme->def->body.iseq.mandatory_only_cme,
vm_cc_cme(cc) == cme->def->body.iseq.mandatory_only_cme);
// debug print
fprintf(stderr, "iseq_overload:%d\n", (int)cme->def->iseq_overload);
rp(cme);
rp(vm_cc_cme(cc));
rb_vm_lookup_overloaded_cme(cme);
#endif
return false;
}

Просмотреть файл

@ -1766,7 +1766,7 @@ vm_ccs_verify(struct rb_class_cc_entries *ccs, ID mid, VALUE klass)
#ifndef MJIT_HEADER
static const rb_callable_method_entry_t *overloaded_cme(const rb_callable_method_entry_t *cme);
static const rb_callable_method_entry_t *check_overloaded_cme(const rb_callable_method_entry_t *cme, const struct rb_callinfo * const ci);
static const struct rb_callcache *
vm_search_cc(const VALUE klass, const struct rb_callinfo * const ci)
@ -1780,7 +1780,6 @@ vm_search_cc(const VALUE klass, const struct rb_callinfo * const ci)
if (rb_id_table_lookup(cc_tbl, mid, &ccs_data)) {
ccs = (struct rb_class_cc_entries *)ccs_data;
const int ccs_len = ccs->len;
VM_ASSERT(vm_ccs_verify(ccs, mid, klass));
if (UNLIKELY(METHOD_ENTRY_INVALIDATED(ccs->cme))) {
rb_vm_ccs_free(ccs);
@ -1788,6 +1787,8 @@ vm_search_cc(const VALUE klass, const struct rb_callinfo * const ci)
ccs = NULL;
}
else {
VM_ASSERT(vm_ccs_verify(ccs, mid, klass));
for (int i=0; i<ccs_len; i++) {
const struct rb_callinfo *ccs_ci = ccs->entries[i].ci;
const struct rb_callcache *ccs_cc = ccs->entries[i].cc;
@ -1852,15 +1853,8 @@ vm_search_cc(const VALUE klass, const struct rb_callinfo * const ci)
}
}
if (cme->def->iseq_overload &&
(vm_ci_flag(ci) & (VM_CALL_ARGS_SIMPLE)) &&
(int)vm_ci_argc(ci) == method_entry_iseqptr(cme)->body->param.lead_num
) {
// use alternative
cme = overloaded_cme(cme);
METHOD_ENTRY_CACHED_SET((struct rb_callable_method_entry_struct *)cme);
// rp(cme);
}
cme = check_overloaded_cme(cme, ci);
const struct rb_callcache *cc = vm_cc_new(klass, cme, vm_call_general);
vm_ccs_push(klass, ccs, ci, cc);

Просмотреть файл

@ -149,6 +149,8 @@ invalidate_negative_cache(ID mid)
static rb_method_entry_t *rb_method_entry_alloc(ID called_id, VALUE owner, VALUE defined_class, const rb_method_definition_t *def);
const rb_method_entry_t * rb_method_entry_clone(const rb_method_entry_t *src_me);
static const rb_callable_method_entry_t *complemented_callable_method_entry(VALUE klass, ID id);
static const rb_callable_method_entry_t *lookup_overloaded_cme(const rb_callable_method_entry_t *cme);
static void delete_overloaded_cme(const rb_callable_method_entry_t *cme);
static void
clear_method_cache_by_id_in_class(VALUE klass, ID mid)
@ -156,6 +158,7 @@ clear_method_cache_by_id_in_class(VALUE klass, ID mid)
VM_ASSERT(RB_TYPE_P(klass, T_CLASS) || RB_TYPE_P(klass, T_ICLASS));
if (rb_objspace_garbage_object_p(klass)) return;
RB_VM_LOCK_ENTER();
if (LIKELY(RCLASS_SUBCLASSES(klass) == NULL)) {
// no subclasses
// check only current class
@ -209,8 +212,12 @@ clear_method_cache_by_id_in_class(VALUE klass, ID mid)
vm_cme_invalidate((rb_callable_method_entry_t *)cme);
RB_DEBUG_COUNTER_INC(cc_invalidate_tree_cme);
if (cme->def->iseq_overload && cme->def->body.iseq.mandatory_only_cme) {
vm_cme_invalidate((rb_callable_method_entry_t *)cme->def->body.iseq.mandatory_only_cme);
if (cme->def->iseq_overload) {
rb_callable_method_entry_t *monly_cme = (rb_callable_method_entry_t *)lookup_overloaded_cme(cme);
if (monly_cme) {
vm_cme_invalidate(monly_cme);
delete_overloaded_cme(monly_cme);
}
}
}
@ -230,6 +237,7 @@ clear_method_cache_by_id_in_class(VALUE klass, ID mid)
invalidate_negative_cache(mid);
}
}
RB_VM_LOCK_LEAVE();
rb_yjit_method_lookup_change(klass, mid);
}
@ -388,6 +396,9 @@ rb_method_definition_release(rb_method_definition_t *def, int complemented)
void
rb_free_method_entry(const rb_method_entry_t *me)
{
if (me->def && me->def->iseq_overload) {
delete_overloaded_cme((const rb_callable_method_entry_t *)me);
}
rb_method_definition_release(me->def, METHOD_ENTRY_COMPLEMENTED(me));
}
@ -548,7 +559,6 @@ method_definition_reset(const rb_method_entry_t *me)
case VM_METHOD_TYPE_ISEQ:
RB_OBJ_WRITTEN(me, Qundef, def->body.iseq.iseqptr);
RB_OBJ_WRITTEN(me, Qundef, def->body.iseq.cref);
RB_OBJ_WRITTEN(me, Qundef, def->body.iseq.mandatory_only_cme);
break;
case VM_METHOD_TYPE_ATTRSET:
case VM_METHOD_TYPE_IVAR:
@ -911,19 +921,96 @@ rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibil
return me;
}
static const rb_callable_method_entry_t *
overloaded_cme(const rb_callable_method_entry_t *cme)
static rb_method_entry_t *rb_method_entry_alloc(ID called_id, VALUE owner, VALUE defined_class, const rb_method_definition_t *def);
static st_table *overloaded_cme_table;
st_table *
rb_vm_overloaded_cme_table(void)
{
VM_ASSERT(cme->def->iseq_overload);
VM_ASSERT(cme->def->type == VM_METHOD_TYPE_ISEQ);
VM_ASSERT(cme->def->body.iseq.iseqptr != NULL);
return overloaded_cme_table;
}
const rb_callable_method_entry_t *monly_cme = cme->def->body.iseq.mandatory_only_cme;
#if VM_CHECK_MODE > 0
static int
vm_dump_overloaded_cme_table(st_data_t key, st_data_t val, st_data_t dmy)
{
fprintf(stderr, "key: "); rp(key);
fprintf(stderr, "val: "); rp(val);
return ST_CONTINUE;
}
if (monly_cme && !METHOD_ENTRY_INVALIDATED(monly_cme)) {
// ok
void
rb_vm_dump_overloaded_cme_table(void)
{
fprintf(stderr, "== rb_vm_dump_overloaded_cme_table\n");
st_foreach(overloaded_cme_table, vm_dump_overloaded_cme_table, 0);
}
#endif
static int
lookup_overloaded_cme_i(st_data_t *key, st_data_t *value, st_data_t data, int existing)
{
if (existing) {
const rb_callable_method_entry_t *cme = (const rb_callable_method_entry_t *)*key;
const rb_callable_method_entry_t *monly_cme = (const rb_callable_method_entry_t *)*value;
const rb_callable_method_entry_t **ptr = (const rb_callable_method_entry_t **)data;
if (rb_objspace_garbage_object_p((VALUE)cme) ||
rb_objspace_garbage_object_p((VALUE)monly_cme) ||
METHOD_ENTRY_INVALIDATED(cme) ||
METHOD_ENTRY_INVALIDATED(monly_cme)) {
*ptr = NULL;
return ST_DELETE;
}
else {
*ptr = monly_cme;
}
}
return ST_STOP;
}
static const rb_callable_method_entry_t *
lookup_overloaded_cme(const rb_callable_method_entry_t *cme)
{
ASSERT_vm_locking();
const rb_callable_method_entry_t *monly_cme = NULL;
st_update(overloaded_cme_table, (st_data_t)cme, lookup_overloaded_cme_i, (st_data_t)&monly_cme);
if (monly_cme) {
return monly_cme;
}
else {
return NULL;
}
}
// used by gc.c
MJIT_FUNC_EXPORTED const rb_callable_method_entry_t *
rb_vm_lookup_overloaded_cme(const rb_callable_method_entry_t *cme)
{
return lookup_overloaded_cme(cme);
}
static void
delete_overloaded_cme(const rb_callable_method_entry_t *cme)
{
ASSERT_vm_locking();
st_delete(overloaded_cme_table, (st_data_t *)&cme, NULL);
}
static const rb_callable_method_entry_t *
get_overloaded_cme(const rb_callable_method_entry_t *cme)
{
const rb_callable_method_entry_t *monly_cme = lookup_overloaded_cme(cme);
if (monly_cme) {
return monly_cme;
}
else {
// create
rb_method_definition_t *def = rb_method_definition_create(VM_METHOD_TYPE_ISEQ, cme->def->original_id);
def->body.iseq.cref = cme->def->body.iseq.cref;
def->body.iseq.iseqptr = cme->def->body.iseq.iseqptr->body->mandatory_only_iseq;
@ -932,12 +1019,30 @@ overloaded_cme(const rb_callable_method_entry_t *cme)
cme->owner,
cme->defined_class,
def);
ASSERT_vm_locking();
st_insert(overloaded_cme_table, (st_data_t)cme, (st_data_t)me);
METHOD_ENTRY_VISI_SET(me, METHOD_ENTRY_VISI(cme));
RB_OBJ_WRITE(cme, &cme->def->body.iseq.mandatory_only_cme, me);
monly_cme = (rb_callable_method_entry_t *)me;
return (rb_callable_method_entry_t *)me;
}
}
static const rb_callable_method_entry_t *
check_overloaded_cme(const rb_callable_method_entry_t *cme, const struct rb_callinfo * const ci)
{
if (UNLIKELY(cme->def->iseq_overload) &&
(vm_ci_flag(ci) & (VM_CALL_ARGS_SIMPLE)) &&
(int)vm_ci_argc(ci) == method_entry_iseqptr(cme)->body->param.lead_num) {
VM_ASSERT(cme->def->type == VM_METHOD_TYPE_ISEQ); // iseq_overload is marked only on ISEQ methods
cme = get_overloaded_cme(cme);
VM_ASSERT(cme != NULL);
METHOD_ENTRY_CACHED_SET((struct rb_callable_method_entry_struct *)cme);
}
return monly_cme;
return cme;
}
#define CALL_METHOD_HOOK(klass, hook, mid) do { \
@ -2723,7 +2828,7 @@ obj_respond_to_missing(VALUE obj, VALUE mid, VALUE priv)
void
Init_Method(void)
{
//
overloaded_cme_table = st_init_numtable();
}
void