Skip to content

Commit 47e0612

Browse files
byrootbyroot
committed
class.c: Make cvc_tbl a managed object
[Bug #21952] Solves the double-free or use after-free concern with boxes. Now entries can safely be used for copy-on-write. Also is likely necessary to make it save to read cvar from secondary ractors, as allowed since: ab32c0e
1 parent 5658d5f commit 47e0612

14 files changed

Lines changed: 166 additions & 125 deletions

File tree

‎class.c‎

Lines changed: 22 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,6 @@ static void rb_class_remove_from_super_subclasses(VALUE klass);
8484
static void rb_class_remove_from_module_subclasses(VALUE klass);
8585
static void rb_class_classext_free_subclasses(rb_classext_t *ext);
8686

87-
static enum rb_id_table_iterator_result
88-
cvar_table_free_i(VALUE value, void *ctx)
89-
{
90-
struct rb_cvar_class_tbl_entry *entry = (struct rb_cvar_class_tbl_entry *)value;
91-
SIZED_FREE(entry);
92-
return ID_TABLE_CONTINUE;
93-
}
94-
9587
rb_classext_t *
9688
rb_class_unlink_classext(VALUE klass, const rb_box_t *box)
9789
{
@@ -114,11 +106,6 @@ rb_class_classext_free(VALUE klass, rb_classext_t *ext, bool is_prime)
114106
rb_free_const_table(tbl);
115107
}
116108

117-
if ((tbl = RCLASSEXT_CVC_TBL(ext)) != NULL) {
118-
rb_id_table_foreach_values(tbl, cvar_table_free_i, NULL);
119-
rb_id_table_free(tbl);
120-
}
121-
122109
if (is_prime) {
123110
rb_class_remove_from_super_subclasses(klass);
124111
rb_class_classext_free_subclasses(ext);
@@ -254,33 +241,6 @@ duplicate_classext_m_tbl(struct rb_id_table *orig, VALUE klass, bool init_missin
254241
return tbl;
255242
}
256243

257-
static enum rb_id_table_iterator_result
258-
duplicate_classext_cvc_tbl_i(ID key, VALUE value, void *data)
259-
{
260-
struct rb_id_table *tbl = (struct rb_id_table *)data;
261-
struct rb_cvar_class_tbl_entry *cvc_entry = (struct rb_cvar_class_tbl_entry *)value;
262-
struct rb_cvar_class_tbl_entry *copy = ALLOC(struct rb_cvar_class_tbl_entry);
263-
MEMCPY(copy, cvc_entry, struct rb_cvar_class_tbl_entry, 1);
264-
rb_id_table_insert(tbl, key, (VALUE)copy);
265-
return ID_TABLE_CONTINUE;
266-
}
267-
268-
static struct rb_id_table *
269-
duplicate_classext_cvc_tbl(struct rb_id_table *orig, bool init_missing)
270-
{
271-
struct rb_id_table *tbl;
272-
273-
if (!orig) {
274-
if (init_missing)
275-
return rb_id_table_create(0);
276-
else
277-
return NULL;
278-
}
279-
tbl = rb_id_table_create(rb_id_table_size(orig));
280-
rb_id_table_foreach(orig, duplicate_classext_cvc_tbl_i, tbl);
281-
return tbl;
282-
}
283-
284244
static rb_const_entry_t *
285245
duplicate_classext_const_entry(rb_const_entry_t *src, VALUE klass)
286246
{
@@ -414,7 +374,14 @@ rb_class_duplicate_classext(rb_classext_t *orig, VALUE klass, const rb_box_t *bo
414374
* RCLASSEXT_CC_TBL(copy) = NULL
415375
*/
416376

417-
RCLASSEXT_CVC_TBL(ext) = duplicate_classext_cvc_tbl(RCLASSEXT_CVC_TBL(orig), dup_iclass);
377+
VALUE cvc_table = RCLASSEXT_CVC_TBL(orig);
378+
if (cvc_table) {
379+
cvc_table = rb_marked_id_table_dup(cvc_table);
380+
}
381+
else if (dup_iclass) {
382+
cvc_table = rb_marked_id_table_new(2);
383+
}
384+
RB_OBJ_WRITE(klass, &RCLASSEXT_CVC_TBL(ext), cvc_table);
418385

419386
// Subclasses/back-pointers are only in the prime classext.
420387

@@ -981,9 +948,15 @@ class_init_copy_check(VALUE clone, VALUE orig)
981948

982949
struct cvc_table_copy_ctx {
983950
VALUE clone;
984-
struct rb_id_table * new_table;
951+
VALUE new_table;
985952
};
986953

954+
static struct rb_cvar_class_tbl_entry *
955+
cvc_table_entry_alloc(void)
956+
{
957+
return (struct rb_cvar_class_tbl_entry *)SHAREABLE_IMEMO_NEW(struct rb_cvar_class_tbl_entry, imemo_cvar_entry, 0);
958+
}
959+
987960
static enum rb_id_table_iterator_result
988961
cvc_table_copy(ID id, VALUE val, void *data)
989962
{
@@ -993,13 +966,11 @@ cvc_table_copy(ID id, VALUE val, void *data)
993966

994967
struct rb_cvar_class_tbl_entry *ent;
995968

996-
ent = ALLOC(struct rb_cvar_class_tbl_entry);
997-
ent->class_value = ctx->clone;
998-
ent->cref = orig_entry->cref;
969+
ent = cvc_table_entry_alloc();
970+
RB_OBJ_WRITE((VALUE)ent, &ent->class_value, ctx->clone);
971+
RB_OBJ_WRITE(ctx->clone, &ent->cref, orig_entry->cref);
999972
ent->global_cvar_state = orig_entry->global_cvar_state;
1000-
rb_id_table_insert(ctx->new_table, id, (VALUE)ent);
1001-
1002-
RB_OBJ_WRITTEN(ctx->clone, Qundef, ent->cref);
973+
rb_marked_id_table_insert(ctx->new_table, id, (VALUE)ent);
1003974

1004975
return ID_TABLE_CONTINUE;
1005976
}
@@ -1012,13 +983,13 @@ copy_tables(VALUE clone, VALUE orig)
1012983
RCLASS_WRITE_CONST_TBL(clone, 0, false);
1013984
}
1014985
if (RCLASS_CVC_TBL(orig)) {
1015-
struct rb_id_table *rb_cvc_tbl = RCLASS_CVC_TBL(orig);
1016-
struct rb_id_table *rb_cvc_tbl_dup = rb_id_table_create(rb_id_table_size(rb_cvc_tbl));
986+
VALUE rb_cvc_tbl = RCLASS_CVC_TBL(orig);
987+
VALUE rb_cvc_tbl_dup = rb_marked_id_table_new(rb_marked_id_table_size(rb_cvc_tbl));
1017988

1018989
struct cvc_table_copy_ctx ctx;
1019990
ctx.clone = clone;
1020991
ctx.new_table = rb_cvc_tbl_dup;
1021-
rb_id_table_foreach(rb_cvc_tbl, cvc_table_copy, &ctx);
992+
rb_marked_id_table_foreach(rb_cvc_tbl, cvc_table_copy, &ctx);
1022993
RCLASS_WRITE_CVC_TBL(clone, rb_cvc_tbl_dup);
1023994
}
1024995
rb_id_table_free(RCLASS_M_TBL(clone));

‎debug_counter.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,7 @@ RB_DEBUG_COUNTER(obj_imemo_ment)
305305
RB_DEBUG_COUNTER(obj_imemo_iseq)
306306
RB_DEBUG_COUNTER(obj_imemo_env)
307307
RB_DEBUG_COUNTER(obj_imemo_tmpbuf)
308+
RB_DEBUG_COUNTER(obj_imemo_cvar_entry)
308309
RB_DEBUG_COUNTER(obj_imemo_cref)
309310
RB_DEBUG_COUNTER(obj_imemo_svar)
310311
RB_DEBUG_COUNTER(obj_imemo_throw_data)

‎ext/objspace/objspace.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,7 @@ count_imemo_objects(int argc, VALUE *argv, VALUE self)
451451
INIT_IMEMO_TYPE_ID(imemo_ment);
452452
INIT_IMEMO_TYPE_ID(imemo_iseq);
453453
INIT_IMEMO_TYPE_ID(imemo_tmpbuf);
454+
INIT_IMEMO_TYPE_ID(imemo_cvar_entry);
454455
INIT_IMEMO_TYPE_ID(imemo_callinfo);
455456
INIT_IMEMO_TYPE_ID(imemo_callcache);
456457
INIT_IMEMO_TYPE_ID(imemo_constcache);

‎gc.c‎

Lines changed: 4 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,6 +1302,7 @@ rb_gc_imemo_needs_cleanup_p(VALUE obj)
13021302
case imemo_svar:
13031303
case imemo_callcache:
13041304
case imemo_throw_data:
1305+
case imemo_cvar_entry:
13051306
return false;
13061307

13071308
case imemo_env:
@@ -2522,9 +2523,6 @@ classext_memsize(rb_classext_t *ext, bool prime, VALUE box_value, void *arg)
25222523
if (RCLASSEXT_M_TBL(ext)) {
25232524
s += rb_id_table_memsize(RCLASSEXT_M_TBL(ext));
25242525
}
2525-
if (RCLASSEXT_CVC_TBL(ext)) {
2526-
s += rb_id_table_memsize(RCLASSEXT_CVC_TBL(ext));
2527-
}
25282526
if (RCLASSEXT_CONST_TBL(ext)) {
25292527
s += rb_id_table_memsize(RCLASSEXT_CONST_TBL(ext));
25302528
}
@@ -3046,26 +3044,6 @@ mark_const_tbl(rb_objspace_t *objspace, struct rb_id_table *tbl)
30463044
rb_id_table_foreach_values(tbl, mark_const_entry_i, objspace);
30473045
}
30483046

3049-
static enum rb_id_table_iterator_result
3050-
mark_cvc_tbl_i(VALUE cvc_entry, void *objspace)
3051-
{
3052-
struct rb_cvar_class_tbl_entry *entry;
3053-
3054-
entry = (struct rb_cvar_class_tbl_entry *)cvc_entry;
3055-
3056-
RUBY_ASSERT(entry->cref == 0 || (BUILTIN_TYPE((VALUE)entry->cref) == T_IMEMO && IMEMO_TYPE_P(entry->cref, imemo_cref)));
3057-
gc_mark_internal((VALUE)entry->cref);
3058-
3059-
return ID_TABLE_CONTINUE;
3060-
}
3061-
3062-
static void
3063-
mark_cvc_tbl(rb_objspace_t *objspace, struct rb_id_table *tbl)
3064-
{
3065-
if (!tbl) return;
3066-
rb_id_table_foreach_values(tbl, mark_cvc_tbl_i, objspace);
3067-
}
3068-
30693047
#if STACK_GROW_DIRECTION < 0
30703048
#define GET_STACK_BOUNDS(start, end, appendix) ((start) = STACK_END, (end) = STACK_START)
30713049
#elif STACK_GROW_DIRECTION > 0
@@ -3310,14 +3288,14 @@ gc_mark_classext_module(rb_classext_t *ext, bool prime, VALUE box_value, void *a
33103288
if (!rb_gc_checking_shareable()) {
33113289
// unshareable
33123290
gc_mark_internal(RCLASSEXT_FIELDS_OBJ(ext));
3291+
gc_mark_internal(RCLASSEXT_CVC_TBL(ext));
33133292
}
33143293

33153294
if (!RCLASSEXT_SHARED_CONST_TBL(ext) && RCLASSEXT_CONST_TBL(ext)) {
33163295
mark_const_tbl(objspace, RCLASSEXT_CONST_TBL(ext));
33173296
}
33183297
mark_m_tbl(objspace, RCLASSEXT_CALLABLE_M_TBL(ext));
33193298
gc_mark_internal(RCLASSEXT_CC_TBL(ext));
3320-
mark_cvc_tbl(objspace, RCLASSEXT_CVC_TBL(ext));
33213299
gc_mark_internal(RCLASSEXT_CLASSPATH(ext));
33223300
}
33233301

@@ -3961,29 +3939,6 @@ update_m_tbl(void *objspace, struct rb_id_table *tbl)
39613939
}
39623940
}
39633941

