From ed18a212abe2d92feb441db2b6f084c0c77a65e4 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Wed, 10 Dec 2025 11:08:55 -0500 Subject: [PATCH 1/4] ZJIT: Check if shape is too complex before reading ivar by index (#15478) This fixes a crash when the new shape after a transition is too complex; we need to check that it's not complex before trying to read by index. --- zjit/src/hir.rs | 4 ++-- zjit/src/hir/opt_tests.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 4e597db936c345..1771f960c3f952 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -3155,8 +3155,6 @@ impl Function { // We're only looking at T_OBJECT so ignore all of the imemo stuff. assert!(recv_type.flags().is_t_object()); next_shape_id = ShapeId(unsafe { rb_shape_transition_add_ivar_no_warnings(class, current_shape_id.0, id) }); - let ivar_result = unsafe { rb_shape_get_iv_index(next_shape_id.0, id, &mut ivar_index) }; - assert!(ivar_result, "New shape must have the ivar index"); // If the VM ran out of shapes, or this class generated too many leaf, // it may be de-optimized into OBJ_TOO_COMPLEX_SHAPE (hash-table). let new_shape_too_complex = unsafe { rb_jit_shape_too_complex_p(next_shape_id.0) }; @@ -3165,6 +3163,8 @@ impl Function { self.push_insn(block, Insn::IncrCounter(Counter::setivar_fallback_new_shape_too_complex)); self.push_insn_id(block, insn_id); continue; } + let ivar_result = unsafe { rb_shape_get_iv_index(next_shape_id.0, id, &mut ivar_index) }; + assert!(ivar_result, "New shape must have the ivar index"); let current_capacity = unsafe { rb_jit_shape_capacity(current_shape_id.0) }; let next_capacity = unsafe { rb_jit_shape_capacity(next_shape_id.0) }; // If the new shape has a different capacity, or is TOO_COMPLEX, we'll have to diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 3e2db528158090..2e20c8fe8a73e2 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -3932,6 +3932,38 @@ mod hir_opt_tests { "); } + #[test] + fn test_dont_specialize_setivar_when_next_shape_is_too_complex() { + eval(r#" + class AboutToBeTooComplex + def test = @abc = 5 + end + SHAPE_MAX_VARIATIONS = 8 # see shape.h + SHAPE_MAX_VARIATIONS.times do + AboutToBeTooComplex.new.instance_variable_set(:"@a#{_1}", 1) + end + AboutToBeTooComplex.new.test + TEST = AboutToBeTooComplex.instance_method(:test) + "#); + assert_snapshot!(hir_string_proc("TEST"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + v10:Fixnum[5] = Const Value(5) + PatchPoint SingleRactorMode + IncrCounter setivar_fallback_new_shape_too_complex + SetIvar v6, :@abc, v10 + CheckInterrupts + Return v10 + "); + } + #[test] fn test_elide_freeze_with_frozen_hash() { eval(" From 1eb10ca3cb6cff98bb8c0946ed905921586c7d52 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Wed, 10 Dec 2025 09:45:09 -0800 Subject: [PATCH 2/4] ZJIT: Exclude failing ruby-bench benchmarks (#15479) --- .github/workflows/zjit-macos.yml | 2 +- .github/workflows/zjit-ubuntu.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/zjit-macos.yml b/.github/workflows/zjit-macos.yml index ed559d64e10d05..d6194825a445a7 100644 --- a/.github/workflows/zjit-macos.yml +++ b/.github/workflows/zjit-macos.yml @@ -158,7 +158,7 @@ jobs: include: # Test --call-threshold=2 with 2 iterations in total - ruby_opts: '--zjit-call-threshold=2' - bench_opts: '--warmup=1 --bench=1' + bench_opts: '--warmup=1 --bench=1 --excludes=fluentd,psych-load,railsbench' configure: '--enable-zjit=dev_nodebug' # --enable-zjit=dev is too slow runs-on: macos-14 diff --git a/.github/workflows/zjit-ubuntu.yml b/.github/workflows/zjit-ubuntu.yml index 37a9000d704c5a..64bb0903f8ae7c 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -215,7 +215,7 @@ jobs: include: # Test --call-threshold=2 with 2 iterations in total - ruby_opts: '--zjit-call-threshold=2' - bench_opts: '--warmup=1 --bench=1' + bench_opts: '--warmup=1 --bench=1 --excludes=fluentd,psych-load,railsbench' configure: '--enable-zjit=dev_nodebug' # --enable-zjit=dev is too slow runs-on: ubuntu-24.04 From 41ee65899a7a6c6abca9701957bc84f598d2491a Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Mon, 8 Dec 2025 15:54:26 -0800 Subject: [PATCH 3/4] Always treat encoding as TYPEDDATA Encodings are RTypedData, not the deprecated RData. Although the structures are compatible we should use the correct API. --- encoding.c | 12 ++++++------ ext/-test-/string/fstring.c | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/encoding.c b/encoding.c index 3d5c1d777283fe..d14d371b04c319 100644 --- a/encoding.c +++ b/encoding.c @@ -222,7 +222,7 @@ enc_check_encoding(VALUE obj) if (!is_obj_encoding(obj)) { return -1; } - return check_encoding(RDATA(obj)->data); + return check_encoding(RTYPEDDATA_GET_DATA(obj)); } NORETURN(static void not_encoding(VALUE enc)); @@ -240,7 +240,7 @@ must_encoding(VALUE enc) if (index < 0) { not_encoding(enc); } - return DATA_PTR(enc); + return RTYPEDDATA_GET_DATA(enc); } static rb_encoding * @@ -328,7 +328,7 @@ str_to_encoding(VALUE enc) rb_encoding * rb_to_encoding(VALUE enc) { - if (enc_check_encoding(enc) >= 0) return RDATA(enc)->data; + if (enc_check_encoding(enc) >= 0) return RTYPEDDATA_GET_DATA(enc); return str_to_encoding(enc); } @@ -336,7 +336,7 @@ rb_encoding * rb_find_encoding(VALUE enc) { int idx; - if (enc_check_encoding(enc) >= 0) return RDATA(enc)->data; + if (enc_check_encoding(enc) >= 0) return RTYPEDDATA_GET_DATA(enc); idx = str_find_encindex(enc); if (idx < 0) return NULL; return rb_enc_from_index(idx); @@ -1345,7 +1345,7 @@ enc_inspect(VALUE self) if (!is_data_encoding(self)) { not_encoding(self); } - if (!(enc = DATA_PTR(self)) || rb_enc_from_index(rb_enc_to_index(enc)) != enc) { + if (!(enc = RTYPEDDATA_GET_DATA(self)) || rb_enc_from_index(rb_enc_to_index(enc)) != enc) { rb_raise(rb_eTypeError, "broken Encoding"); } @@ -1368,7 +1368,7 @@ enc_inspect(VALUE self) static VALUE enc_name(VALUE self) { - return rb_fstring_cstr(rb_enc_name((rb_encoding*)DATA_PTR(self))); + return rb_fstring_cstr(rb_enc_name((rb_encoding*)RTYPEDDATA_GET_DATA(self))); } static int diff --git a/ext/-test-/string/fstring.c b/ext/-test-/string/fstring.c index 92a846a93c2e5c..0b5940f28c46ca 100644 --- a/ext/-test-/string/fstring.c +++ b/ext/-test-/string/fstring.c @@ -19,13 +19,13 @@ bug_s_fstring_fake_str(VALUE self) VALUE bug_s_rb_enc_interned_str(VALUE self, VALUE encoding) { - return rb_enc_interned_str("foo", 3, NIL_P(encoding) ? NULL : RDATA(encoding)->data); + return rb_enc_interned_str("foo", 3, NIL_P(encoding) ? NULL : RTYPEDDATA_GET_DATA(encoding)); } VALUE bug_s_rb_enc_str_new(VALUE self, VALUE encoding) { - return rb_enc_str_new("foo", 3, NIL_P(encoding) ? NULL : RDATA(encoding)->data); + return rb_enc_str_new("foo", 3, NIL_P(encoding) ? NULL : RTYPEDDATA_GET_DATA(encoding)); } void From 330ddccfee1c986add758384b31b0677935bfa3a Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Wed, 10 Dec 2025 10:10:54 -0800 Subject: [PATCH 4/4] ubuntu.yml: Add a ruby-bench job without ZJIT (#15480) --- .github/workflows/ubuntu.yml | 50 ++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml index 9f3ad412f7ba6c..46ad7c82128e23 100644 --- a/.github/workflows/ubuntu.yml +++ b/.github/workflows/ubuntu.yml @@ -202,6 +202,56 @@ jobs: steps: *make-steps + # Separated from `make` job to avoid making it a required status check + ruby-bench: + strategy: + matrix: + include: + # Using the same setup as ZJIT jobs + - bench_opts: '--warmup=1 --bench=1' + + runs-on: ubuntu-24.04 + + if: >- + ${{!(false + || contains(github.event.head_commit.message, '[DOC]') + || contains(github.event.pull_request.title, '[DOC]') + || contains(github.event.pull_request.labels.*.name, 'Documentation') + || (github.event_name == 'push' && github.event.pull_request.user.login == 'dependabot[bot]') + )}} + + steps: + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + + - uses: ./.github/actions/setup/ubuntu + + - uses: ./.github/actions/setup/directories + with: + srcdir: src + builddir: build + makeup: true + + - name: Run configure + run: ../src/configure -C --disable-install-doc --prefix="$(pwd)/install" + + - run: make install + + - name: Checkout ruby-bench + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + with: + repository: ruby/ruby-bench + path: ruby-bench + + - name: Run ruby-bench + run: ruby run_benchmarks.rb -e "ruby::../build/install/bin/ruby" ${{ matrix.bench_opts }} + working-directory: ruby-bench + + - uses: ./.github/actions/slack + with: + label: ruby-bench ${{ matrix.bench_opts }} ${{ matrix.ruby_opts }} + SLACK_WEBHOOK_URL: ${{ secrets.SIMPLER_ALERTS_URL }} # ruby-lang slack: ruby/simpler-alerts-bot + if: ${{ failure() }} + result: if: ${{ always() }} name: ${{ github.workflow }} result