Skip to content

Commit e23a60b

Browse files
peterzhu2118kddnewton
authored andcommitted
Fix GC compaction crash when using local variables in eval
If we have local variables outside of the eval, the local variables names are IDs. We convert these IDs to char * using rb_id2name. However, these char * are actually Ruby strings, which may be embedded. This means that it is not safe to rb_id2name and call any potential GC entrypoints because if a GC compaction runs, the embedded string may move and the pointer will change. For example, if you compile with `-DRGENGC_CHECK_MODE=1`, then the following script will crash: GC.auto_compact = :empty GC.stress = true o = Object.new eval("def o.m(k: 0) k end") The crash message is: test.rb:6: [BUG] Local with constant_id 1 does not exist ruby 3.4.0dev (2024-12-17T18:34:57Z prism-local-compac.. 4343467) +PRISM [arm64-darwin24] -- C level backtrace information ------------------------------------------- miniruby(rb_print_backtrace+0x24) [0x10312fec4] vm_dump.c:823 miniruby(rb_print_backtrace) (null):0 miniruby(rb_vm_bugreport+0x2d4) [0x1031301b8] vm_dump.c:1155 miniruby(rb_bug_without_die_internal+0xa8) [0x102dd6a94] error.c:1097 miniruby(rb_bug+0x28) [0x102dd6b00] error.c:1115 miniruby(pm_lookup_local_index+0xec) [0x102d61e4c] prism_compile.c:1237 miniruby(pm_compile_node+0x45d0) [0x102d252f4] prism_compile.c:9334 miniruby(pm_compile_node+0x1864) [0x102d22588] prism_compile.c:8650 miniruby(pm_compile_node+0x65ec) [0x102d27310] prism_compile.c:9897 miniruby(pm_compile_scope_node+0x3008) [0x102d77bcc] prism_compile.c:6584 miniruby(pm_compile_node+0x5ec4) [0x102d26be8] prism_compile.c:9768 miniruby(pm_iseq_compile_node+0xac) [0x102d20bf0] prism_compile.c:10069 miniruby(pm_iseq_new_with_opt_try+0x2c) [0x102e7d088] iseq.c:1029 miniruby(rb_protect+0x108) [0x102dea9bc] eval.c:1033 miniruby(pm_iseq_new_with_opt+0x264) [0x102e7c444] iseq.c:1082 miniruby(pm_iseq_new_eval+0xec) [0x102e7c8e0] iseq.c:961 miniruby(pm_eval_make_iseq+0x594) [0x1031209cc] vm_eval.c:1770 miniruby(eval_make_iseq+0x54) [0x103120068] vm_eval.c:1799
1 parent 85f3ed8 commit e23a60b

File tree

2 files changed

+26
-2
lines changed

2 files changed

+26
-2
lines changed

test/ruby/test_eval.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,4 +636,19 @@ def test_syntax_error_no_memory_leak
636636
end
637637
end;
638638
end
639+
640+
def test_outer_local_variable_under_gc_compact_stress
641+
omit "compaction is not supported on this platform" unless GC.respond_to?(:compact)
642+
643+
assert_separately([], <<~RUBY)
644+
o = Object.new
645+
def o.m = 1
646+
647+
GC.verify_compaction_references(expand_heap: true, toward: :empty)
648+
649+
EnvUtil.under_gc_compact_stress do
650+
assert_equal(1, eval("o.m"))
651+
end
652+
RUBY
653+
end
639654
end

vm_eval.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1700,7 +1700,8 @@ pm_eval_make_iseq(VALUE src, VALUE fname, int line,
17001700
ID local = ISEQ_BODY(iseq)->local_table[local_index];
17011701

17021702
if (rb_is_local_id(local)) {
1703-
const char *name = rb_id2name(local);
1703+
VALUE name_obj = rb_id2str(local);
1704+
const char *name = RSTRING_PTR(name_obj);
17041705
size_t length = strlen(name);
17051706

17061707
// Explicitly skip numbered parameters. These should not be sent
@@ -1709,7 +1710,15 @@ pm_eval_make_iseq(VALUE src, VALUE fname, int line,
17091710
continue;
17101711
}
17111712

1712-
pm_string_constant_init(scope_local, name, length);
1713+
/* We need to duplicate the string because the Ruby string may
1714+
* be embedded so compaction could move the string and the pointer
1715+
* will change. */
1716+
char *name_dup = xmalloc(length + 1);
1717+
strlcpy(name_dup, name, length + 1);
1718+
1719+
RB_GC_GUARD(name_obj);
1720+
1721+
pm_string_owned_init(scope_local, (uint8_t *)name_dup, length);
17131722
}
17141723
}
17151724

0 commit comments

Comments
 (0)