3964-
static enum rb_id_table_iterator_result
3965-
update_cvc_tbl_i(VALUE cvc_entry, void *objspace)
3966-
{
3967-
struct rb_cvar_class_tbl_entry *entry;
3968-
3969-
entry = (struct rb_cvar_class_tbl_entry *)cvc_entry;
3970-
3971-
if (entry->cref) {
3972-
TYPED_UPDATE_IF_MOVED(objspace, rb_cref_t *, entry->cref);
3973-
}
3974-
3975-
entry->class_value = gc_location_internal(objspace, entry->class_value);
3976-
3977-
return ID_TABLE_CONTINUE;
3978-
}
3979-
3980-
static void
3981-
update_cvc_tbl(void *objspace, struct rb_id_table *tbl)
3982-
{
3983-
if (!tbl) return;
3984-
rb_id_table_foreach_values(tbl, update_cvc_tbl_i, objspace);
3985-
}
3986-
39873942
static enum rb_id_table_iterator_result
39883943
update_const_tbl_i(VALUE value, void *objspace)
39893944
{
@@ -4058,7 +4013,7 @@ update_classext(rb_classext_t *ext, bool is_prime, VALUE box_value, void *arg)
40584013
update_const_tbl(objspace, RCLASSEXT_CONST_TBL(ext));
40594014
}
40604015
UPDATE_IF_MOVED(objspace, RCLASSEXT_CC_TBL(ext));
4061-
update_cvc_tbl(objspace, RCLASSEXT_CVC_TBL(ext));
4016+
UPDATE_IF_MOVED(objspace, RCLASSEXT_CVC_TBL(ext));
40624017
update_superclasses(objspace, ext);
40634018
update_subclasses(objspace, ext);
40644019

