diff --git a/array.c b/array.c index 802270551c9bc6..4d28175c4d73fa 100644 --- a/array.c +++ b/array.c @@ -3963,6 +3963,9 @@ append_values_at_single(VALUE result, VALUE ary, long olen, VALUE idx) /* check if idx is Range */ else if (rb_range_beg_len(idx, &beg, &len, olen, 1)) { if (len > 0) { + // rb_range_beg_len may run arbitrary code that modifies ary, so we + // need to re-calculate olen + const long olen = RARRAY_LEN(ary); const VALUE *const src = RARRAY_CONST_PTR(ary); const long end = beg + len; const long prevlen = RARRAY_LEN(result); diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index 4158556c06bc07..578360a507cf01 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -888,50 +888,47 @@ class C end RUBY -# ivar in shareable-objects are not allowed to access from non-main Ractor -assert_equal 'can not access instance variables of shareable objects from non-main Ractors', %q{ +# setting an ivar on a shareable but unfrozen object is not allowed, by instance_variable_set +assert_equal "can't modify instance variables of a shareable Ractor", %q{ shared = Ractor.new{} - shared.instance_variable_set(:@iv, 'str') - - r = Ractor.new shared do |shared| - p shared.instance_variable_get(:@iv) - end begin - r.value - rescue Ractor::RemoteError => e - e.cause.message + shared.instance_variable_set(:@iv, 'str') + rescue Ractor::IsolationError => e + e.message end } -# ivar in shareable-objects are not allowed to access from non-main Ractor, by @iv (get) -assert_equal 'can not access instance variables of shareable objects from non-main Ractors', %q{ +# setting an ivar on a shareable but unfrozen object is not allowed, by @iv = ... +assert_equal "can't modify instance variables of a shareable Ractor", %q{ class Ractor def setup @foo = '' end + end - def foo - @foo - end + shared = Ractor.new{} + + begin + shared.setup + rescue Ractor::IsolationError => e + e.message end +} +# ivars of a shareable object are frozen, so they can be read from a non-main Ractor +assert_equal 'nil', %q{ shared = Ractor.new{} - shared.setup r = Ractor.new shared do |shared| - p shared.foo + shared.instance_variable_get(:@iv) end - begin - r.value - rescue Ractor::RemoteError => e - e.cause.message - end + r.value.inspect } -# ivar in shareable-objects are not allowed to access from non-main Ractor, by @iv (set) -assert_equal 'can not access instance variables of shareable objects from non-main Ractors', %q{ +# setting an ivar on a shareable object is not allowed from a non-main Ractor either +assert_equal "can't modify instance variables of a shareable Ractor", %q{ class Ractor def setup @foo = '' @@ -941,7 +938,7 @@ def setup shared = Ractor.new{} r = Ractor.new shared do |shared| - p shared.setup + shared.setup end begin @@ -951,6 +948,69 @@ def setup end } +# a Proc which already has ivars can not be isolated +assert_equal 'can not isolate a Proc because it has instance variables', %q{ + pr = Proc.new{} + pr.instance_variable_set(:@iv, Object.new) + + begin + Ractor.new(&pr) + rescue Ractor::IsolationError => e + e.message + end +} + +# freezing the Proc first does not skip the check +assert_equal 'can not isolate a Proc because it has instance variables', %q{ + pr = Proc.new{} + pr.instance_variable_set(:@iv, Object.new) + pr.freeze + + begin + Ractor.new(&pr) + rescue Ractor::IsolationError => e + e.message + end +} + +# an ivar set by Proc#refined is internal, so it does not block isolation +assert_equal 'r', %q{ + module M + refine String do + def foo; 'r'; end + end + end + + rp = Proc.new{ ''.foo }.refined(M) + Ractor.new(&rp).value +} + +# a Proc made shareable by Ractor.new can not be given ivars afterwards +assert_equal "can't modify instance variables of a shareable Proc", %q{ + HAX = -> { } + Ractor.new(&HAX).join + + begin + HAX.instance_variable_set(:@foo, Object.new) + rescue Ractor::IsolationError => e + e.message + end +} + +# freezing a shareable object from another Ractor can not expose an ivar, since +# none could be set after it became shareable +assert_equal 'nil', %q{ + HAX = -> { } + Ractor.new(&HAX).join + + r = Ractor.new do + HAX.freeze + HAX.instance_variable_get(:@foo) + end + + r.value.inspect +} + # But a shareable object is frozen, it is allowed to access ivars from non-main Ractor assert_equal '11', %q{ [Object.new, [], ].map{|obj| diff --git a/eval.c b/eval.c index 17a3f2743ff336..89460c55e67394 100644 --- a/eval.c +++ b/eval.c @@ -82,6 +82,7 @@ ruby_setup(void) rb_w32_init_long_paths(); #endif Init_BareVM(); + Init_default_shapes(); rb_vm_encoded_insn_data_table_init(); Init_enable_box(); Init_vm_objects(); diff --git a/hash.c b/hash.c index bc7bd5b4a36af7..279f540d379ad9 100644 --- a/hash.c +++ b/hash.c @@ -36,6 +36,7 @@ #include "internal/hash.h" #include "internal/object.h" #include "internal/proc.h" +#include "internal/ractor.h" #include "internal/st.h" #include "internal/symbol.h" #include "internal/thread.h" diff --git a/inits.c b/inits.c index e4323cc4a847b6..95ef7402edf6f3 100644 --- a/inits.c +++ b/inits.c @@ -20,7 +20,6 @@ static void Init_builtin_prelude(void); void rb_call_inits(void) { - CALL(default_shapes); CALL(Thread_Mutex); CALL(RandomSeedCore); CALL(encodings); diff --git a/internal/inits.h b/internal/inits.h index be73dac1dcbe8a..4fcced17426187 100644 --- a/internal/inits.h +++ b/internal/inits.h @@ -29,6 +29,9 @@ int Init_enc_set_filesystem_encoding(void); /* newline.c */ void Init_newline(void); +/* shape.c */ +void Init_default_shapes(void); + /* vm.c */ void Init_BareVM(void); void Init_vm_objects(void); diff --git a/internal/variable.h b/internal/variable.h index 47d4c86090f49b..2c832819671c42 100644 --- a/internal/variable.h +++ b/internal/variable.h @@ -50,6 +50,7 @@ void rb_obj_replace_fields(VALUE obj, VALUE fields_obj); VALUE rb_obj_complex_fields_build(VALUE obj); VALUE rb_obj_field_get(VALUE obj, shape_id_t target_shape_id); void rb_ivar_set_internal(VALUE obj, ID id, VALUE val); +void rb_check_ivar_modifiable(VALUE obj); void rb_ivar_foreach_buffered(VALUE obj, int (*func)(ID name, VALUE val, st_data_t arg), st_data_t arg); attr_index_t rb_ivar_set_index(VALUE obj, ID id, VALUE val); attr_index_t rb_obj_field_set(VALUE obj, shape_id_t target_shape_id, ID field_name, VALUE val); diff --git a/ractor.c b/ractor.c index 3c2b54425cebb8..110294a94a37f2 100644 --- a/ractor.c +++ b/ractor.c @@ -715,8 +715,7 @@ ractor_alloc(VALUE klass) { rb_ractor_t *r; VALUE rv = TypedData_Make_Struct(klass, rb_ractor_t, &ractor_data_type, r); - FL_SET_RAW(rv, RUBY_FL_SHAREABLE); - rb_gc_obj_became_shareable(rv); + RB_OBJ_SET_SHAREABLE(rv); r->pub.self = rv; r->next_ec_serial = 1; VM_ASSERT(ractor_status_p(r, ractor_created)); @@ -831,8 +830,7 @@ void rb_ractor_main_setup(rb_vm_t *vm, rb_ractor_t *r, rb_thread_t *th) { VALUE rv = r->pub.self = TypedData_Wrap_Struct(rb_cRactor, &ractor_data_type, r); - FL_SET_RAW(r->pub.self, RUBY_FL_SHAREABLE); - rb_gc_obj_became_shareable(r->pub.self); + RB_OBJ_SET_SHAREABLE(r->pub.self); ractor_init(r, Qnil, Qnil); r->threads.main = th; rb_ractor_living_threads_insert(r, th); @@ -1469,6 +1467,16 @@ rb_obj_set_shareable_no_assert(VALUE obj) FL_SET_RAW(obj, FL_SHAREABLE); rb_gc_obj_became_shareable(obj); + /* Ivars on a shareable object would be mutable shared state, so freeze them + * (not obj itself). A T_IMEMO has no shape id to transition. */ + bool froze_ivars = false; + if (!RB_OBJ_FROZEN_RAW(obj) && !RB_TYPE_P(obj, T_IMEMO) && + !RB_TYPE_P(obj, T_CLASS) && !RB_TYPE_P(obj, T_MODULE) && !RB_TYPE_P(obj, T_ICLASS)) { + + RBASIC_SET_SHAPE_ID(obj, rb_shape_transition_frozen(RBASIC_SHAPE_ID(obj))); + froze_ivars = true; + } + /* A T_OBJECT can have a fields imemo too (too_complex and friends), and an imemo * born while its owner was unshareable stays unshareable * (imemo_fields_complex_from_obj), so align it here. */ @@ -1481,6 +1489,8 @@ rb_obj_set_shareable_no_assert(VALUE obj) // no recursive mark FL_SET_RAW(fields, FL_SHAREABLE); rb_gc_obj_became_shareable(fields); + // the imemo carries its owner's shape id, frozen bit included + if (froze_ivars) RBASIC_SET_SHAPE_ID(fields, RBASIC_SHAPE_ID(obj)); // Field values the traversal never reaches (hidden internal ivars, say) // can stay unshareable, so record their shrefs to keep the shareable // fields imemo's edges correct. @@ -3422,9 +3432,12 @@ ractor_native_shallow_copy(VALUE obj) } /* The traversal rewrites the children inside the copy with raw stores, so the frozen - * bit can be set now: by the time leave runs the original is out of sight. */ + * bit can be set now: by the time leave runs the original is out of sight. The shape + * has to be transitioned along with the flag, because field writes are refused based + * on the shape (see rb_check_ivar_modifiable). */ if (OBJ_FROZEN(obj)) { RB_FL_SET_RAW(copy, RUBY_FL_FREEZE); + RBASIC_SET_SHAPE_ID(copy, rb_obj_shape_transition_frozen(copy)); } return copy; } diff --git a/ractor_sync.c b/ractor_sync.c index 3eb83a6550b30e..047ea540b5317f 100644 --- a/ractor_sync.c +++ b/ractor_sync.c @@ -1016,8 +1016,7 @@ rb_ractor_setup_default_port(rb_ractor_t *r) { VM_ASSERT(r->sync.default_port_value == Qfalse); r->sync.default_port_value = ractor_port_new(r); - FL_SET_RAW(r->sync.default_port_value, RUBY_FL_SHAREABLE); // only default ports are shareable - rb_gc_obj_became_shareable(r->sync.default_port_value); + RB_OBJ_SET_SHAREABLE(r->sync.default_port_value); } // Ractor#value diff --git a/shape.h b/shape.h index 2a416b1b126e32..de83d06b876a41 100644 --- a/shape.h +++ b/shape.h @@ -29,7 +29,8 @@ STATIC_ASSERT(shape_id_num_bits, SHAPE_ID_NUM_BITS == sizeof(shape_id_t) * CHAR_ // 26 SHAPE_ID_FL_COMPLEX // The object is backed by a `st_table`. // 27 SHAPE_ID_FL_FROZEN -// Whether the object is frozen or not. +// Whether field writes are refused: either the object is frozen, or it +// became shareable while unfrozen (see rb_obj_set_shareable in ractor.c). // 28 SHAPE_ID_FL_HAS_OBJECT_ID // Whether the object has an `SHAPE_OBJ_ID` transition. // 29-30 SHAPE_ID_LAYOUT_MASK diff --git a/test/ruby/test_array.rb b/test/ruby/test_array.rb index de629760ca79aa..3d222dd55825ef 100644 --- a/test/ruby/test_array.rb +++ b/test/ruby/test_array.rb @@ -2977,6 +2977,18 @@ def test_values_at2 assert_equal([nil], a.values_at(2**31-1)) end + def test_values_at_ary_modify + a = (0..100_000).to_a + obj = Object.new + obj.define_singleton_method(:begin) do + a.clear + 0 + end + obj.define_singleton_method(:end) { 10_000 } + obj.define_singleton_method(:exclude_end?) { false } + assert_equal(10_001, a.values_at(obj).length) + end + def test_select assert_equal([0, 2], [0, 1, 2, 3].select {|x| x % 2 == 0 }) end diff --git a/test/ruby/test_env.rb b/test/ruby/test_env.rb index 48409f843566e9..3e928a5b52b9ac 100644 --- a/test/ruby/test_env.rb +++ b/test/ruby/test_env.rb @@ -1474,25 +1474,13 @@ def test_shared_substring_in_ractor end; end - def test_ivar_in_env_should_not_be_access_from_non_main_ractors + def test_ivar_in_env_is_not_allowed + # ENV is shareable but can never be frozen, so it may not carry instance + # variables at all: they would be unshareable values reachable from any ractor. assert_ractor <<~RUBY - ENV.instance_eval{ @a = "hello" } - assert_equal "hello", ENV.instance_variable_get(:@a) - - r_get = Ractor.new do - ENV.instance_variable_get(:@a) - rescue Ractor::IsolationError => e - e - end - assert_equal Ractor::IsolationError, r_get.value.class - - r_get = Ractor.new do - ENV.instance_eval{ @a } - rescue Ractor::IsolationError => e - e - end - - assert_equal Ractor::IsolationError, r_get.value.class + assert_raise(Ractor::IsolationError) { ENV.instance_eval{ @a = "hello" } } + assert_nil ENV.instance_variable_get(:@a) + assert_equal [], ENV.instance_variables r_set = Ractor.new do ENV.instance_eval{ @b = "hello" } @@ -1501,6 +1489,13 @@ def test_ivar_in_env_should_not_be_access_from_non_main_ractors end assert_equal Ractor::IsolationError, r_set.value.class + + # Reads are allowed: since writes are forbidden, there is nothing + # unshareable to read. + r_get = Ractor.new do + ENV.instance_variable_get(:@a) + end + assert_nil r_get.value RUBY end diff --git a/variable.c b/variable.c index 9eab776a5547e6..a5e5d465fc562b 100644 --- a/variable.c +++ b/variable.c @@ -30,6 +30,7 @@ #include "internal/object.h" #include "internal/gc.h" #include "internal/re.h" +#include "internal/string.h" #include "internal/struct.h" #include "internal/symbol.h" #include "internal/thread.h" @@ -1280,19 +1281,16 @@ cvar_read_ractor_check(VALUE klass, ID id, VALUE val) } static inline void -ivar_ractor_check(VALUE obj, ID id) +ivar_ractor_assert(VALUE obj, ID id) { - if (LIKELY(rb_is_instance_id(id)) /* not internal ID */ && - !RB_OBJ_FROZEN_RAW(obj) && - UNLIKELY(!rb_ractor_main_p()) && - UNLIKELY(rb_ractor_shareable_p(obj))) { - - if (RB_TYPE_P(obj, T_CLASS) || RB_TYPE_P(obj, T_MODULE)) { - // classes/modules are owner-checked at each read/write site instead - return; - } - rb_raise(rb_eRactorIsolationError, "can not access instance variables of shareable objects from non-main Ractors"); - } + RUBY_ASSERT(!rb_is_instance_id(id) /* internal ID */ || + SPECIAL_CONST_P(obj) || + !rb_ractor_shareable_p(obj) || + RB_OBJ_FROZEN_RAW(obj) || + RB_TYPE_P(obj, T_CLASS) || RB_TYPE_P(obj, T_MODULE) || + RB_TYPE_P(obj, T_ICLASS) || RB_TYPE_P(obj, T_IMEMO) || + rb_shape_frozen_p(RBASIC_SHAPE_ID(obj)), + "shareable object must not have writable instance variables"); } struct st_table * @@ -1378,7 +1376,7 @@ obj_use_generic_fields_tbl_p(VALUE obj) VALUE rb_obj_fields(VALUE obj, ID field_name) { - ivar_ractor_check(obj, field_name); + ivar_ractor_assert(obj, field_name); switch (BUILTIN_TYPE(obj)) { case T_IMEMO: @@ -1467,7 +1465,7 @@ rb_free_generic_ivar(VALUE obj) static void rb_obj_set_fields(VALUE obj, VALUE fields_obj, ID field_name, VALUE original_fields_obj) { - ivar_ractor_check(obj, field_name); + ivar_ractor_assert(obj, field_name); if (!fields_obj) { RUBY_ASSERT(original_fields_obj); @@ -2035,6 +2033,25 @@ obj_ivar_set(VALUE obj, ID id, VALUE val) return obj_field_set(obj, target_shape_id, id, val); } +void +rb_check_ivar_modifiable(VALUE obj) +{ + if (UNLIKELY(!RB_FL_ABLE(obj) || rb_shape_frozen_p(RBASIC_SHAPE_ID(obj)))) { + rb_check_frozen(obj); + + RUBY_ASSERT(RB_OBJ_SHAREABLE_P(obj), "unfrozen object with a frozen shape must be shareable"); + + rb_raise(rb_eRactorIsolationError, + "can't modify instance variables of a shareable %"PRIsVALUE, + rb_obj_class(obj)); + } + else if (UNLIKELY(CHILLED_STRING_P(obj))) { + CHILLED_STRING_MUTATED(obj); + } + + RUBY_ASSERT(!RB_OBJ_FROZEN_RAW(obj), "frozen object with an unfrozen shape"); +} + /* Set the instance variable +val+ on object +obj+ at ivar name +id+. * This function only works with T_OBJECT objects, so make sure * +obj+ is of type T_OBJECT before using this function. @@ -2042,7 +2059,7 @@ obj_ivar_set(VALUE obj, ID id, VALUE val) VALUE rb_vm_set_ivar_id(VALUE obj, ID id, VALUE val) { - rb_check_frozen(obj); + rb_check_ivar_modifiable(obj); obj_ivar_set(obj, id, val); return val; } @@ -2101,7 +2118,7 @@ ivar_set(VALUE obj, ID id, VALUE val) VALUE rb_ivar_set(VALUE obj, ID id, VALUE val) { - rb_check_frozen(obj); + rb_check_ivar_modifiable(obj); ivar_set(obj, id, val); return val; } diff --git a/vm.c b/vm.c index 06da903cf1b073..4dd49f0db47fdb 100644 --- a/vm.c +++ b/vm.c @@ -1566,6 +1566,16 @@ proc_isolate_env(VALUE self, rb_proc_t *proc, VALUE read_only_variables) RB_OBJ_WRITTEN(self, Qundef, env); } +static int +proc_has_ivar_i(ID name, VALUE val, st_data_t arg) +{ + if (rb_is_instance_id(name)) { + *(bool *)arg = true; + return ST_STOP; + } + return ST_CONTINUE; +} + static VALUE proc_shared_outer_variables(struct rb_id_table *outer_variables, bool isolate, const char *message) { @@ -1623,6 +1633,16 @@ rb_proc_isolate_bang(VALUE self, VALUE replace_self) RB_OBJ_WRITE(self, &proc->block.as.captured.self, Qnil); } + /* ivars are not traversed here, so their values may be unshareable */ + if (UNLIKELY(rb_obj_shape_has_ivars(self))) { + bool has_ivar = false; + rb_ivar_foreach(self, proc_has_ivar_i, (st_data_t)&has_ivar); + + if (has_ivar) { + rb_raise(rb_eRactorIsolationError, "can not isolate a Proc because it has instance variables"); + } + } + RB_OBJ_SET_SHAREABLE(self); return self; } diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 1f9d8acc805b09..026b93a6f5ab34 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1394,7 +1394,7 @@ vm_setivar_slowpath(VALUE obj, ID id, VALUE val, const rb_iseq_t *iseq, IVC ic, #if OPT_IC_FOR_IVAR RB_DEBUG_COUNTER_INC(ivar_set_ic_miss); - rb_check_frozen(obj); + rb_check_ivar_modifiable(obj); shape_id_t previous_shape_id = RBASIC_SHAPE_ID(obj); attr_index_t index = rb_ivar_set_index(obj, id, val); diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index a95d66fe911d7e..79d8036451baa1 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -1,15 +1,15 @@ -use std::cell::{Cell, RefCell}; +use std::cell::Cell; use std::collections::hash_map::Entry; use std::collections::{HashMap, HashSet, VecDeque}; use std::fmt; use std::mem::take; use std::rc::Rc; use crate::bitset::BitSet; -use crate::codegen::{perf_symbol_range_start, perf_symbol_range_end, register_with_perf}; +use crate::perf; use crate::cruby::{IseqPtr, RUBY_OFFSET_CFP_ISEQ, RUBY_OFFSET_CFP_JIT_RETURN, RUBY_OFFSET_CFP_PC, RUBY_OFFSET_CFP_SP, SIZEOF_VALUE_I32, VALUE, ZJIT_STACK_MAP_BASE_PTR_INDEX_MASK, ZJIT_STACK_MAP_BASE_PTR_SIZE_SHIFT, ZJIT_STACK_MAP_BASE_PTR_TAG, ZJIT_STACK_MAP_SHIFT, ZJIT_STACK_MAP_SKIP_TAG, ZJIT_STACK_MAP_VREG_TAG, vm_stack_canary, zjit_jit_frame, local_size_and_idx_to_ep_offset}; use crate::hir::{Invariant, SideExitReason}; use crate::hir; -use crate::options::{TraceExits, PerfMap, get_option}; +use crate::options::{TraceExits, get_option}; use crate::payload::IseqVersionRef; use crate::stats::{exit_counter_ptr, exit_counter_ptr_for_opcode, side_exit_counter, CompileError}; use crate::virtualmem::CodePtr; @@ -2021,42 +2021,6 @@ impl Assembler } pub fn linearize_instructions(&self) -> Vec { - // Wrap instructions emitted by `push_insns` with PosMarkers and record - // the emitted byte range under `symbol_name` in the perf map. - fn push_insns_with_perf_symbol( - insns: &mut Vec, - symbol_name: &str, - push_insns: impl FnOnce(&mut Vec), - ) { - // ISEQ perf symbols cover the whole compiled ISEQ, including this - // padding. HIR perf needs a separate symbol because the padding - // doesn't belong to any HIR instruction. - if get_option!(perf) != Some(PerfMap::HIR) { - push_insns(insns); - return; - } - - let symbol_name = symbol_name.to_string(); - let start = Rc::new(RefCell::new(None)); - let current = start.clone(); - insns.push(Insn::PosMarker(Rc::new(move |code_ptr, _| { - let mut current = current.borrow_mut(); - assert!(current.is_none(), "perf symbol range already open"); - *current = Some(code_ptr); - }))); - - push_insns(insns); - - insns.push(Insn::PosMarker(Rc::new(move |end, cb| { - if let Some(start) = start.borrow_mut().take() { - let start_addr = start.raw_addr(cb); - let end_addr = end.raw_addr(cb); - if start_addr < end_addr { - register_with_perf(symbol_name.clone(), start_addr, end_addr - start_addr); - } - } - }))); - } // Emit instructions with labels, expanding branch parameters let mut insns = Vec::with_capacity(ASSEMBLER_INSNS_CAPACITY); @@ -2067,7 +2031,7 @@ impl Assembler // Entry blocks shouldn't ever be preceded by something that can // stomp on this block. if !block.is_entry { - push_insns_with_perf_symbol(&mut insns, "BoundaryPad", |insns| { + perf::push_insns_with_synthetic_symbol(&mut insns, "BoundaryPad", |insns| { insns.push(Insn::BoundaryPad); }); } @@ -2100,7 +2064,7 @@ impl Assembler } } // Make sure we don't stomp on the next function - push_insns_with_perf_symbol(&mut insns, "BoundaryPad", |insns| { + perf::push_insns_with_synthetic_symbol(&mut insns, "BoundaryPad", |insns| { insns.push(Insn::BoundaryPad); }); @@ -3220,12 +3184,8 @@ impl Assembler // Map from SideExit to compiled Label. This table is used to deduplicate side exit code. let mut compiled_exits: HashMap = HashMap::with_capacity(targets.len()); - // Start a new perf range for side exits - let perf_symbol = if get_option!(perf) == Some(PerfMap::HIR) { - Some(perf_symbol_range_start(self, "side exit")) - } else { - None - }; + // Start a new perf range for side exits. + let symbol_range = perf::symbol_range_start(self, "side exit"); // Mark the start of side-exit code so we can measure its size if !targets.is_empty() { @@ -3306,8 +3266,8 @@ impl Assembler } // Close the current perf range for side exits - if let Some(perf_symbol) = &perf_symbol { - perf_symbol_range_end(self, perf_symbol); + if let Some(symbol_range) = &symbol_range { + perf::symbol_range_end(self, symbol_range); } // Extract exit instructions and restore the previous current block diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 58143e819f53c1..3f226af650c926 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -18,6 +18,7 @@ use crate::invariants::{ use crate::gc::append_gc_offsets; use crate::payload::{IseqCodePtrs, IseqStatus, IseqVersion, IseqVersionRef, JITFrame, get_or_create_iseq_payload}; use crate::profile::reset_profiles_remaining; +use crate::perf; use crate::state::{rb_zjit_compiling_p, ZJITState}; use crate::stats::{CompileError, exit_counter_for_compile_error, exit_counter_for_unhandled_hir_insn, incr_counter, incr_counter_by, send_fallback_counter, send_fallback_counter_for_method_type, send_fallback_counter_for_super_method_type, send_fallback_counter_ptr_for_opcode, send_fallback_counter_for_optimized_method_type}; use crate::stats::{counter_ptr, with_time_stat, trace_compile_phase, Counter, Counter::{compile_time_ns, exit_compile_error}}; @@ -26,7 +27,7 @@ use crate::backend::lir::{self, Assembler, CArgLocation, C_ARG_OPNDS, C_RET_OPND use crate::hir::{self, iseq_to_hir, BlockId, Invariant, RangeType, SideExitReason::{self, *}, SpecialBackrefSymbol, SpecialObjectType}; use crate::hir::{BlockHandler, CCallVariadicData, CCallWithFrameData, Const, FieldName, FrameState, Function, Insn, InsnId, Recompile, SendDirectData, SendFallbackReason, qualified_method_name}; use crate::hir_type::{types, Type}; -use crate::options::{get_option, InlineDepth, PerfMap, DEFAULT_MAX_VERSIONS}; +use crate::options::{get_option, InlineDepth, DEFAULT_MAX_VERSIONS}; use crate::cast::IntoUsize; /// Maximum number of compiled versions per ISEQ. @@ -313,31 +314,6 @@ pub fn gen_iseq_call(cb: &mut CodeBlock, iseq_call: &IseqCallRef) -> Result<(), }) } -/// Write an entry to the perf map in /tmp -pub(crate) fn register_with_perf(symbol_name: String, start_ptr: usize, code_size: usize) { - use std::io::Write; - let perf_map = format!("/tmp/perf-{}.map", std::process::id()); - let Ok(file) = std::fs::OpenOptions::new().create(true).append(true).open(&perf_map) else { - debug!("Failed to open perf map file: {perf_map}"); - return; - }; - let mut file = std::io::BufWriter::new(file); - let Ok(_) = writeln!(file, "{start_ptr:#x} {code_size:#x} ZJIT: {symbol_name}") else { - debug!("Failed to write {symbol_name} to perf map file: {perf_map}"); - return; - }; -} - -/// Register the code emitted from `start` through the current write pointer -/// under `symbol_name` in the perf map, if perf output is enabled. -fn register_current_code_range_with_perf(cb: &CodeBlock, symbol_name: &str, start: CodePtr) { - if get_option!(perf).is_some() { - let start_ptr = start.raw_addr(cb); - let end_ptr = cb.get_write_ptr().raw_addr(cb); - register_with_perf(symbol_name.to_string(), start_ptr, end_ptr - start_ptr); - } -} - /// Compile a shared JIT entry trampoline pub fn gen_entry_trampoline(cb: &mut CodeBlock) -> Result { // Set up registers for CFP, EC, SP, and basic block arguments @@ -357,7 +333,7 @@ pub fn gen_entry_trampoline(cb: &mut CodeBlock) -> Result let (code_ptr, gc_offsets) = asm.compile(cb)?; assert!(gc_offsets.is_empty()); - register_current_code_range_with_perf(cb, "entry trampoline", code_ptr); + perf::register_current_code_range(cb, "entry trampoline", code_ptr); Ok(code_ptr) } @@ -513,7 +489,7 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, func // Compile all instructions for (insn_idx, &insn_id) in block.insns().enumerate() { let insn = function.find(insn_id); - let perf_symbol = hir_perf_symbol_range_start(&mut asm, &insn); + let symbol_range = perf::hir_symbol_range_start(&mut asm, &insn); let result = match &insn { Insn::CondBranch { val, if_true, if_false } => { @@ -557,12 +533,12 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, func }; // Close the current perf range for the HIR instruction. - if let Some(perf_symbol) = &perf_symbol { + if let Some(symbol_range) = &symbol_range { if result.is_ok() && insn.is_terminator() { assert!(asm.current_block().insns.last().is_some_and(|insn| insn.is_terminator())); - perf_symbol_range_end_at_block_end(&mut asm, perf_symbol); + perf::symbol_range_end_at_block_end(&mut asm, symbol_range); } else { - perf_symbol_range_end(&mut asm, perf_symbol); + perf::symbol_range_end(&mut asm, symbol_range); } } @@ -595,13 +571,7 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, func // Generate code if everything can be compiled let result = asm.compile(cb); if let Ok((start_ptr, _)) = result { - if get_option!(perf) == Some(PerfMap::ISEQ) { - let start_usize = start_ptr.raw_addr(cb); - let end_usize = cb.get_write_ptr().raw_addr(cb); - let code_size = end_usize - start_usize; - let iseq_name = iseq_get_location(iseq, 0); - register_with_perf(iseq_name, start_usize, code_size); - } + perf::register_current_iseq_range(cb, iseq, start_ptr); if ZJITState::should_log_compiled_iseqs() { let iseq_name = iseq_get_location(iseq, 0); ZJITState::log_compile(iseq_name); @@ -4124,7 +4094,7 @@ pub fn gen_function_stub_hit_trampoline(cb: &mut CodeBlock) -> Result Result asm.compile(cb).map(|(code_ptr, gc_offsets)| { assert_eq!(gc_offsets.len(), 0); - register_current_code_range_with_perf(cb, "exit trampoline", code_ptr); + perf::register_current_code_range(cb, "exit trampoline", code_ptr); code_ptr }) } @@ -4167,7 +4137,7 @@ pub fn gen_materialize_exit_trampoline(cb: &mut CodeBlock, exit_trampoline: Code asm.compile(cb).map(|(code_ptr, gc_offsets)| { assert_eq!(gc_offsets.len(), 0); - register_current_code_range_with_perf(cb, "materialize_exit trampoline", code_ptr); + perf::register_current_code_range(cb, "materialize_exit trampoline", code_ptr); code_ptr }) } @@ -4183,7 +4153,7 @@ pub fn gen_materialize_exit_trampoline_with_counter(cb: &mut CodeBlock, material asm.compile(cb).map(|(code_ptr, gc_offsets)| { assert_eq!(gc_offsets.len(), 0); - register_current_code_range_with_perf(cb, "materialize_exit_with_counter trampoline", code_ptr); + perf::register_current_code_range(cb, "materialize_exit_with_counter trampoline", code_ptr); code_ptr }) } @@ -4468,60 +4438,6 @@ impl IseqCall { } } -type PerfSymbol = Rc>>; - -/// Start a HIR perf symbol range when --zjit-perf=hir is enabled. -fn hir_perf_symbol_range_start(asm: &mut Assembler, insn: &Insn) -> Option { - if get_option!(perf) == Some(PerfMap::HIR) { - let insn_name = format!("{insn}").split_whitespace().next().unwrap().to_string(); - Some(perf_symbol_range_start(asm, &insn_name)) - } else { - None - } -} - -/// Mark the start of a perf symbol range via pos_marker. -/// Returns a handle to pass to perf_symbol_range_end. -pub fn perf_symbol_range_start(asm: &mut Assembler, symbol_name: &str) -> PerfSymbol { - let symbol_name = symbol_name.to_string(); - let perf_symbol: PerfSymbol = Rc::new(RefCell::new(None)); - let current = perf_symbol.clone(); - asm.pos_marker(move |start, _| { - let mut current = current.borrow_mut(); - assert!(current.is_none(), "perf symbol range already open"); - *current = Some((start, symbol_name.clone())); - }); - perf_symbol -} - -/// Mark the end of a perf symbol range via pos_marker. -pub fn perf_symbol_range_end(asm: &mut Assembler, perf_symbol: &PerfSymbol) { - let current = perf_symbol.clone(); - asm.pos_marker(move |end, cb| { - if let Some((start, name)) = current.borrow_mut().take() { - let start_addr = start.raw_addr(cb); - let code_size = end.raw_addr(cb) - start_addr; - register_with_perf(name, start_addr, code_size); - } - }); -} - -/// Mark the end of a perf symbol range at the end of the current LIR block. -pub fn perf_symbol_range_end_at_block_end(asm: &mut Assembler, perf_symbol: &PerfSymbol) { - let current = perf_symbol.clone(); - asm.pos_marker_at_block_end(move |end, cb| { - if let Some((start, name)) = current.borrow_mut().take() { - let start_addr = start.raw_addr(cb); - let end_addr = end.raw_addr(cb); - // A terminator's jump can be removed when it targets the next - // linear block, leaving no code between the range start and the - // block-end marker. Skip zero-sized perf map entries. - if start_addr < end_addr { - register_with_perf(name, start_addr, end_addr - start_addr); - } - } - }); -} #[cfg(test)] #[path = "codegen_tests.rs"] diff --git a/zjit/src/lib.rs b/zjit/src/lib.rs index 552b9d2c7e39e3..25881b871eaa88 100644 --- a/zjit/src/lib.rs +++ b/zjit/src/lib.rs @@ -29,6 +29,7 @@ mod backend; #[cfg(feature = "disasm")] mod disasm; mod options; +mod perf; mod profile; mod invariants; mod bitset; diff --git a/zjit/src/perf.rs b/zjit/src/perf.rs new file mode 100644 index 00000000000000..7c883134c51a22 --- /dev/null +++ b/zjit/src/perf.rs @@ -0,0 +1,140 @@ +//! Linux perf(1) [annotations](https://github.com/torvalds/linux/blob/v7.2/tools/perf/Documentation/jit-interface.txt) that help to symbolicate generated code. + +use std::cell::RefCell; +use std::rc::Rc; + +use crate::asm::CodeBlock; +use crate::cruby::{IseqPtr, iseq_get_location}; +use crate::backend::lir::{Assembler, Insn as LirInsn}; +use crate::hir::Insn; +use crate::options::{get_option, PerfMap}; +use crate::options::debug; +use crate::virtualmem::CodePtr; + +type SymbolRange = Rc>>; + +/// Register a non-empty code range under `symbol_name` in the perf map. +pub(crate) fn register_range(cb: &CodeBlock, symbol_name: String, start: CodePtr, end: CodePtr) { + let start_ptr = start.raw_addr(cb); + let end_ptr = end.raw_addr(cb); + if start_ptr < end_ptr { + register(symbol_name, start_ptr, end_ptr - start_ptr); + } +} + +/// Register the code emitted from `start` through the current write pointer +/// under `symbol_name` in the perf map, if perf output is enabled. +pub(crate) fn register_current_code_range(cb: &CodeBlock, symbol_name: &str, start: CodePtr) { + if get_option!(perf).is_some() { + register_range(cb, symbol_name.to_string(), start, cb.get_write_ptr()); + } +} + +/// Register an ISEQ code range when ISEQ perf output is enabled. +pub(crate) fn register_current_iseq_range(cb: &CodeBlock, iseq: IseqPtr, start: CodePtr) { + if get_option!(perf) == Some(PerfMap::ISEQ) { + register_range(cb, iseq_get_location(iseq, 0), start, cb.get_write_ptr()); + } +} + +/// Start a HIR symbol range when HIR perf output is enabled. +pub(crate) fn hir_symbol_range_start(asm: &mut Assembler, insn: &Insn) -> Option { + let symbol_range = new_hir_symbol_range()?; + let insn_name = format!("{insn}"); + Some(install_symbol_range_start(asm, symbol_range, insn_name.split_whitespace().next().unwrap())) +} + +/// Mark the start of a symbol range when HIR perf output is enabled. +/// Returns None otherwise. +pub(crate) fn symbol_range_start(asm: &mut Assembler, symbol_name: &str) -> Option { + let symbol_range = new_hir_symbol_range()?; + Some(install_symbol_range_start(asm, symbol_range, symbol_name)) +} + +/// Mark the end of a symbol range via pos_marker. +pub(crate) fn symbol_range_end(asm: &mut Assembler, symbol_range: &SymbolRange) { + asm.pos_marker(symbol_range_end_marker(symbol_range)); +} + +/// Mark the end of a symbol range at the end of the current LIR block. +/// A terminator jump can be removed when it targets the next linear block. +/// This can leave an empty range. `register_range` skips that entry. +pub(crate) fn symbol_range_end_at_block_end(asm: &mut Assembler, symbol_range: &SymbolRange) { + asm.pos_marker_at_block_end(symbol_range_end_marker(symbol_range)); +} + +/// Push instructions under a synthetic HIR perf symbol. +/// HIR output maps each HIR instruction to its emitted code range. +/// This records code with no HIR instruction, such as `BoundaryPad`, under `symbol_name`. +pub(crate) fn push_insns_with_synthetic_symbol( + insns: &mut Vec, + symbol_name: &str, + push_insns: impl FnOnce(&mut Vec), +) { + let Some(symbol_range) = start_synthetic_hir_symbol_range(insns, symbol_name) else { + push_insns(insns); + return; + }; + + push_insns(insns); + insns.push(LirInsn::PosMarker(Rc::new(symbol_range_end_marker(&symbol_range)))); +} + +/// Write an entry to the perf map in /tmp. +fn register(symbol_name: String, start_ptr: usize, code_size: usize) { + use std::io::Write; + let perf_map = format!("/tmp/perf-{}.map", std::process::id()); + let Ok(file) = std::fs::OpenOptions::new().create(true).append(true).open(&perf_map) else { + debug!("Failed to open perf map file: {perf_map}"); + return; + }; + let mut file = std::io::BufWriter::new(file); + let Ok(_) = writeln!(file, "{start_ptr:#x} {code_size:#x} ZJIT: {symbol_name}") else { + debug!("Failed to write {symbol_name} to perf map file: {perf_map}"); + return; + }; +} + +/// Add a start marker for a synthetic HIR symbol if HIR output is enabled. +fn start_synthetic_hir_symbol_range( + insns: &mut Vec, + symbol_name: &str, +) -> Option { + let symbol_range = new_hir_symbol_range()?; + insns.push(LirInsn::PosMarker(Rc::new(symbol_range_start_marker(&symbol_range, symbol_name.to_string())))); + Some(symbol_range) +} + +fn new_hir_symbol_range() -> Option { + (get_option!(perf) == Some(PerfMap::HIR)).then(|| Rc::new(RefCell::new(None))) +} + +fn install_symbol_range_start( + asm: &mut Assembler, + symbol_range: SymbolRange, + symbol_name: &str, +) -> SymbolRange { + asm.pos_marker(symbol_range_start_marker(&symbol_range, symbol_name.to_string())); + symbol_range +} + +fn symbol_range_start_marker( + symbol_range: &SymbolRange, + symbol_name: String, +) -> impl Fn(CodePtr, &CodeBlock) + 'static { + let current = symbol_range.clone(); + move |start, _| { + let mut current = current.borrow_mut(); + assert!(current.is_none(), "perf symbol range already open"); + *current = Some((start, symbol_name.clone())); + } +} + +fn symbol_range_end_marker(symbol_range: &SymbolRange) -> impl Fn(CodePtr, &CodeBlock) + 'static { + let current = symbol_range.clone(); + move |end, cb| { + if let Some((start, name)) = current.borrow_mut().take() { + register_range(cb, name, start, end); + } + } +}