diff --git a/NEWS.md b/NEWS.md index 8d9c34f9326579..6f6671c2f3611a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -140,7 +140,7 @@ The following default gems are updated. * RubyGems 3.7.0.dev * bundler 2.7.0.dev -* erb 5.0.1 +* erb 5.0.2 * etc 1.4.6 * io-console 0.8.1 * io-nonblock 0.3.2 diff --git a/concurrent_set.c b/concurrent_set.c index dac6e9ce397ae5..876b4083e62491 100644 --- a/concurrent_set.c +++ b/concurrent_set.c @@ -21,7 +21,7 @@ struct concurrent_set { rb_atomic_t size; unsigned int capacity; unsigned int deleted_entries; - struct rb_concurrent_set_funcs *funcs; + const struct rb_concurrent_set_funcs *funcs; struct concurrent_set_entry *entries; }; @@ -51,7 +51,7 @@ static const rb_data_type_t concurrent_set_type = { }; VALUE -rb_concurrent_set_new(struct rb_concurrent_set_funcs *funcs, int capacity) +rb_concurrent_set_new(const struct rb_concurrent_set_funcs *funcs, int capacity) { struct concurrent_set *set; VALUE obj = TypedData_Make_Struct(0, struct concurrent_set, &concurrent_set_type, set); diff --git a/ext/erb/escape/escape.c b/ext/erb/escape/escape.c index db2abd97738b0b..2a5903c3b738c8 100644 --- a/ext/erb/escape/escape.c +++ b/ext/erb/escape/escape.c @@ -89,6 +89,10 @@ erb_escape_html(VALUE self, VALUE str) void Init_escape(void) { +#ifdef HAVE_RB_EXT_RACTOR_SAFE + rb_ext_ractor_safe(true); +#endif + rb_cERB = rb_define_class("ERB", rb_cObject); rb_mEscape = rb_define_module_under(rb_cERB, "Escape"); rb_define_module_function(rb_mEscape, "html_escape", erb_escape_html, 1); diff --git a/ext/erb/escape/extconf.rb b/ext/erb/escape/extconf.rb index 783e8c1f5562f8..b211a9783f459e 100644 --- a/ext/erb/escape/extconf.rb +++ b/ext/erb/escape/extconf.rb @@ -4,5 +4,6 @@ when 'jruby', 'truffleruby' File.write('Makefile', dummy_makefile($srcdir).join) else + have_func("rb_ext_ractor_safe", "ruby.h") create_makefile 'erb/escape' end diff --git a/internal/concurrent_set.h b/internal/concurrent_set.h index 3000fc31bf058f..d0f546b888b43d 100644 --- a/internal/concurrent_set.h +++ b/internal/concurrent_set.h @@ -13,7 +13,7 @@ struct rb_concurrent_set_funcs { rb_concurrent_set_create_func create; }; -VALUE rb_concurrent_set_new(struct rb_concurrent_set_funcs *funcs, int capacity); +VALUE rb_concurrent_set_new(const struct rb_concurrent_set_funcs *funcs, int capacity); VALUE rb_concurrent_set_find_or_insert(VALUE *set_obj_ptr, VALUE key, void *data); VALUE rb_concurrent_set_delete_by_identity(VALUE set_obj, VALUE key); void rb_concurrent_set_foreach_with_replace(VALUE set_obj, int (*callback)(VALUE *key, void *data), void *data); diff --git a/lib/erb/version.rb b/lib/erb/version.rb index 3d0cedcd48475d..0875dcb42a8227 100644 --- a/lib/erb/version.rb +++ b/lib/erb/version.rb @@ -1,4 +1,4 @@ # frozen_string_literal: true class ERB - VERSION = '5.0.1' + VERSION = '5.0.2' end diff --git a/string.c b/string.c index c425eb56f08a68..39508c939d9f83 100644 --- a/string.c +++ b/string.c @@ -548,7 +548,7 @@ fstring_concurrent_set_create(VALUE str, void *data) return str; } -static struct rb_concurrent_set_funcs fstring_concurrent_set_funcs = { +static const struct rb_concurrent_set_funcs fstring_concurrent_set_funcs = { .hash = fstring_concurrent_set_hash, .cmp = fstring_concurrent_set_cmp, .create = fstring_concurrent_set_create, @@ -8146,19 +8146,12 @@ downcase_single(VALUE str) * call-seq: * downcase!(mapping) -> self or nil * - * Downcases the characters in +self+; - * returns +self+ if any changes were made, +nil+ otherwise: - * - * s = 'Hello World!' # => "Hello World!" - * s.downcase! # => "hello world!" - * s # => "hello world!" - * s.downcase! # => nil + * Like String#downcase, except that: * - * The casing may be affected by the given +mapping+; - * see {Case Mapping}[rdoc-ref:case_mapping.rdoc]. - * - * Related: String#downcase, String#upcase, String#upcase!. + * - Changes character casings in +self+ (not in a copy of +self+). + * - Returns +self+ if any changes are made, +nil+ otherwise. * + * Related: See {Modifying}[rdoc-ref:String@Modifying]. */ static VALUE diff --git a/test/ruby/test_proc.rb b/test/ruby/test_proc.rb index f6b9e6d063324f..2cd97ca32464ab 100644 --- a/test/ruby/test_proc.rb +++ b/test/ruby/test_proc.rb @@ -1655,7 +1655,7 @@ def test_local_variable_set def test_numparam_is_not_local_variables "foo".tap do - _9 + _9 and flunk assert_equal([], binding.local_variables) assert_raise(NameError) { binding.local_variable_get(:_9) } assert_raise(NameError) { binding.local_variable_set(:_9, 1) } @@ -1674,7 +1674,7 @@ def test_numparam_is_not_local_variables assert_raise(NameError) { binding.local_variable_get(:_9) } assert_raise(NameError) { binding.local_variable_set(:_9, 1) } "bar".tap do - _9 + _9 and flunk assert_equal([], binding.local_variables) assert_raise(NameError) { binding.local_variable_get(:_9) } assert_raise(NameError) { binding.local_variable_set(:_9, 1) } diff --git a/test/ruby/test_rubyoptions.rb b/test/ruby/test_rubyoptions.rb index 54ad953ee9944e..631b1886770273 100644 --- a/test/ruby/test_rubyoptions.rb +++ b/test/ruby/test_rubyoptions.rb @@ -875,7 +875,7 @@ def test_segv_loaded_features '-e', '$".clear', '-e', '$".unshift Bogus.new', '-e', '(p $"; abort) unless $".size == 1', - ], success: false) + ], bug7402, success: false) end def test_segv_setproctitle diff --git a/test/ruby/test_variable.rb b/test/ruby/test_variable.rb index 984045e05d0479..cc784e7644fdf3 100644 --- a/test/ruby/test_variable.rb +++ b/test/ruby/test_variable.rb @@ -419,6 +419,7 @@ def test_exivar_resize_with_compaction_stress x end end + objs or flunk end def test_local_variables_with_kwarg @@ -440,7 +441,7 @@ def test_many_instance_variables end def test_local_variables_encoding - α = 1 + α = 1 or flunk b = binding b.eval("".encode("us-ascii")) assert_equal(%i[α b], b.local_variables) diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index f71509bc7cf808..fdcee3cafd858e 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -776,6 +776,11 @@ def test test } + + assert_compiles '1', %q{ + def a(n1,n2,n3,n4,n5,n6,n7,n8,n9) = n1+n9 + a(2,0,0,0,0,0,0,0,-1) + } end def test_opt_aref_with diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index 4f804a3406ce84..ac5c9908f55c43 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -206,6 +206,11 @@ impl Assembler vec![X1_REG, X9_REG, X10_REG, X11_REG, X12_REG, X13_REG, X14_REG, X15_REG] } + /// How many bytes a call and a [Self::frame_setup] would change native SP + pub fn frame_size() -> i32 { + 0x10 + } + /// Split platform-specific instructions /// The transformations done here are meant to make our lives simpler in later /// stages of the compilation pipeline. diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index c890281496cf1b..e5453e4d554b67 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -32,13 +32,13 @@ pub enum MemBase pub struct Mem { // Base register number or instruction index - pub(super) base: MemBase, + pub base: MemBase, // Offset relative to the base pointer - pub(super) disp: i32, + pub disp: i32, // Size in bits - pub(super) num_bits: u8, + pub num_bits: u8, } impl fmt::Debug for Mem { @@ -2150,6 +2150,7 @@ impl Assembler { } pub fn load_into(&mut self, dest: Opnd, opnd: Opnd) { + assert!(matches!(dest, Opnd::Reg(_) | Opnd::VReg{..}), "Destination of load_into must be a register"); match (dest, opnd) { (Opnd::Reg(dest), Opnd::Reg(opnd)) if dest == opnd => {}, // skip if noop _ => self.push_insn(Insn::LoadInto { dest, opnd }), @@ -2312,5 +2313,13 @@ mod tests { assert!(matches!(opnd_iter.next(), None)); } + + #[test] + #[should_panic] + fn load_into_memory_is_invalid() { + let mut asm = Assembler::new(); + let mem = Opnd::mem(64, SP, 0); + asm.load_into(mem, mem); + } } diff --git a/zjit/src/backend/x86_64/mod.rs b/zjit/src/backend/x86_64/mod.rs index 85a6a0f5b47755..1c905a475ca9ed 100644 --- a/zjit/src/backend/x86_64/mod.rs +++ b/zjit/src/backend/x86_64/mod.rs @@ -110,6 +110,11 @@ impl Assembler vec![RAX_REG, RCX_REG, RDX_REG, RSI_REG, RDI_REG, R8_REG, R9_REG, R10_REG, R11_REG] } + /// How many bytes a call and a [Self::frame_setup] would change native SP + pub fn frame_size() -> i32 { + 0x8 + } + // These are the callee-saved registers in the x86-64 SysV ABI // RBX, RSP, RBP, and R12–R15 @@ -475,6 +480,7 @@ impl Assembler // (e.g. with Linux `perf record --call-graph fp`) Insn::FrameSetup => { if false { // We don't support --zjit-perf yet + // TODO(alan): Change Assembler::frame_size() when adding --zjit-perf support push(cb, RBP); mov(cb, RBP, RSP); push(cb, RBP); diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 6398d3bce0d6cc..ce3decd56f319b 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -107,14 +107,14 @@ fn gen_iseq_entry_point(iseq: IseqPtr) -> *const u8 { // Compile the High-level IR let cb = ZJITState::get_code_block(); let (start_ptr, mut branch_iseqs) = match gen_function(cb, iseq, &function) { - Some((start_ptr, gc_offsets, branch_iseqs)) => { + Some((start_ptr, gc_offsets, jit)) => { // 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); // Compile an entry point to the JIT code - (gen_entry(cb, iseq, &function, start_ptr), branch_iseqs) + (gen_entry(cb, iseq, &function, start_ptr, jit.c_stack_bytes), jit.branch_iseqs) }, None => (None, vec![]), }; @@ -145,11 +145,11 @@ fn gen_iseq_entry_point(iseq: IseqPtr) -> *const u8 { } /// Compile a JIT entry -fn gen_entry(cb: &mut CodeBlock, iseq: IseqPtr, function: &Function, function_ptr: CodePtr) -> Option { +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 let mut asm = Assembler::new(); gen_entry_prologue(&mut asm, iseq); - gen_entry_params(&mut asm, iseq, function.block(BlockId(0))); + gen_entry_params(&mut asm, iseq, function.block(BlockId(0)), c_stack_bytes); // Jump to the first block using a call instruction asm.ccall(function_ptr.raw_ptr(cb) as *const u8, vec![]); @@ -185,17 +185,17 @@ fn gen_iseq(cb: &mut CodeBlock, iseq: IseqPtr) -> Option<(CodePtr, Vec<(Rc Option<(CodePtr, Vec, Vec<(Rc, IseqPtr)>)> { +fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, function: &Function) -> Option<(CodePtr, Vec, JITState)> { let c_stack_bytes = aligned_stack_bytes(max_num_params(function).saturating_sub(ALLOC_REGS.len())); let mut jit = JITState::new(iseq, function.num_insns(), function.num_blocks(), c_stack_bytes); let mut asm = Assembler::new(); @@ -249,7 +249,7 @@ 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.branch_iseqs)) + asm.compile(cb).map(|(start_ptr, gc_offsets)| (start_ptr, gc_offsets, jit)) } /// Compile an instruction @@ -527,7 +527,7 @@ fn gen_entry_prologue(asm: &mut Assembler, iseq: IseqPtr) { } /// Assign method arguments to basic block arguments at JIT entry -fn gen_entry_params(asm: &mut Assembler, iseq: IseqPtr, entry_block: &Block) { +fn gen_entry_params(asm: &mut Assembler, iseq: IseqPtr, entry_block: &Block, c_stack_bytes: usize) { let self_param = gen_param(asm, SELF_PARAM_IDX); asm.mov(self_param, Opnd::mem(VALUE_BITS, CFP, RUBY_OFFSET_CFP_SELF)); @@ -536,14 +536,34 @@ fn gen_entry_params(asm: &mut Assembler, iseq: IseqPtr, entry_block: &Block) { asm_comment!(asm, "set method params: {num_params}"); // Allocate registers for basic block arguments - let params: Vec = (0..num_params).map(|idx| - gen_param(asm, idx + 1) // +1 for self - ).collect(); + for idx in 0..num_params { + let param = gen_param(asm, idx + 1); // +1 for self + + // Funky offset adjustment to write into the native stack frame of the + // HIR function we'll be calling into. This only makes sense in context + // of the schedule of instructions in gen_entry() for the JIT entry point. + // + // The entry point needs to load VALUEs into native stack slots _before_ the + // frame containing the slots exists. So, we anticipate the stack frame size + // of the Function and subtract offsets based on that. + // + // native SP at entry point ─────►┌────────────┐ Native SP grows downwards + // │ │ ↓ on all arches we support. + // SP-0x8 ├────────────┤ + // │ │ + // where native SP SP-0x10├────────────┤ + // would be while │ │ + // the HIR function ────────────► └────────────┘ + // is running + let param = if let Opnd::Mem(lir::Mem { base, disp, num_bits }) = param { + Opnd::Mem(lir::Mem { num_bits, base, disp: disp - c_stack_bytes as i32 - Assembler::frame_size() }) + } else { + param + }; - // Assign local variables to the basic block arguments - for (idx, ¶m) in params.iter().enumerate() { + // Assign local variables to the basic block arguments let local = gen_entry_param(asm, iseq, idx); - asm.load_into(param, local); + asm.mov(param, local); } } } @@ -1045,11 +1065,12 @@ fn gen_push_frame(asm: &mut Assembler, argc: usize, state: &FrameState, frame: C /// Return an operand we use for the basic block argument at a given index fn param_opnd(idx: usize) -> Opnd { // To simplify the implementation, allocate a fixed register or a stack slot for each basic block argument for now. + // Note that this is implemented here as opposed to automatically inside LIR machineries. // TODO: Allow allocating arbitrary registers for basic block arguments if idx < ALLOC_REGS.len() { Opnd::Reg(ALLOC_REGS[idx]) } else { - Opnd::mem(64, NATIVE_STACK_PTR, -((idx - ALLOC_REGS.len() + 1) as i32) * SIZEOF_VALUE_I32) + Opnd::mem(64, NATIVE_STACK_PTR, (idx - ALLOC_REGS.len()) as i32 * SIZEOF_VALUE_I32) } } diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 0fee728b1223da..598774de8cff72 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -939,6 +939,14 @@ pub enum ValidationError { DuplicateInstruction(BlockId, InsnId), } +fn can_direct_send(iseq: *const rb_iseq_t) -> bool { + if unsafe { rb_get_iseq_flags_has_rest(iseq) } { false } + else if unsafe { rb_get_iseq_flags_has_opt(iseq) } { false } + else if unsafe { rb_get_iseq_flags_has_kw(iseq) } { false } + else if unsafe { rb_get_iseq_flags_has_kwrest(iseq) } { false } + else if unsafe { rb_get_iseq_flags_has_block(iseq) } { false } + else { true } +} /// A [`Function`], which is analogous to a Ruby ISeq, is a control-flow graph of [`Block`]s /// containing instructions. @@ -1507,8 +1515,13 @@ impl Function { // TODO(max): Allow non-iseq; cache cme self.push_insn_id(block, insn_id); continue; } - self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid }, state }); + // Only specialize positional-positional calls + // TODO(max): Handle other kinds of parameter passing let iseq = unsafe { get_def_iseq_ptr((*cme).def) }; + if !can_direct_send(iseq) { + self.push_insn_id(block, insn_id); continue; + } + self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid }, state }); if let Some(expected) = guard_equal_to { self_val = self.push_insn(block, Insn::GuardBitEquals { val: self_val, expected, state }); } @@ -6313,6 +6326,88 @@ mod opt_tests { "#]]); } + #[test] + fn dont_specialize_call_to_iseq_with_opt() { + eval(" + def foo(arg=1) = 1 + def test = foo 1 + test + test + "); + assert_optimized_method_hir("test", expect![[r#" + fn test@:3: + bb0(v0:BasicObject): + v2:Fixnum[1] = Const Value(1) + v4:BasicObject = SendWithoutBlock v0, :foo, v2 + Return v4 + "#]]); + } + + #[test] + fn dont_specialize_call_to_iseq_with_block() { + eval(" + def foo(&block) = 1 + def test = foo {|| } + test + test + "); + assert_optimized_method_hir("test", expect![[r#" + fn test@:3: + bb0(v0:BasicObject): + v3:BasicObject = Send v0, 0x1000, :foo + Return v3 + "#]]); + } + + #[test] + fn dont_specialize_call_to_iseq_with_rest() { + eval(" + def foo(*args) = 1 + def test = foo 1 + test + test + "); + assert_optimized_method_hir("test", expect![[r#" + fn test@:3: + bb0(v0:BasicObject): + v2:Fixnum[1] = Const Value(1) + v4:BasicObject = SendWithoutBlock v0, :foo, v2 + Return v4 + "#]]); + } + + #[test] + fn dont_specialize_call_to_iseq_with_kw() { + eval(" + def foo(a:) = 1 + def test = foo(a: 1) + test + test + "); + assert_optimized_method_hir("test", expect![[r#" + fn test@:3: + bb0(v0:BasicObject): + v2:Fixnum[1] = Const Value(1) + SideExit UnknownCallType + "#]]); + } + + #[test] + fn dont_specialize_call_to_iseq_with_kwrest() { + eval(" + def foo(**args) = 1 + def test = foo(a: 1) + test + test + "); + assert_optimized_method_hir("test", expect![[r#" + fn test@:3: + bb0(v0:BasicObject): + v2:Fixnum[1] = Const Value(1) + SideExit UnknownCallType + "#]]); + } + #[test] fn string_bytesize_simple() { eval("