diff --git a/array.c b/array.c index 4b1343b6fa51bc..ed0c8e10bc48bb 100644 --- a/array.c +++ b/array.c @@ -3387,6 +3387,7 @@ rb_ary_reverse_m(VALUE ary) const VALUE *p1 = RARRAY_CONST_PTR(ary); VALUE *p2 = (VALUE *)RARRAY_CONST_PTR(dup) + len - 1; do *p2-- = *p1++; while (--len > 0); + rb_gc_writebarrier_remember(dup); } ARY_SET_LEN(dup, RARRAY_LEN(ary)); return dup; diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index a288cd1149787d..64ce8a91165b3c 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -2715,3 +2715,40 @@ def foo(a:, b:, c:) = super(a: a, b: b, c: c) r.send(nil) r.value } + +# Ractor::Port.allocate leaves the owner Ractor NULL, so every method reachable +# from Ruby must reject such a port instead of dereferencing it. [Bug #22214] +assert_equal 'ok', %q{ + port = Ractor::Port.allocate + + messages = [ + -> { port.send(1) }, + -> { port << 1 }, + -> { port.receive }, + -> { port.close }, + -> { port.closed? }, + -> { port.inspect }, + -> { Ractor::Port.new.__send__(:initialize_copy, port) }, + -> { Ractor.new { :x }.monitor(port) }, + -> { Ractor.new { :x }.unmonitor(port) }, + -> { Ractor.select(port) }, + ].map do |blk| + begin + blk.call + 'not raised' + rescue TypeError => e + e.message + end + end + + messages.uniq == ['uninitialized Ractor::Port'] ? :ok : messages +} + +assert_equal 'uninitialized MyPort', %q{ + class MyPort < Ractor::Port; end + begin + MyPort.allocate.closed? + rescue TypeError => e + e.message + end +} diff --git a/ractor.rb b/ractor.rb index 1168ed7803599d..180825d534ed8d 100644 --- a/ractor.rb +++ b/ractor.rb @@ -825,7 +825,6 @@ def close # # Returns whether or not the port is closed. def closed? - Primitive.attr! :leaf __builtin_cexpr! %q{ ractor_port_closed_p(ec, self); } @@ -836,7 +835,7 @@ def closed? # port.inspect -> string def inspect "#r)))" + __builtin_cexpr! "SIZET2NUM(rb_ractor_id(ractor_port_ptr_check(self)->r))" } id:#{ __builtin_cexpr! "SIZET2NUM(ractor_port_id(RACTOR_PORT_PTR(self)))" }>" diff --git a/ractor_sync.c b/ractor_sync.c index 5b513a1315d831..6ac0c11f632bce 100644 --- a/ractor_sync.c +++ b/ractor_sync.c @@ -53,6 +53,19 @@ RACTOR_PORT_PTR(VALUE self) return RTYPEDDATA_GET_DATA(self); } +// r is NULL between Ractor::Port.allocate and ractor_port_init() +static struct ractor_port * +ractor_port_ptr_check(VALUE self) +{ + struct ractor_port *rp = RACTOR_PORT_PTR(self); + + if (UNLIKELY(rp->r == NULL)) { + rb_raise(rb_eTypeError, "uninitialized %"PRIsVALUE, rb_obj_class(self)); + } + + return rp; +} + static VALUE ractor_port_alloc(VALUE klass) { @@ -94,8 +107,8 @@ ractor_port_initialize(VALUE self) static VALUE ractor_port_initialize_copy(VALUE self, VALUE orig) { - struct ractor_port *dst = RACTOR_PORT_PTR(self); - struct ractor_port *src = RACTOR_PORT_PTR(orig); + struct ractor_port *dst = RACTOR_PORT_PTR(self); // uninitialized by definition + struct ractor_port *src = ractor_port_ptr_check(orig); dst->r = src->r; RB_OBJ_WRITTEN(self, Qundef, dst->r->pub.self); dst->id_ = ractor_port_id(src); @@ -120,7 +133,7 @@ ractor_port_p(VALUE self) static VALUE ractor_port_receive(rb_execution_context_t *ec, VALUE self) { - const struct ractor_port *rp = RACTOR_PORT_PTR(self); + const struct ractor_port *rp = ractor_port_ptr_check(self); if (rp->r != rb_ec_ractor_ptr(ec)) { rb_raise(rb_eRactorError, "only allowed from the creator Ractor of this port"); @@ -134,7 +147,7 @@ ractor_port_receive(rb_execution_context_t *ec, VALUE self) static VALUE ractor_port_send(rb_execution_context_t *ec, VALUE self, VALUE obj, VALUE move) { - const struct ractor_port *rp = RACTOR_PORT_PTR(self); + const struct ractor_port *rp = ractor_port_ptr_check(self); ractor_send(ec, rp, obj, RTEST(move)); RB_GC_GUARD(self); return self; @@ -146,7 +159,7 @@ static bool ractor_close_port(rb_execution_context_t *ec, rb_ractor_t *r, const static VALUE ractor_port_closed_p(rb_execution_context_t *ec, VALUE self) { - const struct ractor_port *rp = RACTOR_PORT_PTR(self); + const struct ractor_port *rp = ractor_port_ptr_check(self); rb_ractor_t *r = rp->r; bool closed; @@ -173,7 +186,7 @@ ractor_port_closed_p(rb_execution_context_t *ec, VALUE self) static VALUE ractor_port_close(rb_execution_context_t *ec, VALUE self) { - const struct ractor_port *rp = RACTOR_PORT_PTR(self); + const struct ractor_port *rp = ractor_port_ptr_check(self); rb_ractor_t *cr = rb_ec_ractor_ptr(ec); if (cr != rp->r) { @@ -560,7 +573,7 @@ ractor_monitor(rb_execution_context_t *ec, VALUE self, VALUE port) { rb_ractor_t *r = RACTOR_PTR(self); bool terminated = false; - const struct ractor_port *rp = RACTOR_PORT_PTR(port); + const struct ractor_port *rp = ractor_port_ptr_check(port); struct ractor_monitor *rm = ALLOC(struct ractor_monitor); rm->port = *rp; // copy port information @@ -592,7 +605,7 @@ static VALUE ractor_unmonitor(rb_execution_context_t *ec, VALUE self, VALUE port) { rb_ractor_t *r = RACTOR_PTR(self); - const struct ractor_port *rp = RACTOR_PORT_PTR(port); + const struct ractor_port *rp = ractor_port_ptr_check(port); RACTOR_LOCK(r); { @@ -1310,7 +1323,7 @@ ractor_selector_add(VALUE selv, VALUE rpv) } struct ractor_selector *s = RACTOR_SELECTOR_PTR(selv); - const struct ractor_port *rp = RACTOR_PORT_PTR(rpv); + const struct ractor_port *rp = ractor_port_ptr_check(rpv); if (st_lookup(s->ports, (st_data_t)rpv, NULL)) { rb_raise(rb_eArgError, "already added"); diff --git a/string.c b/string.c index 33377dc39497d3..ce001d35aaec5d 100644 --- a/string.c +++ b/string.c @@ -1967,10 +1967,10 @@ str_duplicate_setup_heap(VALUE klass, VALUE str, VALUE dup) str_duplicate_setup_encoding(str, dup, flags); } -/* Force duplicated strings above 1024 bytes to be views rather than copies since +/* Force duplicated strings above 256 bytes to be views rather than copies since * copying will use memory and have significant overhead. - * Calculated as: 1024 - header size - NUL terminator size */ -#define STR_DUPLICATE_MAX_EMBED_LEN ((long)(1024 - offsetof(struct RString, as.embed) - 1)) + * Calculated as: 256 - header size - NUL terminator size */ +#define STR_DUPLICATE_MAX_EMBED_LEN ((long)(256 - offsetof(struct RString, as.embed) - 1)) static inline VALUE str_duplicate(VALUE klass, VALUE str) diff --git a/test/objspace/test_objspace.rb b/test/objspace/test_objspace.rb index c96222e7630c3e..cfb38e69750023 100644 --- a/test/objspace/test_objspace.rb +++ b/test/objspace/test_objspace.rb @@ -28,7 +28,7 @@ def test_memsize_of ObjectSpace.memsize_of(//.match(""))) end - STR_DUPLICATE_MAX_EMBED_LEN = 999 # From macro defined in string.c + STR_DUPLICATE_MAX_EMBED_LEN = 256 - (RbConfig::SIZEOF["void*"] * 3) - 1 # From macro defined in string.c def test_memsize_of_root_shared_string a = "a" * (STR_DUPLICATE_MAX_EMBED_LEN + 1) @@ -36,7 +36,7 @@ def test_memsize_of_root_shared_string c = nil ObjectSpace.each_object(String) {|x| break c = x if a == x and x.frozen?} rv_size = Integer(ObjectSpace.dump(a)[/"slot_size":(\d+)/, 1]) - assert_equal([rv_size, rv_size, a.length + 1 + rv_size], [a, b, c].map {|x| ObjectSpace.memsize_of(x)}) + assert_equal([rv_size, GC::INTERNAL_CONSTANTS[:RVALUE_SIZE], rv_size], [a, b, c].map {|x| ObjectSpace.memsize_of(x)}) end def test_argf_memsize diff --git a/zjit/src/codegen_tests.rs b/zjit/src/codegen_tests.rs index 22fde4a9ef21c7..d03bd2d65e52e3 100644 --- a/zjit/src/codegen_tests.rs +++ b/zjit/src/codegen_tests.rs @@ -1056,6 +1056,26 @@ fn test_send_optional_and_rest_arguments() { assert_snapshot!(assert_compiles("entry"), @"[[1, 2, []], [3, 4, []], [5, 6, [7, 8]]]"); } +#[test] +fn test_send_optional_return_default_without_argument() { + eval(" + def test(arg = nil || (return :default)) = arg + def entry = test + entry + "); + assert_snapshot!(assert_compiles("entry"), @":default"); +} + +#[test] +fn test_send_optional_return_default_with_argument() { + eval(" + def test(arg = nil || (return :default)) = arg + def entry = test(1) + entry + "); + assert_snapshot!(assert_compiles("entry"), @"1"); +} + #[test] fn test_send_rest_arguments_with_keyword_to_positional_hash() { eval(" diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index bc65eae1b206b2..21c1c701cf0897 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -2750,6 +2750,12 @@ fn iseq_get_return_value(iseq: IseqPtr, captured_opnd: Option, ci_flags: if second_insn != YARVINSN_leave { return None; } + // Make sure the leave is the final instruction. Otherwise this is a + // non-trivial ISEQ that should go through the general inliner. + let iseq_size = unsafe { get_iseq_encoded_size(iseq) }; + if insn_len(first_insn as usize) + insn_len(second_insn as usize) != iseq_size { + return None; + } match first_insn { YARVINSN_getlocal_WC_0 => { // Accept only cases where only positional arguments are used by both the callee and the caller. diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 9cc439834b07f0..1682fe23eca4bb 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -19347,6 +19347,74 @@ mod hir_opt_tests { "); } + #[test] + fn test_inline_method_with_omitted_optional_return_default() { + // With the optional omitted, the general inliner enters entry 0 and + // runs the default expression path. + eval(" + def callee(arg = nil || (return :default)) + arg + end + def test = callee + test + test + "); + assert_snapshot!(hir_string_with_inlining("test"), @" + fn test@:5: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v4:BasicObject = LoadArg :self@0 + Jump bb3(v4) + bb3(v6:BasicObject): + PatchPoint MethodRedefined(Object@0x1000, callee@0x1008, cme:0x1010) + v18:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile + v40:NilClass = Const Value(nil) + PushInlineFrame v18 (0x1038) + v26:StaticSymbol[:default] = Const Value(VALUE(0x1040)) + CheckInterrupts + PopInlineFrame + Return v26 + "); + } + + #[test] + fn test_inline_method_with_supplied_optional_return_default() { + // With the optional supplied, the general inliner uses the selected + // optional entry instead of entry 0, so the default-expression return + // is not inlined. + eval(" + def callee(arg = nil || (return :default)) + arg + end + def test = callee(3) + test + test + "); + assert_snapshot!(hir_string_with_inlining("test"), @" + fn test@:5: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v4:BasicObject = LoadArg :self@0 + Jump bb3(v4) + bb3(v6:BasicObject): + v11:Fixnum[3] = Const Value(3) + PatchPoint MethodRedefined(Object@0x1000, callee@0x1008, cme:0x1010) + v20:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile + PushInlineFrame v20 (0x1038), v11 + CheckInterrupts + PopInlineFrame + Return v11 + "); + } + #[test] fn test_inline_method_with_rescue_handler() { eval("