diff --git a/cont.c b/cont.c index 0d7245ed9c20f1..f885cdb1095032 100644 --- a/cont.c +++ b/cont.c @@ -774,44 +774,40 @@ static void fiber_pool_stack_release(struct fiber_pool_stack * stack) { struct fiber_pool * pool = stack->pool; - RB_VM_LOCK_ENTER(); - { - struct fiber_pool_vacancy * vacancy = fiber_pool_vacancy_pointer(stack->base, stack->size); + struct fiber_pool_vacancy * vacancy = fiber_pool_vacancy_pointer(stack->base, stack->size); - if (DEBUG) fprintf(stderr, "fiber_pool_stack_release: %p used=%"PRIuSIZE"\n", stack->base, stack->pool->used); + if (DEBUG) fprintf(stderr, "fiber_pool_stack_release: %p used=%"PRIuSIZE"\n", stack->base, stack->pool->used); - // Copy the stack details into the vacancy area: - vacancy->stack = *stack; - // After this point, be careful about updating/using state in stack, since it's copied to the vacancy area. + // Copy the stack details into the vacancy area: + vacancy->stack = *stack; + // After this point, be careful about updating/using state in stack, since it's copied to the vacancy area. - // Reset the stack pointers and reserve space for the vacancy data: - fiber_pool_vacancy_reset(vacancy); + // Reset the stack pointers and reserve space for the vacancy data: + fiber_pool_vacancy_reset(vacancy); - // Push the vacancy into the vancancies list: - pool->vacancies = fiber_pool_vacancy_push(vacancy, pool->vacancies); - pool->used -= 1; + // Push the vacancy into the vancancies list: + pool->vacancies = fiber_pool_vacancy_push(vacancy, pool->vacancies); + pool->used -= 1; #ifdef FIBER_POOL_ALLOCATION_FREE - struct fiber_pool_allocation * allocation = stack->allocation; + struct fiber_pool_allocation * allocation = stack->allocation; - allocation->used -= 1; + allocation->used -= 1; - // Release address space and/or dirty memory: - if (allocation->used == 0) { - fiber_pool_allocation_free(allocation); - } - else if (stack->pool->free_stacks) { - fiber_pool_stack_free(&vacancy->stack); - } + // Release address space and/or dirty memory: + if (allocation->used == 0) { + fiber_pool_allocation_free(allocation); + } + else if (stack->pool->free_stacks) { + fiber_pool_stack_free(&vacancy->stack); + } #else - // This is entirely optional, but clears the dirty flag from the stack - // memory, so it won't get swapped to disk when there is memory pressure: - if (stack->pool->free_stacks) { - fiber_pool_stack_free(&vacancy->stack); - } -#endif + // This is entirely optional, but clears the dirty flag from the stack + // memory, so it won't get swapped to disk when there is memory pressure: + if (stack->pool->free_stacks) { + fiber_pool_stack_free(&vacancy->stack); } - RB_VM_LOCK_LEAVE(); +#endif } static inline void @@ -924,6 +920,17 @@ fiber_stack_release(rb_fiber_t * fiber) rb_ec_clear_vm_stack(ec); } +static void +fiber_stack_release_locked(rb_fiber_t *fiber) +{ + if (!ruby_vm_during_cleanup) { + // We can't try to acquire the VM lock here because MMTK calls free in its own native thread which has no ec. + // This assertion will fail on MMTK but we currently don't have CI for debug releases of MMTK, so we can assert for now. + ASSERT_vm_locking_with_barrier(); + } + fiber_stack_release(fiber); +} + static const char * fiber_status_name(enum fiber_status s) { @@ -1084,7 +1091,7 @@ cont_free(void *ptr) else { rb_fiber_t *fiber = (rb_fiber_t*)cont; coroutine_destroy(&fiber->context); - fiber_stack_release(fiber); + fiber_stack_release_locked(fiber); } RUBY_FREE_UNLESS_NULL(cont->saved_vm_stack.ptr); @@ -2741,7 +2748,9 @@ fiber_switch(rb_fiber_t *fiber, int argc, const VALUE *argv, int kw_splat, rb_fi // We cannot free the stack until the pthread is joined: #ifndef COROUTINE_PTHREAD_CONTEXT if (resuming_fiber && FIBER_TERMINATED_P(fiber)) { - fiber_stack_release(fiber); + RB_VM_LOCKING() { + fiber_stack_release(fiber); + } } #endif diff --git a/prism/prism.c b/prism/prism.c index cc1896ee3486ed..0ebcae62f9e822 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -8591,64 +8591,64 @@ parser_lex_magic_comment(pm_parser_t *parser, bool semantic_token_seen) { static const uint32_t context_terminators[] = { [PM_CONTEXT_NONE] = 0, - [PM_CONTEXT_BEGIN] = (1 << PM_TOKEN_KEYWORD_ENSURE) | (1 << PM_TOKEN_KEYWORD_RESCUE) | (1 << PM_TOKEN_KEYWORD_ELSE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_BEGIN_ENSURE] = (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_BEGIN_ELSE] = (1 << PM_TOKEN_KEYWORD_ENSURE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_BEGIN_RESCUE] = (1 << PM_TOKEN_KEYWORD_ENSURE) | (1 << PM_TOKEN_KEYWORD_RESCUE) | (1 << PM_TOKEN_KEYWORD_ELSE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_BLOCK_BRACES] = (1 << PM_TOKEN_BRACE_RIGHT), - [PM_CONTEXT_BLOCK_KEYWORDS] = (1 << PM_TOKEN_KEYWORD_END) | (1 << PM_TOKEN_KEYWORD_RESCUE) | (1 << PM_TOKEN_KEYWORD_ENSURE), - [PM_CONTEXT_BLOCK_ENSURE] = (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_BLOCK_ELSE] = (1 << PM_TOKEN_KEYWORD_ENSURE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_BLOCK_RESCUE] = (1 << PM_TOKEN_KEYWORD_ENSURE) | (1 << PM_TOKEN_KEYWORD_RESCUE) | (1 << PM_TOKEN_KEYWORD_ELSE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_CASE_WHEN] = (1 << PM_TOKEN_KEYWORD_WHEN) | (1 << PM_TOKEN_KEYWORD_END) | (1 << PM_TOKEN_KEYWORD_ELSE), - [PM_CONTEXT_CASE_IN] = (1 << PM_TOKEN_KEYWORD_IN) | (1 << PM_TOKEN_KEYWORD_END) | (1 << PM_TOKEN_KEYWORD_ELSE), - [PM_CONTEXT_CLASS] = (1 << PM_TOKEN_KEYWORD_END) | (1 << PM_TOKEN_KEYWORD_RESCUE) | (1 << PM_TOKEN_KEYWORD_ENSURE), - [PM_CONTEXT_CLASS_ENSURE] = (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_CLASS_ELSE] = (1 << PM_TOKEN_KEYWORD_ENSURE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_CLASS_RESCUE] = (1 << PM_TOKEN_KEYWORD_ENSURE) | (1 << PM_TOKEN_KEYWORD_RESCUE) | (1 << PM_TOKEN_KEYWORD_ELSE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_DEF] = (1 << PM_TOKEN_KEYWORD_END) | (1 << PM_TOKEN_KEYWORD_RESCUE) | (1 << PM_TOKEN_KEYWORD_ENSURE), - [PM_CONTEXT_DEF_ENSURE] = (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_DEF_ELSE] = (1 << PM_TOKEN_KEYWORD_ENSURE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_DEF_RESCUE] = (1 << PM_TOKEN_KEYWORD_ENSURE) | (1 << PM_TOKEN_KEYWORD_RESCUE) | (1 << PM_TOKEN_KEYWORD_ELSE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_DEF_PARAMS] = (1 << PM_TOKEN_EOF), - [PM_CONTEXT_DEFINED] = (1 << PM_TOKEN_EOF), - [PM_CONTEXT_DEFAULT_PARAMS] = (1 << PM_TOKEN_COMMA) | (1 << PM_TOKEN_PARENTHESIS_RIGHT), - [PM_CONTEXT_ELSE] = (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_ELSIF] = (1 << PM_TOKEN_KEYWORD_ELSE) | (1 << PM_TOKEN_KEYWORD_ELSIF) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_EMBEXPR] = (1 << PM_TOKEN_EMBEXPR_END), - [PM_CONTEXT_FOR] = (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_FOR_INDEX] = (1 << PM_TOKEN_KEYWORD_IN), - [PM_CONTEXT_IF] = (1 << PM_TOKEN_KEYWORD_ELSE) | (1 << PM_TOKEN_KEYWORD_ELSIF) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_LAMBDA_BRACES] = (1 << PM_TOKEN_BRACE_RIGHT), - [PM_CONTEXT_LAMBDA_DO_END] = (1 << PM_TOKEN_KEYWORD_END) | (1 << PM_TOKEN_KEYWORD_RESCUE) | (1 << PM_TOKEN_KEYWORD_ENSURE), - [PM_CONTEXT_LAMBDA_ENSURE] = (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_LAMBDA_ELSE] = (1 << PM_TOKEN_KEYWORD_ENSURE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_LAMBDA_RESCUE] = (1 << PM_TOKEN_KEYWORD_ENSURE) | (1 << PM_TOKEN_KEYWORD_RESCUE) | (1 << PM_TOKEN_KEYWORD_ELSE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_LOOP_PREDICATE] = (1 << PM_TOKEN_KEYWORD_DO) | (1 << PM_TOKEN_KEYWORD_THEN), - [PM_CONTEXT_MAIN] = (1 << PM_TOKEN_EOF), - [PM_CONTEXT_MODULE] = (1 << PM_TOKEN_KEYWORD_END) | (1 << PM_TOKEN_KEYWORD_RESCUE) | (1 << PM_TOKEN_KEYWORD_ENSURE), - [PM_CONTEXT_MODULE_ENSURE] = (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_MODULE_ELSE] = (1 << PM_TOKEN_KEYWORD_ENSURE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_MODULE_RESCUE] = (1 << PM_TOKEN_KEYWORD_ENSURE) | (1 << PM_TOKEN_KEYWORD_RESCUE) | (1 << PM_TOKEN_KEYWORD_ELSE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_MULTI_TARGET] = (1 << PM_TOKEN_EOF), - [PM_CONTEXT_PARENS] = (1 << PM_TOKEN_PARENTHESIS_RIGHT), - [PM_CONTEXT_POSTEXE] = (1 << PM_TOKEN_BRACE_RIGHT), - [PM_CONTEXT_PREDICATE] = (1 << PM_TOKEN_KEYWORD_THEN) | (1 << PM_TOKEN_NEWLINE) | (1 << PM_TOKEN_SEMICOLON), - [PM_CONTEXT_PREEXE] = (1 << PM_TOKEN_BRACE_RIGHT), - [PM_CONTEXT_RESCUE_MODIFIER] = (1 << PM_TOKEN_EOF), - [PM_CONTEXT_SCLASS] = (1 << PM_TOKEN_KEYWORD_END) | (1 << PM_TOKEN_KEYWORD_RESCUE) | (1 << PM_TOKEN_KEYWORD_ENSURE), - [PM_CONTEXT_SCLASS_ENSURE] = (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_SCLASS_ELSE] = (1 << PM_TOKEN_KEYWORD_ENSURE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_SCLASS_RESCUE] = (1 << PM_TOKEN_KEYWORD_ENSURE) | (1 << PM_TOKEN_KEYWORD_RESCUE) | (1 << PM_TOKEN_KEYWORD_ELSE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_TERNARY] = (1 << PM_TOKEN_EOF), - [PM_CONTEXT_UNLESS] = (1 << PM_TOKEN_KEYWORD_ELSE) | (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_UNTIL] = (1 << PM_TOKEN_KEYWORD_END), - [PM_CONTEXT_WHILE] = (1 << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_BEGIN] = (1U << PM_TOKEN_KEYWORD_ENSURE) | (1U << PM_TOKEN_KEYWORD_RESCUE) | (1U << PM_TOKEN_KEYWORD_ELSE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_BEGIN_ENSURE] = (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_BEGIN_ELSE] = (1U << PM_TOKEN_KEYWORD_ENSURE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_BEGIN_RESCUE] = (1U << PM_TOKEN_KEYWORD_ENSURE) | (1U << PM_TOKEN_KEYWORD_RESCUE) | (1U << PM_TOKEN_KEYWORD_ELSE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_BLOCK_BRACES] = (1U << PM_TOKEN_BRACE_RIGHT), + [PM_CONTEXT_BLOCK_KEYWORDS] = (1U << PM_TOKEN_KEYWORD_END) | (1U << PM_TOKEN_KEYWORD_RESCUE) | (1U << PM_TOKEN_KEYWORD_ENSURE), + [PM_CONTEXT_BLOCK_ENSURE] = (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_BLOCK_ELSE] = (1U << PM_TOKEN_KEYWORD_ENSURE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_BLOCK_RESCUE] = (1U << PM_TOKEN_KEYWORD_ENSURE) | (1U << PM_TOKEN_KEYWORD_RESCUE) | (1U << PM_TOKEN_KEYWORD_ELSE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_CASE_WHEN] = (1U << PM_TOKEN_KEYWORD_WHEN) | (1U << PM_TOKEN_KEYWORD_END) | (1U << PM_TOKEN_KEYWORD_ELSE), + [PM_CONTEXT_CASE_IN] = (1U << PM_TOKEN_KEYWORD_IN) | (1U << PM_TOKEN_KEYWORD_END) | (1U << PM_TOKEN_KEYWORD_ELSE), + [PM_CONTEXT_CLASS] = (1U << PM_TOKEN_KEYWORD_END) | (1U << PM_TOKEN_KEYWORD_RESCUE) | (1U << PM_TOKEN_KEYWORD_ENSURE), + [PM_CONTEXT_CLASS_ENSURE] = (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_CLASS_ELSE] = (1U << PM_TOKEN_KEYWORD_ENSURE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_CLASS_RESCUE] = (1U << PM_TOKEN_KEYWORD_ENSURE) | (1U << PM_TOKEN_KEYWORD_RESCUE) | (1U << PM_TOKEN_KEYWORD_ELSE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_DEF] = (1U << PM_TOKEN_KEYWORD_END) | (1U << PM_TOKEN_KEYWORD_RESCUE) | (1U << PM_TOKEN_KEYWORD_ENSURE), + [PM_CONTEXT_DEF_ENSURE] = (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_DEF_ELSE] = (1U << PM_TOKEN_KEYWORD_ENSURE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_DEF_RESCUE] = (1U << PM_TOKEN_KEYWORD_ENSURE) | (1U << PM_TOKEN_KEYWORD_RESCUE) | (1U << PM_TOKEN_KEYWORD_ELSE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_DEF_PARAMS] = (1U << PM_TOKEN_EOF), + [PM_CONTEXT_DEFINED] = (1U << PM_TOKEN_EOF), + [PM_CONTEXT_DEFAULT_PARAMS] = (1U << PM_TOKEN_COMMA) | (1U << PM_TOKEN_PARENTHESIS_RIGHT), + [PM_CONTEXT_ELSE] = (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_ELSIF] = (1U << PM_TOKEN_KEYWORD_ELSE) | (1U << PM_TOKEN_KEYWORD_ELSIF) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_EMBEXPR] = (1U << PM_TOKEN_EMBEXPR_END), + [PM_CONTEXT_FOR] = (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_FOR_INDEX] = (1U << PM_TOKEN_KEYWORD_IN), + [PM_CONTEXT_IF] = (1U << PM_TOKEN_KEYWORD_ELSE) | (1U << PM_TOKEN_KEYWORD_ELSIF) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_LAMBDA_BRACES] = (1U << PM_TOKEN_BRACE_RIGHT), + [PM_CONTEXT_LAMBDA_DO_END] = (1U << PM_TOKEN_KEYWORD_END) | (1U << PM_TOKEN_KEYWORD_RESCUE) | (1U << PM_TOKEN_KEYWORD_ENSURE), + [PM_CONTEXT_LAMBDA_ENSURE] = (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_LAMBDA_ELSE] = (1U << PM_TOKEN_KEYWORD_ENSURE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_LAMBDA_RESCUE] = (1U << PM_TOKEN_KEYWORD_ENSURE) | (1U << PM_TOKEN_KEYWORD_RESCUE) | (1U << PM_TOKEN_KEYWORD_ELSE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_LOOP_PREDICATE] = (1U << PM_TOKEN_KEYWORD_DO) | (1U << PM_TOKEN_KEYWORD_THEN), + [PM_CONTEXT_MAIN] = (1U << PM_TOKEN_EOF), + [PM_CONTEXT_MODULE] = (1U << PM_TOKEN_KEYWORD_END) | (1U << PM_TOKEN_KEYWORD_RESCUE) | (1U << PM_TOKEN_KEYWORD_ENSURE), + [PM_CONTEXT_MODULE_ENSURE] = (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_MODULE_ELSE] = (1U << PM_TOKEN_KEYWORD_ENSURE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_MODULE_RESCUE] = (1U << PM_TOKEN_KEYWORD_ENSURE) | (1U << PM_TOKEN_KEYWORD_RESCUE) | (1U << PM_TOKEN_KEYWORD_ELSE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_MULTI_TARGET] = (1U << PM_TOKEN_EOF), + [PM_CONTEXT_PARENS] = (1U << PM_TOKEN_PARENTHESIS_RIGHT), + [PM_CONTEXT_POSTEXE] = (1U << PM_TOKEN_BRACE_RIGHT), + [PM_CONTEXT_PREDICATE] = (1U << PM_TOKEN_KEYWORD_THEN) | (1U << PM_TOKEN_NEWLINE) | (1U << PM_TOKEN_SEMICOLON), + [PM_CONTEXT_PREEXE] = (1U << PM_TOKEN_BRACE_RIGHT), + [PM_CONTEXT_RESCUE_MODIFIER] = (1U << PM_TOKEN_EOF), + [PM_CONTEXT_SCLASS] = (1U << PM_TOKEN_KEYWORD_END) | (1U << PM_TOKEN_KEYWORD_RESCUE) | (1U << PM_TOKEN_KEYWORD_ENSURE), + [PM_CONTEXT_SCLASS_ENSURE] = (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_SCLASS_ELSE] = (1U << PM_TOKEN_KEYWORD_ENSURE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_SCLASS_RESCUE] = (1U << PM_TOKEN_KEYWORD_ENSURE) | (1U << PM_TOKEN_KEYWORD_RESCUE) | (1U << PM_TOKEN_KEYWORD_ELSE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_TERNARY] = (1U << PM_TOKEN_EOF), + [PM_CONTEXT_UNLESS] = (1U << PM_TOKEN_KEYWORD_ELSE) | (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_UNTIL] = (1U << PM_TOKEN_KEYWORD_END), + [PM_CONTEXT_WHILE] = (1U << PM_TOKEN_KEYWORD_END), }; static inline bool context_terminator(pm_context_t context, pm_token_t *token) { - return token->type < 32 && (context_terminators[context] & (1 << token->type)); + return token->type < 32 && (context_terminators[context] & (1U << token->type)); } /** diff --git a/vm.c b/vm.c index 042a1ac12c98ca..3281fad0181916 100644 --- a/vm.c +++ b/vm.c @@ -59,6 +59,8 @@ int ruby_assert_critical_section_entered = 0; static void *native_main_thread_stack_top; +bool ruby_vm_during_cleanup = false; + VALUE rb_str_concat_literals(size_t, const VALUE*); VALUE vm_exec(rb_execution_context_t *); @@ -3287,6 +3289,7 @@ int ruby_vm_destruct(rb_vm_t *vm) { RUBY_FREE_ENTER("vm"); + ruby_vm_during_cleanup = true; if (vm) { rb_thread_t *th = vm->ractor.main_thread; diff --git a/vm_core.h b/vm_core.h index 5068e200575c18..e8e6a6a3a6b3f9 100644 --- a/vm_core.h +++ b/vm_core.h @@ -823,6 +823,8 @@ typedef struct rb_vm_struct { } default_params; } rb_vm_t; +extern bool ruby_vm_during_cleanup; + /* default values */ #define RUBY_VM_SIZE_ALIGN 4096 diff --git a/zjit/bindgen/src/main.rs b/zjit/bindgen/src/main.rs index 975a2d91570613..77d482db4e9d97 100644 --- a/zjit/bindgen/src/main.rs +++ b/zjit/bindgen/src/main.rs @@ -130,6 +130,8 @@ fn main() { .allowlist_function("rb_singleton_class") .allowlist_function("rb_define_class") .allowlist_function("rb_class_get_superclass") + .allowlist_function("rb_gc_disable_no_rest") + .allowlist_function("rb_gc_enable") .allowlist_function("rb_gc_mark") .allowlist_function("rb_gc_mark_movable") .allowlist_function("rb_gc_location") diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index c5bdbcfe0a5a83..0bf0205b5351d2 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -431,6 +431,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::ObjToString { val, cd, state, .. } => gen_objtostring(jit, asm, opnd!(val), *cd, &function.frame_state(*state)), &Insn::CheckInterrupts { state } => no_output!(gen_check_interrupts(jit, asm, &function.frame_state(state))), &Insn::HashDup { val, state } => { gen_hash_dup(asm, opnd!(val), &function.frame_state(state)) }, + &Insn::HashAref { hash, key, state } => { gen_hash_aref(jit, asm, opnd!(hash), opnd!(key), &function.frame_state(state)) }, &Insn::ArrayPush { array, val, state } => { no_output!(gen_array_push(asm, opnd!(array), opnd!(val), &function.frame_state(state))) }, &Insn::ToNewArray { val, state } => { gen_to_new_array(jit, asm, opnd!(val), &function.frame_state(state)) }, &Insn::ToArray { val, state } => { gen_to_array(jit, asm, opnd!(val), &function.frame_state(state)) }, @@ -860,6 +861,11 @@ fn gen_hash_dup(asm: &mut Assembler, val: Opnd, state: &FrameState) -> lir::Opnd asm_ccall!(asm, rb_hash_resurrect, val) } +fn gen_hash_aref(jit: &mut JITState, asm: &mut Assembler, hash: Opnd, key: Opnd, state: &FrameState) -> lir::Opnd { + gen_prepare_non_leaf_call(jit, asm, state); + asm_ccall!(asm, rb_hash_aref, hash, key) +} + fn gen_array_push(asm: &mut Assembler, array: Opnd, val: Opnd, state: &FrameState) { gen_prepare_leaf_call_with_gc(asm, state); asm_ccall!(asm, rb_ary_push, array, val); diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index 1a2dce03eda316..4eb1d3a17c836e 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -890,6 +890,14 @@ where let mut recursive_lock_level: c_uint = 0; unsafe { rb_jit_vm_lock_then_barrier(&mut recursive_lock_level, file, line) }; + // Ensure GC is off while we have the VM lock because: + // 1. We create many transient Rust collections that hold VALUEs during compilation. + // It's extremely tricky to properly marked and reference update these, not to + // mention the overhead and ergonomics issues. + // 2. If we yield to the GC while compiling, it re-enters our mark and update functions. + // This breaks `&mut` exclusivity since mark functions derive fresh `&mut` from statics + // while there is a stack frame below it that has an overlapping `&mut`. That's UB. + let gc_disabled_pre_call = unsafe { rb_gc_disable_no_rest() }.test(); let ret = match catch_unwind(func) { Ok(result) => result, @@ -909,7 +917,12 @@ where } }; - unsafe { rb_jit_vm_unlock(&mut recursive_lock_level, file, line) }; + unsafe { + if !gc_disabled_pre_call { + rb_gc_enable(); + } + rb_jit_vm_unlock(&mut recursive_lock_level, file, line); + }; ret } diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index f18c0035ee0f67..6a6263ab15110a 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -745,6 +745,7 @@ unsafe extern "C" { pub fn rb_gc_mark(obj: VALUE); pub fn rb_gc_mark_movable(obj: VALUE); pub fn rb_gc_location(obj: VALUE) -> VALUE; + pub fn rb_gc_enable() -> VALUE; pub fn rb_gc_writebarrier(old: VALUE, young: VALUE); pub fn rb_class_get_superclass(klass: VALUE) -> VALUE; pub static mut rb_cObject: VALUE; @@ -876,6 +877,7 @@ unsafe extern "C" { buff_size: usize, obj: VALUE, ) -> *const ::std::os::raw::c_char; + pub fn rb_gc_disable_no_rest() -> VALUE; pub fn rb_ec_stack_check(ec: *mut rb_execution_context_struct) -> ::std::os::raw::c_int; pub fn rb_gc_writebarrier_remember(obj: VALUE); pub fn rb_shape_id_offset() -> i32; diff --git a/zjit/src/cruby_methods.rs b/zjit/src/cruby_methods.rs index bc8f1d3b847501..27e6b151bf5020 100644 --- a/zjit/src/cruby_methods.rs +++ b/zjit/src/cruby_methods.rs @@ -198,6 +198,7 @@ pub fn init() -> Annotations { annotate!(rb_cArray, "reverse", types::ArrayExact, leaf, elidable); annotate!(rb_cArray, "join", types::StringExact); annotate!(rb_cArray, "[]", inline_array_aref); + annotate!(rb_cHash, "[]", inline_hash_aref); annotate!(rb_cHash, "empty?", types::BoolExact, no_gc, leaf, elidable); annotate!(rb_cNilClass, "nil?", types::TrueClass, no_gc, leaf, elidable); annotate!(rb_mKernel, "nil?", types::FalseClass, no_gc, leaf, elidable); @@ -248,3 +249,11 @@ fn inline_array_aref(fun: &mut hir::Function, block: hir::BlockId, recv: hir::In } None } + +fn inline_hash_aref(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option { + if let &[key] = args { + let result = fun.push_insn(block, hir::Insn::HashAref { hash: recv, key, state }); + return Some(result); + } + None +} diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 33c034b819a7ec..4ad20b9c1d8295 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -582,6 +582,7 @@ pub enum Insn { ArrayPush { array: InsnId, val: InsnId, state: InsnId }, ArrayArefFixnum { array: InsnId, index: InsnId }, + HashAref { hash: InsnId, key: InsnId, state: InsnId }, HashDup { val: InsnId, state: InsnId }, /// Allocate an instance of the `val` object without calling `#initialize` on it. @@ -923,6 +924,7 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { } Insn::ArrayDup { val, .. } => { write!(f, "ArrayDup {val}") } Insn::HashDup { val, .. } => { write!(f, "HashDup {val}") } + Insn::HashAref { hash, key, .. } => { write!(f, "HashAref {hash}, {key}")} Insn::ObjectAlloc { val, .. } => { write!(f, "ObjectAlloc {val}") } &Insn::ObjectAllocClass { class, .. } => { let class_name = get_class_name(class); @@ -1580,6 +1582,7 @@ impl Function { &InvokeBuiltin { bf, ref args, state, return_type } => InvokeBuiltin { bf, args: find_vec!(args), state, return_type }, &ArrayDup { val, state } => ArrayDup { val: find!(val), state }, &HashDup { val, state } => HashDup { val: find!(val), state }, + &HashAref { hash, key, state } => HashAref { hash: find!(hash), key: find!(key), state }, &ObjectAlloc { val, state } => ObjectAlloc { val: find!(val), state }, &ObjectAllocClass { class, state } => ObjectAllocClass { class, state: find!(state) }, &CCall { cfunc, ref args, name, return_type, elidable } => CCall { cfunc, args: find_vec!(args), name, return_type, elidable }, @@ -1680,6 +1683,7 @@ impl Function { Insn::NewArray { .. } => types::ArrayExact, Insn::ArrayDup { .. } => types::ArrayExact, Insn::ArrayArefFixnum { .. } => types::BasicObject, + Insn::HashAref { .. } => types::BasicObject, Insn::NewHash { .. } => types::HashExact, Insn::HashDup { .. } => types::HashExact, Insn::NewRange { .. } => types::RangeExact, @@ -2630,9 +2634,13 @@ impl Function { Insn::ArrayArefFixnum { array, index } if self.type_of(array).ruby_object_known() && self.type_of(index).ruby_object_known() => { let array_obj = self.type_of(array).ruby_object().unwrap(); - let index = self.type_of(index).fixnum_value().unwrap(); - let val = unsafe { rb_yarv_ary_entry_internal(array_obj, index) }; - self.new_insn(Insn::Const { val: Const::Value(val) }) + if array_obj.is_frozen() { + let index = self.type_of(index).fixnum_value().unwrap(); + let val = unsafe { rb_yarv_ary_entry_internal(array_obj, index) }; + self.new_insn(Insn::Const { val: Const::Value(val) }) + } else { + insn_id + } } Insn::Test { val } if self.type_of(val).is_known_falsy() => { self.new_insn(Insn::Const { val: Const::CBool(false) }) @@ -2771,6 +2779,11 @@ impl Function { worklist.push_back(array); worklist.push_back(index); } + &Insn::HashAref { hash, key, state } => { + worklist.push_back(hash); + worklist.push_back(key); + worklist.push_back(state); + } &Insn::Send { recv, ref args, state, .. } | &Insn::SendForward { recv, ref args, state, .. } | &Insn::SendWithoutBlock { recv, ref args, state, .. } @@ -9238,7 +9251,7 @@ mod opt_tests { PatchPoint MethodRedefined(Hash@0x1000, []@0x1008, cme:0x1010) PatchPoint NoSingletonClass(Hash@0x1000) v26:HashExact = GuardType v9, HashExact - v27:BasicObject = CCallWithFrame []@0x1038, v26, v13 + v27:BasicObject = HashAref v26, v13 CheckInterrupts Return v27 "); @@ -11556,6 +11569,37 @@ mod opt_tests { "); } + #[test] + fn test_dont_eliminate_load_from_non_frozen_array() { + eval(r##" + S = [4,5,6] + def test = S[0] + test + "##); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, S) + v24:ArrayExact[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + v12:Fixnum[0] = Const Value(0) + PatchPoint MethodRedefined(Array@0x1010, []@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(Array@0x1010) + v28:BasicObject = ArrayArefFixnum v24, v12 + CheckInterrupts + Return v28 + "); + // TODO(max): Check the result of `S[0] = 5; test` using `inspect` to make sure that we + // actually do the load at run-time. + } + #[test] fn test_eliminate_load_from_frozen_array_in_bounds() { eval(r##" @@ -12792,6 +12836,125 @@ mod opt_tests { "); } + #[test] + fn test_hash_aref_literal() { + eval(" + def test + arr = {1 => 3} + arr[1] + end + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:NilClass = Const Value(nil) + Jump bb2(v1, v2) + bb1(v5:BasicObject): + EntryPoint JIT(0) + v6:NilClass = Const Value(nil) + Jump bb2(v5, v6) + bb2(v8:BasicObject, v9:NilClass): + v13:HashExact[VALUE(0x1000)] = Const Value(VALUE(0x1000)) + v15:HashExact = HashDup v13 + v18:Fixnum[1] = Const Value(1) + PatchPoint MethodRedefined(Hash@0x1008, []@0x1010, cme:0x1018) + PatchPoint NoSingletonClass(Hash@0x1008) + v31:BasicObject = HashAref v15, v18 + CheckInterrupts + Return v31 + "); + } + + #[test] + fn test_hash_aref_profiled() { + eval(" + def test(hash, key) + hash[key] + end + test({1 => 3}, 1) + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal l0, SP@5 + v3:BasicObject = GetLocal l0, SP@4 + Jump bb2(v1, v2, v3) + bb1(v6:BasicObject, v7:BasicObject, v8:BasicObject): + EntryPoint JIT(0) + Jump bb2(v6, v7, v8) + bb2(v10:BasicObject, v11:BasicObject, v12:BasicObject): + PatchPoint MethodRedefined(Hash@0x1000, []@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(Hash@0x1000) + v28:HashExact = GuardType v11, HashExact + v29:BasicObject = HashAref v28, v12 + CheckInterrupts + Return v29 + "); + } + + #[test] + fn test_hash_aref_subclass() { + eval(" + class C < Hash; end + def test(hash, key) + hash[key] + end + test(C.new({0 => 3}), 0) + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:4: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal l0, SP@5 + v3:BasicObject = GetLocal l0, SP@4 + Jump bb2(v1, v2, v3) + bb1(v6:BasicObject, v7:BasicObject, v8:BasicObject): + EntryPoint JIT(0) + Jump bb2(v6, v7, v8) + bb2(v10:BasicObject, v11:BasicObject, v12:BasicObject): + PatchPoint MethodRedefined(C@0x1000, []@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(C@0x1000) + v28:HeapObject[class_exact:C] = GuardType v11, HeapObject[class_exact:C] + v29:BasicObject = HashAref v28, v12 + CheckInterrupts + Return v29 + "); + } + + #[test] + fn test_does_not_fold_hash_aref_with_frozen_hash() { + eval(" + H = {a: 0}.freeze + def test = H[:a] + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, H) + v24:HashExact[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + v12:StaticSymbol[:a] = Const Value(VALUE(0x1010)) + PatchPoint MethodRedefined(Hash@0x1018, []@0x1020, cme:0x1028) + PatchPoint NoSingletonClass(Hash@0x1018) + v28:BasicObject = HashAref v24, v12 + CheckInterrupts + Return v28 + "); + } + #[test] fn test_optimize_thread_current() { eval("