mmtk / ruby

Fork of The Ruby Programming Language [mirror], with added support for MMTk
https://www.ruby-lang.org/
Other
0 stars 1 forks source link

Run main Ruby tests with MMTk enabled in CI #78

Closed eightbitraptor closed 1 month ago

eightbitraptor commented 1 month ago

Replaces mmtk/ruby#77 - I've opened a new PR from the Shopify Ruby fork to enable @eileencodes to take over maintenance.


This commit creates a github actions workflow that builds Ruby with the latest commit of the mmtk-ruby binding and runs make test-all with MMTk enabled.

Several changes were necessary to make this CI pipeline pass:

Fix a bug in the setup-directories task so the directory hierarchy can be created succesfully.

C99 prohibits empty compile units, so some code had to be moved around in mmtk_support.c to introduce some superfluous #include directives. These should either be optimised out or remain unused so have little impact on the final binary.

Correct the dependancy lists in common.mk, by building with the appropiate flags defined in tool/update-deps and subsequently running make update-deps

Pre-fixing all externally visible MMTk related symbols and globals with rb_

Skipping tests related to GC compaction or other functionality specific to Ruby's GC.

Updating YJIT bindings

wks commented 1 month ago

The following patch fixes a compilation error in debug build (if -DUSE_RUBY_DEBUG_LOG=1)

commit fb6d11f0991f2c6c25620e49c6f912076d4dbb96 (HEAD -> mvh-tests-ci)
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Fri Jul 26 13:35:15 2024 +0800

    Fix compilation error in debug build

    A variable name in a RUBY_DEBUG_LOG macro is not updated.

diff --git a/mmtk_support.c b/mmtk_support.c
index 74cbc0836f..c6c8e5523d 100644
--- a/mmtk_support.c
+++ b/mmtk_support.c
@@ -289,7 +289,7 @@ set_variables_from_options(MMTk_Builder *mmtk_builder)
     RUBY_DEBUG_LOG("mmtk_plan_uses_bump_pointer = %d\n", rb_mmtk_plan_uses_bump_pointer);

     rb_mmtk_plan_implicitly_pinning = mmtk_builder_is_mark_sweep(mmtk_builder);
-    RUBY_DEBUG_LOG("mmtk_plan_implicitly_pinning = %d\n", mmtk_plan_implicitly_pinning);
+    RUBY_DEBUG_LOG("mmtk_plan_implicitly_pinning = %d\n", rb_mmtk_plan_implicitly_pinning);

     // We sometimes for disabling or enabling barriers to measure the impact of barriers.
     const char* barrier_env_var = getenv("RB_MMTK_FORCE_BARRIER");
wks commented 1 month ago

The GitHub CI reports the MMTk test failed when running with the Immix plan. According to the error message (https://github.com/mmtk/ruby/actions/runs/10082854589/job/27878079140?pr=78#step:10:285), a segmentation fault happened when trying to scan a "potential pinning parent" (PPP) object in order to pin its children. The segmentation fault was caused by accessing a very low address 0x0000000000000028. It looks like some invalid object references got added to the list of potential pinning parents for some reason.

The log said it happens while running test/prism/snippets_test.rb, but I can't reproduce this locally by running this test case alone. I think an error happened while running other tests, corrupting some data structure in the mmtk-ruby binding (including the C part or the Rust part), but only manifests in a GC triggered during snippets_test.rb. So the test case snippets_test.rb itself should be innocent.

eileencodes commented 1 month ago

Replaced by https://github.com/mmtk/ruby/pull/80 which squashes the commits and cleans up some changes.

eileencodes commented 1 month ago

Closing this now that I merged #80. We'll work on the remaining crashes in separate PRs (some are already fixed).