@@ -4077,6 +4032,7 @@ update_iclass_classext(rb_classext_t *ext, bool is_prime, VALUE box_value, void
40774032
update_m_tbl(objspace, RCLASSEXT_M_TBL(ext));
40784033
update_m_tbl(objspace, RCLASSEXT_CALLABLE_M_TBL(ext));
40794034
UPDATE_IF_MOVED(objspace, RCLASSEXT_CC_TBL(ext));
4035+
UPDATE_IF_MOVED(objspace, RCLASSEXT_CVC_TBL(ext));
40804036
update_subclasses(objspace, ext);
40814037

40824038
update_classext_values(objspace, ext, true);

‎id_table.c‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,3 +431,82 @@ rb_managed_id_table_delete(VALUE table, ID id)
431431
{
432432
return rb_id_table_delete(managed_id_table_ptr(table), id);
433433
}
434+
435+
static enum rb_id_table_iterator_result
436+
marked_id_table_mark_i(VALUE val, void *data)
437+
{
438+
rb_gc_mark_movable(val);
439+
return ID_TABLE_CONTINUE;
440+
}
441+
442+
static void
443+
marked_id_table_mark(void *ptr)
444+
{
445+
struct rb_id_table *tbl = (struct rb_id_table *)ptr;
446+
rb_id_table_foreach_values(tbl, marked_id_table_mark_i, NULL);
447+
}
448+
449+
static enum rb_id_table_iterator_result
450+
marked_id_table_compact_check_i(VALUE value, void *data)
451+
{
452+
if (rb_gc_location(value) != value) {
453+
return ID_TABLE_REPLACE;
454+
}
455+
return ID_TABLE_CONTINUE;
456+
}
457+
458+
static enum rb_id_table_iterator_result
459+
marked_id_table_compact_replace_i(VALUE *value, void *data, int existing)
460+
{
461+
*value = rb_gc_location(*value);
462+
return ID_TABLE_CONTINUE;
463+
}
464+
465+
static void
466+
marked_id_table_compact(void *ptr)
467+
{
468+
struct rb_id_table *tbl = (struct rb_id_table *)ptr;
469+
rb_id_table_foreach_values_with_replace(tbl, marked_id_table_compact_check_i, marked_id_table_compact_replace_i, NULL);
470+
}
471+
472+
const rb_data_type_t rb_marked_id_table_type = {
473+
.wrap_struct_name = "VM/marked_id_table",
474+
.function = {
475+
.dmark = marked_id_table_mark,
476+
.dfree = managed_id_table_free,
477+
.dsize = managed_id_table_memsize,
478+
.dcompact = marked_id_table_compact,
479+
},
480+
.parent = &rb_managed_id_table_type,
481+
.flags = RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED | RUBY_TYPED_EMBEDDABLE,
482+
};
483+
484+
VALUE
485+
rb_marked_id_table_new(size_t capa)
486+
{
487+
return rb_managed_id_table_create(&rb_marked_id_table_type, capa);
488+
}
489+
490+
int
491+
rb_marked_id_table_insert(VALUE table, ID id, VALUE val)
492+
{
493+
int result = rb_managed_id_table_insert(table, id, val);
494+
RB_OBJ_WRITTEN(table, Qundef, val);
495+
return result;
496+
}
497+
498+
static enum rb_id_table_iterator_result
499+
marked_id_table_dup_i(VALUE val, void *data)
500+
{
501+
VALUE new_table = (VALUE)data;
502+
RB_OBJ_WRITTEN(new_table, Qundef, val);
503+
return ID_TABLE_CONTINUE;
504+
}
505+
506+
VALUE
507+
rb_marked_id_table_dup(VALUE old_table)
508+
{
509+
VALUE new_table = rb_managed_id_table_dup(old_table);
510+
rb_managed_id_table_foreach_values(new_table, marked_id_table_dup_i, (void *)new_table);
511+
return new_table;
512+
}

‎id_table.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,16 @@ int rb_managed_id_table_delete(VALUE table, ID id);
5454

5555
extern const rb_data_type_t rb_managed_id_table_type;
5656

57+
VALUE rb_marked_id_table_new(size_t capa);
58+
int rb_marked_id_table_insert(VALUE table, ID id, VALUE val);
59+
VALUE rb_marked_id_table_dup(VALUE table);
60+
61+
// alisases
62+
#define rb_marked_id_table_size rb_managed_id_table_size
63+
#define rb_marked_id_table_lookup rb_managed_id_table_lookup
64+
#define rb_marked_id_table_foreach rb_managed_id_table_foreach
65+
#define rb_marked_id_table_foreach_values rb_managed_id_table_foreach_values
66+
5767
RUBY_SYMBOL_EXPORT_BEGIN
5868
size_t rb_id_table_size(const struct rb_id_table *tbl);
5969
RUBY_SYMBOL_EXPORT_END

‎imemo.c‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ rb_imemo_name(enum imemo_type type)
2929
IMEMO_NAME(svar);
3030
IMEMO_NAME(throw_data);
3131
IMEMO_NAME(tmpbuf);
32+
IMEMO_NAME(cvar_entry);
3233
IMEMO_NAME(fields);
3334
#undef IMEMO_NAME
3435
}
@@ -262,6 +263,8 @@ rb_imemo_memsize(VALUE obj)
262263
case imemo_tmpbuf:
263264
size += ((rb_imemo_tmpbuf_t *)obj)->size;
264265

