From e23a60b929cff951252167935e8868b69ae3dc74 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 17 Dec 2024 14:14:36 -0500 Subject: [PATCH] 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.. 434346726c) +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 --- test/ruby/test_eval.rb | 15 +++++++++++++++ vm_eval.c | 13 +++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/test/ruby/test_eval.rb b/test/ruby/test_eval.rb index 2129272b00..b880b03e08 100644 --- a/test/ruby/test_eval.rb +++ b/test/ruby/test_eval.rb @@ -636,4 +636,19 @@ class TestEval < Test::Unit::TestCase end end; end + + def test_outer_local_variable_under_gc_compact_stress + omit "compaction is not supported on this platform" unless GC.respond_to?(:compact) + + assert_separately([], <<~RUBY) + o = Object.new + def o.m = 1 + + GC.verify_compaction_references(expand_heap: true, toward: :empty) + + EnvUtil.under_gc_compact_stress do + assert_equal(1, eval("o.m")) + end + RUBY + end end diff --git a/vm_eval.c b/vm_eval.c index 7c7f5c0bf4..d9d6fb7064 100644 --- a/vm_eval.c +++ b/vm_eval.c @@ -1700,7 +1700,8 @@ pm_eval_make_iseq(VALUE src, VALUE fname, int line, ID local = ISEQ_BODY(iseq)->local_table[local_index]; if (rb_is_local_id(local)) { - const char *name = rb_id2name(local); + VALUE name_obj = rb_id2str(local); + const char *name = RSTRING_PTR(name_obj); size_t length = strlen(name); // Explicitly skip numbered parameters. These should not be sent @@ -1709,7 +1710,15 @@ pm_eval_make_iseq(VALUE src, VALUE fname, int line, continue; } - pm_string_constant_init(scope_local, name, length); + /* We need to duplicate the string because the Ruby string may + * be embedded so compaction could move the string and the pointer + * will change. */ + char *name_dup = xmalloc(length + 1); + strlcpy(name_dup, name, length + 1); + + RB_GC_GUARD(name_obj); + + pm_string_owned_init(scope_local, (uint8_t *)name_dup, length); } }