[pull] master from ruby:master

prism_compile.c

@@ -887,7 +887,7 @@ pm_compile_logical(rb_iseq_t *iseq, LINK_ANCHOR *const ret, pm_node_t *cond, LAB
 static void
 pm_compile_flip_flop_bound(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret, bool popped, pm_scope_node_t *scope_node)
 {
-    const pm_node_location_t location = { .line = ISEQ_BODY(iseq)->location.first_lineno, .node_id = -1 };
+    const pm_node_location_t location = PM_NODE_START_LOCATION(scope_node->parser, node);
 
     if (PM_NODE_TYPE_P(node, PM_INTEGER_NODE)) {
         PM_COMPILE_NOT_POPPED(node);
@@ -906,7 +906,7 @@ pm_compile_flip_flop_bound(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *
 static void
 pm_compile_flip_flop(const pm_flip_flop_node_t *flip_flop_node, LABEL *else_label, LABEL *then_label, rb_iseq_t *iseq, const int lineno, LINK_ANCHOR *const ret, bool popped, pm_scope_node_t *scope_node)
 {
-    const pm_node_location_t location = { .line = ISEQ_BODY(iseq)->location.first_lineno, .node_id = -1 };
+    const pm_node_location_t location = { .line = lineno, .node_id = -1 };
     LABEL *lend = NEW_LABEL(location.line);
 
     int again = !(flip_flop_node->base.flags & PM_RANGE_FLAGS_EXCLUDE_END);
@@ -1762,9 +1762,14 @@ pm_setup_args_core(const pm_arguments_node_t *arguments_node, const pm_node_t *b
                     break;
                 }
 
-                orig_argc += 2;
+                if (has_splat) {
+                    // If we already have a splat, we're concatenating to existing array
+                    orig_argc += 1;
+                } else {
+                    orig_argc += 2;
+                }
 
-                *flags |= VM_CALL_ARGS_SPLAT | VM_CALL_ARGS_SPLAT_MUT | VM_CALL_ARGS_BLOCKARG | VM_CALL_KW_SPLAT;
+                *flags |= VM_CALL_ARGS_SPLAT | VM_CALL_ARGS_BLOCKARG | VM_CALL_KW_SPLAT;
 
                 // Forwarding arguments nodes are treated as foo(*, **, &)
                 // So foo(...) equals foo(*, **, &) and as such the local
@@ -1773,7 +1778,13 @@ pm_setup_args_core(const pm_arguments_node_t *arguments_node, const pm_node_t *b
                 // Push the *
                 pm_local_index_t mult_local = pm_lookup_local_index(iseq, scope_node, PM_CONSTANT_MULT, 0);
                 PUSH_GETLOCAL(ret, location, mult_local.index, mult_local.level);
-                PUSH_INSN1(ret, location, splatarray, Qtrue);
+
+                if (has_splat) {
+                    // If we already have a splat, we need to concatenate arrays
+                    PUSH_INSN(ret, location, concattoarray);
+                } else {
+                    PUSH_INSN1(ret, location, splatarray, Qfalse);
+                }
 
                 // Push the **
                 pm_local_index_t pow_local = pm_lookup_local_index(iseq, scope_node, PM_CONSTANT_POW, 0);
@@ -1782,7 +1793,6 @@ pm_setup_args_core(const pm_arguments_node_t *arguments_node, const pm_node_t *b
                 // Push the &
                 pm_local_index_t and_local = pm_lookup_local_index(iseq, scope_node, PM_CONSTANT_AND, 0);
                 PUSH_INSN2(ret, location, getblockparamproxy, INT2FIX(and_local.index + VM_ENV_DATA_SIZE - 1), INT2FIX(and_local.level));
-                PUSH_INSN(ret, location, splatkw);
 
                 break;
               }
@@ -6767,6 +6777,17 @@ pm_compile_scope_node(rb_iseq_t *iseq, pm_scope_node_t *scope_node, const pm_nod
         body->param.flags.has_lead = true;
     }
 
+    if (scope_node->parameters && PM_NODE_TYPE_P(scope_node->parameters, PM_IT_PARAMETERS_NODE)) {
+        const uint8_t param_name[] = { 'i', 't' };
+        pm_constant_id_t constant_id = pm_constant_pool_find(&scope_node->parser->constant_pool, param_name, 2);
+        RUBY_ASSERT(constant_id && "parser should fill in `it` parameter");
+        pm_insert_local_index(constant_id, local_index, index_lookup_table, local_table_for_iseq, scope_node);
+
+        local_index++;
+        body->param.lead_num = 1;
+        body->param.flags.has_lead = true;
+    }
+
     //********END OF STEP 3**********
 
     //********STEP 4**********
@@ -10613,7 +10634,9 @@ pm_parse_errors_format_sort(const pm_parser_t *parser, const pm_list_t *error_li
     if (errors == NULL) return NULL;
 
     int32_t start_line = parser->start_line;
-    for (pm_diagnostic_t *error = (pm_diagnostic_t *) error_list->head; error != NULL; error = (pm_diagnostic_t *) error->node.next) {
+    pm_diagnostic_t *finish = (pm_diagnostic_t * )error_list->tail->next;
+
+    for (pm_diagnostic_t *error = (pm_diagnostic_t *) error_list->head; error != finish; error = (pm_diagnostic_t *) error->node.next) {
         pm_line_column_t start = pm_newline_list_line_column(newline_list, error->location.start, start_line);
         pm_line_column_t end = pm_newline_list_line_column(newline_list, error->location.end, start_line);
 

test/ruby/test_compile_prism.rb

@@ -2183,6 +2183,56 @@ def o.foo(...) = 1.times { bar(...) }
       RUBY
     end
 
+    def test_ForwardingArgumentsNode_instruction_sequence_consistency
+      # Test that both parsers generate identical instruction sequences for forwarding arguments
+      # This prevents regressions like the one fixed in prism_compile.c for PM_FORWARDING_ARGUMENTS_NODE
+
+      # Test case from the bug report: def bar(buz, ...) = foo(buz, ...)
+      source = <<~RUBY
+        def foo(*, &block) = block
+        def bar(buz, ...) = foo(buz, ...)
+      RUBY
+
+      compare_instruction_sequences(source)
+
+      # Test simple forwarding
+      source = <<~RUBY
+        def target(...) = nil
+        def forwarder(...) = target(...)
+      RUBY
+
+      compare_instruction_sequences(source)
+
+      # Test mixed forwarding with regular arguments
+      source = <<~RUBY
+        def target(a, b, c) = [a, b, c]
+        def forwarder(x, ...) = target(x, ...)
+      RUBY
+
+      compare_instruction_sequences(source)
+
+      # Test forwarding with splat
+      source = <<~RUBY
+        def target(a, b, c) = [a, b, c]
+        def forwarder(x, ...); target(*x, ...); end
+      RUBY
+
+      compare_instruction_sequences(source)
+    end
+
+    private
+
+    def compare_instruction_sequences(source)
+      # Get instruction sequences from both parsers
+      parsey_iseq = RubyVM::InstructionSequence.compile_parsey(source)
+      prism_iseq = RubyVM::InstructionSequence.compile_prism(source)
+
+      # Compare instruction sequences
+      assert_equal parsey_iseq.disasm, prism_iseq.disasm
+    end
+
+    public
+
     def test_ForwardingSuperNode
       assert_prism_eval("class Forwarding; def to_s; super; end; end")
       assert_prism_eval("class Forwarding; def eval(code); super { code }; end; end")

test/ruby/test_syntax.rb

@@ -1957,6 +1957,19 @@ def test_it
     assert_equal(/9/, eval('9.then { /#{it}/o }'))
   end
 
+  def test_it_with_splat_super_method
+    bug21256 = '[ruby-core:121592] [Bug #21256]'
+
+    a = Class.new do
+      define_method(:foo) { it }
+    end
+    b = Class.new(a) do
+      def foo(*args) = super
+    end
+
+    assert_equal(1, b.new.foo(1), bug21256)
+  end
+
   def test_value_expr_in_condition
     mesg = /void value expression/
     assert_syntax_error("tap {a = (true ? next : break)}", mesg)

test/ruby/test_zjit.rb

@@ -133,6 +133,21 @@ def test3 = baz(4, 1)
     }
   end
 
+  def test_send_with_six_args
+    assert_compiles '[1, 2, 3, 4, 5, 6]', %q{
+      def foo(a1, a2, a3, a4, a5, a6)
+        [a1, a2, a3, a4, a5, a6]
+      end
+
+      def test
+        foo(1, 2, 3, 4, 5, 6)
+      end
+
+      test # profile send
+      test
+    }, call_threshold: 2
+  end
+
   def test_invokebuiltin
     omit 'Test fails at the moment due to not handling optional parameters'
     assert_compiles '["."]', %q{

vm_method.c

@@ -2606,21 +2606,53 @@ rb_mod_public(int argc, VALUE *argv, VALUE module)
  *     protected(method_name, method_name, ...) -> array
  *     protected(array)                         -> array
  *
- *  With no arguments, sets the default visibility for subsequently
- *  defined methods to protected. With arguments, sets the named methods
- *  to have protected visibility.
- *  String arguments are converted to symbols.
- *  An Array of Symbols and/or Strings is also accepted.
- *  If a single argument is passed, it is returned.
- *  If no argument is passed, nil is returned.
- *  If multiple arguments are passed, the arguments are returned as an array.
+ *  Sets the visibility of a section or of a list of method names as protected.
+ *  Accepts no arguments, a splat of method names (symbols or strings) or an
+ *  array of method names. Returns the arguments that it received.
+ *
+ *  == Important difference between protected in other languages
+ *
+ *  Protected methods in Ruby are different from other languages such as Java,
+ *  where methods are marked as protected to give access to subclasses. In Ruby,
+ *  subclasses <b>already have access to all methods defined in the parent
+ *  class</b>, even private ones.
+ *
+ *  Marking a method as protected allows <b>different objects of the same
+ *  class</b> to call it.
+ *
+ *  One use case is for comparison methods, such as <code>==</code>, if we want
+ *  to expose a method for comparison between objects of the same class without
+ *  making the method public to objects of other classes.
  *
- *  If a method has protected visibility, it is callable only where
- *  <code>self</code> of the context is the same as the method.
- *  (method definition or instance_eval). This behavior is different from
- *  Java's protected method. Usually <code>private</code> should be used.
+ *  == Performance considerations
  *
- *  Note that a protected method is slow because it can't use inline cache.
+ *  Protected methods are slower than others because they can't use inline
+ *  cache.
+ *
+ *  == Example
+ *
+ *    class Account
+ *      # Mark balance as protected, so that we can compare between accounts
+ *      # without making it public.
+ *      attr_reader :balance
+ *      protected :balance
+ *
+ *      def initialize(balance)
+ *        @balance = balance
+ *      end
+ *
+ *      def >(other)
+ *        # The invocation to `other.balance` is allowed because `other` is a
+ *        # different object of the same class (Account).
+ *        balance > other.balance
+ *      end
+ *    end
+ *
+ *    account1 = Account.new(100)
+ *    account2 = Account.new(50)
+ *
+ *    account1 > account2  # => true (works)
+ *    account1.balance     # => NoMethodError (fails because balance is not public)
  *
  *  To show a private method on RDoc, use <code>:doc:</code> instead of this.
  */
@@ -3196,4 +3228,3 @@ Init_eval_method(void)
         REPLICATE_METHOD(rb_eException, idRespond_to_missing);
     }
 }
-

zjit/src/codegen.rs

@@ -4,7 +4,7 @@ use std::rc::Rc;
 use crate::asm::Label;
 use crate::backend::current::{Reg, ALLOC_REGS};
 use crate::invariants::track_bop_assumption;
-use crate::gc::get_or_create_iseq_payload;
+use crate::gc::{get_or_create_iseq_payload, append_gc_offsets};
 use crate::state::ZJITState;
 use crate::{asm::CodeBlock, cruby::*, options::debug, virtualmem::CodePtr};
 use crate::backend::lir::{self, asm_comment, asm_ccall, Assembler, Opnd, SideExitContext, Target, CFP, C_ARG_OPNDS, C_RET_OPND, EC, NATIVE_STACK_PTR, SP};
@@ -124,7 +124,7 @@ fn gen_iseq_entry_point_body(cb: &mut CodeBlock, iseq: IseqPtr) -> *const u8 {
             // Remember the block address to reuse it later
             let payload = get_or_create_iseq_payload(iseq);
             payload.start_ptr = Some(start_ptr);
-            payload.gc_offsets.extend(gc_offsets);
+            append_gc_offsets(iseq, &gc_offsets);
 
             // Compile an entry point to the JIT code
             (gen_entry(cb, iseq, &function, start_ptr, jit.c_stack_bytes), jit.branch_iseqs)
@@ -154,6 +154,20 @@ fn gen_iseq_entry_point_body(cb: &mut CodeBlock, iseq: IseqPtr) -> *const u8 {
     start_ptr.map(|start_ptr| start_ptr.raw_ptr(cb)).unwrap_or(std::ptr::null())
 }
 
+/// Write an entry to the perf map in /tmp
+fn register_with_perf(iseq_name: String, start_ptr: usize, code_size: usize) {
+    use std::io::Write;
+    let perf_map = format!("/tmp/perf-{}.map", std::process::id());
+    let Ok(mut file) = std::fs::OpenOptions::new().create(true).append(true).open(&perf_map) else {
+        debug!("Failed to open perf map file: {perf_map}");
+        return;
+    };
+    let Ok(_) = writeln!(file, "{:#x} {:#x} zjit::{}", start_ptr, code_size, iseq_name) else {
+        debug!("Failed to write {iseq_name} to perf map file: {perf_map}");
+        return;
+    };
+}
+
 /// Compile a JIT entry
 fn gen_entry(cb: &mut CodeBlock, iseq: IseqPtr, function: &Function, function_ptr: CodePtr, c_stack_bytes: usize) -> Option<CodePtr> {
     // Set up registers for CFP, EC, SP, and basic block arguments
@@ -172,7 +186,17 @@ fn gen_entry(cb: &mut CodeBlock, iseq: IseqPtr, function: &Function, function_pt
     asm.frame_teardown();
     asm.cret(C_RET_OPND);
 
-    asm.compile(cb).map(|(start_ptr, _)| start_ptr)
+    let result = asm.compile(cb).map(|(start_ptr, _)| start_ptr);
+    if let Some(start_addr) = result {
+        if get_option!(perf) {
+            let start_ptr = start_addr.raw_ptr(cb) as usize;
+            let end_ptr = cb.get_write_ptr().raw_ptr(cb) as usize;
+            let code_size = end_ptr - start_ptr;
+            let iseq_name = iseq_get_location(iseq, 0);
+            register_with_perf(format!("entry for {iseq_name}"), start_ptr, code_size);
+        }
+    }
+    result
 }
 
 /// Compile an ISEQ into machine code
@@ -193,7 +217,7 @@ fn gen_iseq(cb: &mut CodeBlock, iseq: IseqPtr) -> Option<(CodePtr, Vec<(Rc<Branc
     let result = gen_function(cb, iseq, &function);
     if let Some((start_ptr, gc_offsets, jit)) = result {
         payload.start_ptr = Some(start_ptr);
-        payload.gc_offsets.extend(gc_offsets);
+        append_gc_offsets(iseq, &gc_offsets);
         Some((start_ptr, jit.branch_iseqs))
     } else {
         None
@@ -255,7 +279,17 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, function: &Function) -> Optio
     }
 
     // Generate code if everything can be compiled
-    asm.compile(cb).map(|(start_ptr, gc_offsets)| (start_ptr, gc_offsets, jit))
+    let result = asm.compile(cb).map(|(start_ptr, gc_offsets)| (start_ptr, gc_offsets, jit));
+    if let Some((start_ptr, _, _)) = result {
+        if get_option!(perf) {
+            let start_usize = start_ptr.raw_ptr(cb) as usize;
+            let end_usize = cb.get_write_ptr().raw_ptr(cb) as usize;
+            let code_size = end_usize - start_usize;
+            let iseq_name = iseq_get_location(iseq, 0);
+            register_with_perf(iseq_name, start_usize, code_size);
+        }
+    }
+    result
 }
 
 /// Compile an instruction
@@ -291,6 +325,9 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
         Insn::IfTrue { val, target } => return gen_if_true(jit, asm, opnd!(val), target),
         Insn::IfFalse { val, target } => return gen_if_false(jit, asm, opnd!(val), target),
         Insn::SendWithoutBlock { call_info, cd, state, self_val, args, .. } => gen_send_without_block(jit, asm, call_info, *cd, &function.frame_state(*state), opnd!(self_val), opnds!(args))?,
+        // Give up SendWithoutBlockDirect for 6+ args since asm.ccall() doesn't support it.
+        Insn::SendWithoutBlockDirect { call_info, cd, state, self_val, args, .. } if args.len() + 1 > C_ARG_OPNDS.len() => // +1 for self
+            gen_send_without_block(jit, asm, call_info, *cd, &function.frame_state(*state), opnd!(self_val), opnds!(args))?,
         Insn::SendWithoutBlockDirect { cme, iseq, self_val, args, state, .. } => gen_send_without_block_direct(cb, jit, asm, *cme, *iseq, opnd!(self_val), opnds!(args), &function.frame_state(*state))?,
         Insn::InvokeBuiltin { bf, args, state } => gen_invokebuiltin(asm, &function.frame_state(*state), bf, opnds!(args))?,
         Insn::Return { val } => return Some(gen_return(jit, asm, opnd!(val))?),
@@ -999,7 +1036,7 @@ fn gen_guard_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, guard
 
 /// Compile an identity check with a side exit
 fn gen_guard_bit_equals(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, expected: VALUE, state: &FrameState) -> Option<lir::Opnd> {
-    asm.cmp(val, Opnd::UImm(expected.into()));
+    asm.cmp(val, Opnd::Value(expected));
     asm.jnz(side_exit(jit, state, GuardBitEquals(expected))?);
     Some(val)
 }

zjit/src/gc.rs

@@ -90,6 +90,23 @@ pub extern "C" fn rb_zjit_iseq_mark(payload: *mut c_void) {
     }
 }
 
+/// Append a set of gc_offsets to the iseq's payload
+pub fn append_gc_offsets(iseq: IseqPtr, offsets: &Vec<CodePtr>) {
+    let payload = get_or_create_iseq_payload(iseq);
+    payload.gc_offsets.extend(offsets);
+
+    // Call writebarrier on each newly added value
+    let cb = ZJITState::get_code_block();
+    for &offset in offsets.iter() {
+        let value_ptr: *const u8 = offset.raw_ptr(cb);
+        let value_ptr = value_ptr as *const VALUE;
+        unsafe {
+            let object = value_ptr.read_unaligned();
+            rb_gc_writebarrier(iseq.into(), object);
+        }
+    }
+}
+
 /// GC callback for updating GC objects in the per-iseq payload.
 /// This is a mirror of [rb_zjit_iseq_mark].
 #[unsafe(no_mangle)]