From cb33f22f5b64b4d12cda7b7222898c3b20438fcc Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 17 Jul 2025 10:05:12 -0700 Subject: [PATCH 01/10] ZJIT: Precise GC writebarriers This issues writebarriers for objects added via gc_offsets or by profiling. This may be slower than writebarrier_remember, but we would like it to be more debuggable. Co-authored-by: Max Bernstein Co-authored-by: Stan Lo --- zjit/src/codegen.rs | 8 ++++---- zjit/src/gc.rs | 17 +++++++++++++++++ zjit/src/profile.rs | 7 +++---- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 8c9bf5d4d113a2..6d8692840e9bd8 100644 --- a/zjit/src/codegen.rs +++ b/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) @@ -193,7 +193,7 @@ fn gen_iseq(cb: &mut CodeBlock, iseq: IseqPtr) -> Option<(CodePtr, Vec<(Rc Option { - asm.cmp(val, Opnd::UImm(expected.into())); + asm.cmp(val, Opnd::Value(expected)); asm.jnz(side_exit(jit, state, GuardBitEquals(expected))?); Some(val) } diff --git a/zjit/src/gc.rs b/zjit/src/gc.rs index 23e2003661c54d..01bcc9fe5d532e 100644 --- a/zjit/src/gc.rs +++ b/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) { + 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)] diff --git a/zjit/src/profile.rs b/zjit/src/profile.rs index 4b5c7b60d40384..7db8e44c7a8276 100644 --- a/zjit/src/profile.rs +++ b/zjit/src/profile.rs @@ -88,11 +88,10 @@ fn profile_operands(profiler: &mut Profiler, profile: &mut IseqProfile, n: usize for i in 0..n { let opnd_type = Type::from_value(profiler.peek_at_stack((n - i - 1) as isize)); types[i] = types[i].union(opnd_type); + if let Some(object) = types[i].gc_object() { + unsafe { rb_gc_writebarrier(profiler.iseq.into(), object) }; + } } - // In the loop above, we probably added a new reference to the profile through - // the VALUE in Type. It's messy and relatively slow to conditionally run a - // write barrier for each Type, so just remember to re-mark the iseq. - unsafe { rb_gc_writebarrier_remember(profiler.iseq.into()) }; } #[derive(Debug)] From dabdd81d178ed91ca0feb0875ce0df927df0f3fb Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 17 Jul 2025 11:10:41 -0700 Subject: [PATCH 02/10] Fix linked list iteration when displaying errors When a script has problem with the magic comment encoding, we only display that error. However, if there are other syntax errors in the file, the error linked list could contain multiple items. This lead to an inconsistency in the "size" field of the linked list, and the actual items in the linked list. In other words, the linked list had more than one item, but the size field was one. The error display routine would only allocate `size` items, but iterating the linked list would overrun the array. This commit changes the iterator to compare the current node to the "finish" node in the linked list, no longer assuming the linked list ends with NULL. [Bug #21461] --- prism_compile.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/prism_compile.c b/prism_compile.c index e9585805243ffa..ded330f3cb5b2d 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -10613,7 +10613,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); From d1f38ce4acf844327a5efc8213be1a2822f16cf5 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Thu, 17 Jul 2025 15:19:51 -0400 Subject: [PATCH 03/10] Make protected documentation more explicit about differences (#13849) [DOC] Make protected documentation more explicit about differences Protected is a common source of confusion for devs coming from different languages to Ruby. This commit makes the documentation more explicit about the differences, so that the use case for protected is clearer. --- vm_method.c | 59 ++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 14 deletions(-) diff --git a/vm_method.c b/vm_method.c index 08fcd2ca4571fc..e63804d34d1626 100644 --- a/vm_method.c +++ b/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 already have access to all methods defined in the parent + * class, even private ones. + * + * Marking a method as protected allows different objects of the same + * class to call it. + * + * One use case is for comparison methods, such as ==, 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 - * self of the context is the same as the method. - * (method definition or instance_eval). This behavior is different from - * Java's protected method. Usually private 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 :doc: instead of this. */ @@ -3196,4 +3228,3 @@ Init_eval_method(void) REPLICATE_METHOD(rb_eException, idRespond_to_missing); } } - From 04d43e1870bb9a1b096fa78aaf0846c020d51444 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Thu, 17 Jul 2025 12:22:26 -0700 Subject: [PATCH 04/10] ZJIT: Give up JIT-to-JIT calls for 6+ args (#13939) --- test/ruby/test_zjit.rb | 15 +++++++++++++++ zjit/src/codegen.rs | 3 +++ 2 files changed, 18 insertions(+) diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index 0f83a75db7c53e..d598b094a49ceb 100644 --- a/test/ruby/test_zjit.rb +++ b/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{ diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 6d8692840e9bd8..ced20d82215341 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -291,6 +291,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))?), From 265059603c3aa6a13f90096c71b32046a17938f3 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Sun, 13 Apr 2025 15:10:32 +0200 Subject: [PATCH 05/10] [Bug #21256] Fix `it` parameter when splatting and `define_method` is used It was failing to set the leads, like numblocks do, causing the result to be wrapped in an array --- prism_compile.c | 11 +++++++++++ test/ruby/test_syntax.rb | 13 +++++++++++++ 2 files changed, 24 insertions(+) diff --git a/prism_compile.c b/prism_compile.c index ded330f3cb5b2d..59a2a716b76487 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -6767,6 +6767,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********** diff --git a/test/ruby/test_syntax.rb b/test/ruby/test_syntax.rb index b7e021a4ff4976..9cace7ec2b6dc9 100644 --- a/test/ruby/test_syntax.rb +++ b/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) From 148db9c80f11af1780f0f3685201f28de8f6b47a Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 17 Jul 2025 12:40:59 -0700 Subject: [PATCH 06/10] Fix flipflop line numbers [ruby-core:121605] --- prism_compile.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/prism_compile.c b/prism_compile.c index 59a2a716b76487..bf786f19a2657d 100644 --- a/prism_compile.c +++ b/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); From 014df99c9401056862fae741d0d2fc9892de5eba Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Thu, 17 Jul 2025 14:12:54 -0700 Subject: [PATCH 07/10] ZJIT: Remove obsoleted exit_trampoline (#13943) --- zjit/src/state.rs | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/zjit/src/state.rs b/zjit/src/state.rs index cb68f8a8effb93..001b550747e904 100644 --- a/zjit/src/state.rs +++ b/zjit/src/state.rs @@ -3,8 +3,6 @@ use crate::cruby_methods; use crate::invariants::Invariants; use crate::options::Options; use crate::asm::CodeBlock; -use crate::backend::lir::{asm_comment, Assembler, C_RET_OPND}; -use crate::virtualmem::CodePtr; #[allow(non_upper_case_globals)] #[unsafe(no_mangle)] @@ -31,9 +29,6 @@ pub struct ZJITState { /// Properties of core library methods method_annotations: cruby_methods::Annotations, - - /// Trampoline to propagate a callee's side exit to the caller - exit_trampoline: Option, } /// Private singleton instance of the codegen globals @@ -88,14 +83,8 @@ impl ZJITState { invariants: Invariants::default(), assert_compiles: false, method_annotations: cruby_methods::init(), - exit_trampoline: None, }; unsafe { ZJIT_STATE = Some(zjit_state); } - - // Generate trampolines after initializing ZJITState, which Assembler will use - let cb = ZJITState::get_code_block(); - let exit_trampoline = Self::gen_exit_trampoline(cb).unwrap(); - ZJITState::get_instance().exit_trampoline = Some(exit_trampoline); } /// Return true if zjit_state has been initialized @@ -137,20 +126,6 @@ impl ZJITState { let instance = ZJITState::get_instance(); instance.assert_compiles = true; } - - /// Generate a trampoline to propagate a callee's side exit to the caller - fn gen_exit_trampoline(cb: &mut CodeBlock) -> Option { - let mut asm = Assembler::new(); - asm_comment!(asm, "ZJIT exit trampoline"); - asm.frame_teardown(); - asm.cret(C_RET_OPND); - asm.compile(cb).map(|(start_ptr, _)| start_ptr) - } - - /// Get the trampoline to propagate a callee's side exit to the caller - pub fn get_exit_trampoline() -> CodePtr { - ZJITState::get_instance().exit_trampoline.unwrap() - } } /// Initialize ZJIT, given options allocated by rb_zjit_init_options() From 86320a53002a3adaf35ad7434c70e86747a8b345 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 17 Jul 2025 11:43:49 -0700 Subject: [PATCH 08/10] Fix compilation for forwarding params in Prism [Bug #21326] --- prism_compile.c | 18 +++++++++--- test/ruby/test_compile_prism.rb | 50 +++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/prism_compile.c b/prism_compile.c index bf786f19a2657d..dd925fc9c93f45 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -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; } diff --git a/test/ruby/test_compile_prism.rb b/test/ruby/test_compile_prism.rb index 86f7f0b14f1fb2..b95add5bd45fe4 100644 --- a/test/ruby/test_compile_prism.rb +++ b/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") From 30b1368829820721e502f4e0edcaa4de511959a8 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Thu, 17 Jul 2025 18:36:44 -0400 Subject: [PATCH 09/10] ZJIT: Create perf map files for profilers (#13941) This lets us ZJIT compiled functions show up in the profiles of, say, perf, or samply. Fix https://github.com/Shopify/ruby/issues/634 --- zjit/src/codegen.rs | 38 ++++++++++++++++++++++++++++++++++++-- zjit/src/options.rs | 6 ++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index ced20d82215341..49dbf46dcec2e8 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -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 { // 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 @@ -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 diff --git a/zjit/src/options.rs b/zjit/src/options.rs index 68bbca07a470fe..e97c5ff99fd3be 100644 --- a/zjit/src/options.rs +++ b/zjit/src/options.rs @@ -33,6 +33,9 @@ pub struct Options { /// Dump all compiled machine code. pub dump_disasm: bool, + + /// Dump code map to /tmp for performance profilers. + pub perf: bool, } /// Return an Options with default values @@ -44,6 +47,7 @@ pub fn init_options() -> Options { dump_hir_opt: None, dump_lir: false, dump_disasm: false, + perf: false, } } @@ -143,6 +147,8 @@ fn parse_option(options: &mut Options, str_ptr: *const std::os::raw::c_char) -> ("dump-disasm", "") => options.dump_disasm = true, + ("perf", "") => options.perf = true, + _ => return None, // Option name not recognized } From 81515aca67a0d3cc50205ff0a99cdc44ed0a927d Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Fri, 18 Jul 2025 00:48:53 +0100 Subject: [PATCH 10/10] ZJIT: Fix fixnum folding for negative values (#13942) Use `fixnum_from_isize` instead of `fixnum_from_usize` in `fold_fixnum_bop` to properly handle negative values. Casting negative `i64` to `usize` produces large unsigned values that exceed `RUBY_FIXNUM_MAX`. --- test/.excludes-zjit/TestTime.rb | 1 - test/.excludes-zjit/TestTimeTZ.rb | 1 - zjit/src/hir.rs | 20 +++++++++++++++++++- 3 files changed, 19 insertions(+), 3 deletions(-) delete mode 100644 test/.excludes-zjit/TestTime.rb delete mode 100644 test/.excludes-zjit/TestTimeTZ.rb diff --git a/test/.excludes-zjit/TestTime.rb b/test/.excludes-zjit/TestTime.rb deleted file mode 100644 index b2ca8c67f77f16..00000000000000 --- a/test/.excludes-zjit/TestTime.rb +++ /dev/null @@ -1 +0,0 @@ -exclude(/test_/, 'Tests make ZJIT panic') diff --git a/test/.excludes-zjit/TestTimeTZ.rb b/test/.excludes-zjit/TestTimeTZ.rb deleted file mode 100644 index b2ca8c67f77f16..00000000000000 --- a/test/.excludes-zjit/TestTimeTZ.rb +++ /dev/null @@ -1 +0,0 @@ -exclude(/test_/, 'Tests make ZJIT panic') diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index fa72b569b49c57..41efce66245b34 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -1703,7 +1703,7 @@ impl Function { fn fold_fixnum_bop(&mut self, insn_id: InsnId, left: InsnId, right: InsnId, f: impl FnOnce(Option, Option) -> Option) -> InsnId { f(self.type_of(left).fixnum_value(), self.type_of(right).fixnum_value()) .filter(|&n| n >= (RUBY_FIXNUM_MIN as i64) && n <= RUBY_FIXNUM_MAX as i64) - .map(|n| self.new_insn(Insn::Const { val: Const::Value(VALUE::fixnum_from_usize(n as usize)) })) + .map(|n| self.new_insn(Insn::Const { val: Const::Value(VALUE::fixnum_from_isize(n as isize)) })) .unwrap_or(insn_id) } @@ -5191,6 +5191,24 @@ mod opt_tests { "#]]); } + #[test] + fn test_fold_fixnum_sub_large_negative_result() { + eval(" + def test + 0 - 1073741825 + end + "); + assert_optimized_method_hir("test", expect![[r#" + fn test@:3: + bb0(v0:BasicObject): + v2:Fixnum[0] = Const Value(0) + v3:Fixnum[1073741825] = Const Value(1073741825) + PatchPoint BOPRedefined(INTEGER_REDEFINED_OP_FLAG, BOP_MINUS) + v9:Fixnum[-1073741825] = Const Value(-1073741825) + Return v9 + "#]]); + } + #[test] fn test_fold_fixnum_mult() { eval("