266+
break;
267+
case imemo_cvar_entry:
265268
break;
266269
case imemo_fields:
267270
if (FL_TEST_RAW(obj, OBJ_FIELD_HEAP)) {
@@ -510,6 +513,12 @@ rb_imemo_mark_and_move(VALUE obj, bool reference_updating)
510513

511514
break;
512515
}
516+
case imemo_cvar_entry: {
517+
struct rb_cvar_class_tbl_entry *ent = (struct rb_cvar_class_tbl_entry *)obj;
518+
rb_gc_mark_and_move(&ent->class_value);
519+
rb_gc_mark_and_move((VALUE *)&ent->cref);
520+
break;
521+
}
513522
case imemo_fields: {
514523
rb_gc_mark_and_move((VALUE *)&RBASIC(obj)->klass);
515524

@@ -641,6 +650,10 @@ rb_imemo_free(VALUE obj)
641650
ruby_xfree_sized(((rb_imemo_tmpbuf_t *)obj)->ptr, ((rb_imemo_tmpbuf_t *)obj)->size);
642651
RB_DEBUG_COUNTER_INC(obj_imemo_tmpbuf);
643652

653+
break;
654+
case imemo_cvar_entry:
655+
RB_DEBUG_COUNTER_INC(obj_imemo_cvar_entry);
656+
644657
break;
645658
case imemo_fields:
646659
imemo_fields_free(IMEMO_OBJ_FIELDS(obj));

0 commit comments

Comments
 (0)