From aeb08bc8a71006b63a6588ef5129b7723c8219fc Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Mon, 22 Apr 2024 14:33:34 -0400 Subject: [PATCH] YJIT: Fix String#setbyte crashing for converted arguments Previously, passing objects that respond to #to_int to `String#setbyte` resulted in a crash when compiled by YJIT. This was due to the lazily pushed frame from rb_yjit_lazy_push_frame() lingering and not being popped by an exception as expected. The fix is to ensure that `ec->cfp` is restored to before the lazy frame push in case the method call for conversion succeeds. Right now, this is only for conversion to integers. Found running `ruby/spec`. * clarify comment We just need to make sure `ec->cfp` is always preserved and this can convert without rising when `raise` is true. --- bootstraptest/test_yjit.rb | 17 +++++++++++++++++ object.c | 9 ++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb index c7a5883986..2e190a5b9c 100644 --- a/bootstraptest/test_yjit.rb +++ b/bootstraptest/test_yjit.rb @@ -4799,3 +4799,20 @@ assert_equal [0x80000000000, 'a+', :ok].inspect, %q{ tests } + +# test String#stebyte with arguments that need conversion +assert_equal "abc", %q{ + str = +"a00" + def change_bytes(str, one, two) + str.setbyte(one, "b".ord) + str.setbyte(2, two) + end + + to_int_1 = Object.new + to_int_99 = Object.new + def to_int_1.to_int = 1 + def to_int_99.to_int = 99 + + change_bytes(str, to_int_1, to_int_99) + str +} diff --git a/object.c b/object.c index 54240f0774..0a8ce4dc0f 100644 --- a/object.c +++ b/object.c @@ -3236,15 +3236,22 @@ ALWAYS_INLINE(static VALUE rb_to_integer_with_id_exception(VALUE val, const char static inline VALUE rb_to_integer_with_id_exception(VALUE val, const char *method, ID mid, int raise) { + // We need to pop the lazily pushed frame when not raising an exception. + rb_control_frame_t *current_cfp; VALUE v; if (RB_INTEGER_TYPE_P(val)) return val; + current_cfp = GET_EC()->cfp; rb_yjit_lazy_push_frame(GET_EC()->cfp->pc); v = try_to_int(val, mid, raise); - if (!raise && NIL_P(v)) return Qnil; + if (!raise && NIL_P(v)) { + GET_EC()->cfp = current_cfp; + return Qnil; + } if (!RB_INTEGER_TYPE_P(v)) { conversion_mismatch(val, "Integer", method, v); } + GET_EC()->cfp = current_cfp; return v; } #define rb_to_integer(val, method, mid) \