Skip to content

Fix complex transi compact #547

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

casperisfine
Copy link

I think this PR fixes the compaction issue, but now I run into the bug I was looking to in the first place:

$ make test/ruby/test_shapes.rb
TestShapes#test_evacuate_class_ivar_and_compaction [/Users/byroot/src/github.com/Shopify/ruby/test/ruby/test_shapes.rb:279]:
pid 83296 killed by SIGABRT (signal 6)
| <OBJ_INFO:[email protected]:6966> 0x0000000107ccfc10 [0 M    ] T_NONE 
| /Users/byroot/src/github.com/Shopify/ruby/tool/lib/core_assertions.rb:524: [BUG] try to mark T_NONE object
| -- C level backtrace information -------------------------------------------
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(rb_vm_bugreport+0xb60) [0x1052bb5f0] vm_dump.c:1143
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(rb_vm_bugreport) (null):0
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(rb_bug_without_die+0xf0) [0x1050daa84] error.c:1042
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(rb_bug+0x1c) [0x1053b495c] error.c:1050
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(rb_gc_mark_weak+0x0) [0x1050fbce4] gc.c:6967
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(gc_mark_ptr) (null):0
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(gc_mark+0x10) [0x105100ad8] gc.c:7003
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(mark_method_entry) gc.c:6635
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(gc_mark_imemo) gc.c:7153
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(gc_mark_children) gc.c:7275
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(gc_mark_stacked_objects+0x78) [0x105108bd8] gc.c:7471
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(gc_mark_stacked_objects_incremental) gc.c:7503
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(gc_marks_rest+0xac) [0x1051089e0] gc.c:8693
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(gc_rest+0x110) [0x1050fa154] gc.c:9528
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(garbage_collect+0x5c) [0x1050fdf74] gc.c:9382
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(objspace_xmalloc0+0x70) [0x10510167c] gc.c:12408
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(vm_ccs_push+0x14c) [0x1052a2cfc] ./vm_insnhelper.c:1997
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(imemo_type_p+0x0) [0x105285d30] ./vm_insnhelper.c:2138
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(vm_cc_cme) ./vm_callinfo.h:394
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(vm_search_cc) ./vm_insnhelper.c:2140
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(rb_vm_search_method_slowpath) ./vm_insnhelper.c:2156
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(vm_search_method_slowpath0+0x24) [0x1052af124] ./vm_insnhelper.c:2177
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(vm_search_method_fastpath+0x10) [0x105287a4c] ./vm_insnhelper.c:2238
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(vm_sendish) ./vm_insnhelper.c:5581
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(vm_exec_core+0x31bc) [0x10528c344] insns.def:822
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(rb_vm_exec+0x1f4) [0x105287e90] vm.c:2467
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(rb_ec_exec_node+0xac) [0x1050e6ab4] eval.c:287
| /Users/byroot/src/github.com/Shopify/ruby/libruby.3.3.dylib(ruby_run_node+0x70) [0x1050e6998] eval.c:328
| /Users/byroot/src/github.com/Shopify/ruby/ruby(rb_main+0x1c) [0x1048bff18] ./main.c:39

@casperisfine casperisfine force-pushed the fix-complex-transi-compact branch 2 times, most recently from 9232d8c to a4d6719 Compare November 16, 2023 17:39
@casperisfine
Copy link
Author

It's crashing on:

          case VM_METHOD_TYPE_ISEQ:
            if (def->body.iseq.iseqptr) gc_mark(objspace, (VALUE)def->body.iseq.iseqptr);

When transitioning an object to TOO_COMPLEX we copy all its
ivar in a table, but if GC (and compaction) trigger in the middle
of the transition, the old `iv_ptr` might still be the canonical
ivar list and will be updated by the GC, but the reference we
copied in the table will be outdated.

So we must pin these reference until they are all copied and
the object `iv_ptr` is pointing to the table.

To do this we allocate a temporary TOO_COMPLEX object
which we use as the host for the copy. TOO_COMPLEX objects
are marked with `mark_tbl` which does pin references.

Co-Authored-By: Alan Wu <[email protected]>
@casperisfine casperisfine force-pushed the fix-complex-transi-compact branch from a4d6719 to 97fb07d Compare November 18, 2023 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants