[PATCH] ZJIT: Drop has_blockiseq guard on local variable access (#16113)
So `has_blockiseq` was masking two other bugs.
First, that we had only a static EP escape check for an iseq. We need to check
both that an iseq is not known to statically escape them *and that it has not
escaped them before*. Only then can we use SSA locals. This popped up in a
weird PP unit test that was reducible to the test cases added in the PR. So I
fixed that.
Second, that we didn't VM lock when code patching for EP escape. That showed up
in the Ractor btests in my previous PRs #14600 and #15254#15316, and I had no
idea what was going on at the time, so I dropped it. But if we add this lock,
like all the other invariant patching cases have, the problem goes away. Seems
reasonable.
Finally, we can use normal full SSA locals even in the presence of a blockiseq.
We still reload all of them after each send-with-block, but we can in a later
PR reduce that to just the locals that are (recursively) visibly written to.
[PATCH] [Bug #22224] Ractors, YJIT: Skip re-reporting recorded EP escapes to the JITs (#18176)
* [22224] Ractors, YJIT: Skip re-reporting recorded EP escapes to the JITs
vm_make_env_each reports every EP escape, and both
rb_yjit_invalidate_ep_is_bp and rb_zjit_invalidate_no_ep_escape take
the VM lock with a stop-all-Ractors barrier on every call. For a given
iseq only the first report does anything: afterwards YJIT's
no_ep_escape_iseqs entry is permanently an empty set and ZJIT's
ep_escape_iseqs insert is a no-op, so the barrier is paid for nothing.
Under multiple Ractors this serializes the whole process on every Proc
created from an already-escaped iseq: a workload creating one lambda
per iteration runs 150x slower on 8 Ractors than on 1 with YJIT
enabled.
Record the report in a jit_ep_escape_recorded flag on the iseq body,
set by the active JIT while it holds the VM lock, and skip the calls
in vm_make_env_each once it is set. The flag sits in existing padding
and the body is ZALLOC'd, so it starts false and dies with the iseq.
Setting it in the JIT rather than in the caller keeps mid-process
RubyVM::YJIT.enable correct: escapes reported while no JIT is
initialized are recorded by nobody and must not set the flag. The
unlocked read is benign: a stale false causes one redundant,
idempotent report.
On the micro-benchmark from the bug, --yjit at 8 Ractors goes from
0.04M to 11.07M iterations/s (277x).
* [22224] Deduplicate the EP escape record onto the iseq body
The jit_ep_escape_recorded flag introduced in the previous commit is
now the JITs' only record of an EP escape: YJIT's no_ep_escape_iseqs
keeps only iseqs with live assumptions (the empty-set escape markers
are gone), and ZJIT's ep_escape_iseqs set is removed together with its
update_ep_escape_iseqs compaction pass, since the flag lives in the
iseq body, which the GC does not move. Both JITs read the flag back at
compile time through a new rb_jit_iseq_ep_escape_recorded_p helper.
Co-authored-by: Alan Wu <XrXr@users.noreply.github.com>
Only the two correctness changes of 9efe557da5 are backported: the runtime
EP-escape check in iseq_to_hir() and the VM lock in
rb_zjit_invalidate_no_ep_escape(). Without the former, suppressing repeat
escape reports would leave NoEPEscape patch points registered after an
escape never invalidated, because ZJIT here registers them unconditionally.
Its has_blockiseq removal is an optimization and is left out; keeping
has_blockiseq is strictly more conservative.
ZJIT here keeps the name invariants::iseq_escapes_ep() rather than renaming
it to iseq_seen_ep_escape(), since e0184fff1a is not backported.
[PATCH] Fix the `intern` VM instruction to be `leaf=false` (also in YJIT, ZJIT)
rb_str_intern can raise EncodingError and call its initializer.
This call can push a Ruby frame.
Fix in the VM, YJIT, and ZJIT.
Multiple YJIT functions created overlapping `&'static mut IseqPayload`
references by calling `get_iseq_payload()` multiple times for the same
iseq. LLVM's noalias optimizations could then cache stale version_map
Vec headers, causing the version_map to point to freed backing storage.
This manifested as crashes in various YJIT operations (block lookup,
GC marking, block removal) that dereference the stale pointer.
Fix by moving `delayed_deallocation` and `get_or_create_version_list`
from free functions (which each call `get_iseq_payload()` internally)
to methods on `IseqPayload` that operate through `&mut self`. This
lets callers obtain a single payload reference and use it for all
operations without creating overlapping mutable borrows.
The three fixed call sites:
1. `rb_yjit_tracing_invalidate_all` (invariants.rs): The loop called
`delayed_deallocation()` which internally called `get_iseq_payload()`,
creating a second `&mut` overlapping with the outer `payload` reference.
Fix: call `payload.delayed_deallocation()` method instead.
2. `add_block_version` (core.rs): Called `get_or_create_version_list()`
then later `get_iseq_payload()` for pages, creating two references.
Fix: use a single `get_or_create_iseq_payload()` call then call the
`get_or_create_version_list()` method on it for both version_map and
pages access.
3. `invalidate_block_version` (core.rs): Called `remove_block_version()`
(which calls `get_iseq_payload()`) then `delayed_deallocation()`
(which also calls `get_iseq_payload()`), creating two references.
Fix: get the payload once, inline the version_map removal, and call
`payload.delayed_deallocation()` method for the rest.
Also adds regression tests exercising tracing invalidation with
on-stack methods and suspended fibers.
(cherry picked from commit a6fcad276a)
Split rb_yjit_init into rb_yjit_init_builtin_cmes (called before
ruby_init_prelude) and rb_yjit_init (called after). The prelude may
load bundler via BUNDLER_SETUP which can call Kernel.prepend, moving
core methods to an origin iclass. Registering method codegen before
the prelude ensures we capture CMEs while core classes are pristine.
Previously, local indices greater than 255 were truncated when
converted to RegOpnd::Local. This could make a high-index local alias
a tracked low-index local in the register mapping and produce incorrect
values after compilation.
Cap overflowing indices to MAX_CTX_LOCALS. RegMapping treats that index
as untrackable because it is just outside the tracked local range.
Fixes [Backport #22074].
class.c: rb_class_duplicate_classext also dup content of cvc_tbl
[Bug #21952]
Shallow copying the table result in the same memory being shared
between multiple box, causing double free when one of the box
is garbage collected.
---
class.c: Make cvc_tbl a managed object
[Bug #21952]
Solves the double-free or use after-free concern with boxes.
Now entries can safely be used for copy-on-write.
Also is likely necessary to make it save to read cvar from
secondary ractors, as allowed since: ab32c0e690
[Backport #21941]
In case of `--yjit-disable`, YJIT only starts to record environment
escapes after `RubyVM::YJIT.enable`. Previously we falsely assumed that
we always have a full history all the way back to VM boot. This had YJIT
install and run code that assume EP=BP when EP≠BP for some exceptional
entry into the middle of a running frame, if the environment escaped
before `YJIT.enable`.
The fix is to reject exceptional entry with an escaped environment.
Rename things and explain in more detail how the predicate for deciding
to assume EP=BP works. It's quite subtle since it reasons about all
parties in the system that push a control frame and then run JIT code.
Note that while can_assume_on_stack_env() checks the currently running
environment if it so happens to be the one YJIT is compiling against, it
can return true for any ISEQ. The check isn't necessary for fixing the
bug, and the load bearing part of this patch is the change to
exceptional entries.
This fix is flat on speed and space on ruby-bench headline benchmarks.
Many thanks for the community effort to create a small test case for
this bug.
Co-authored-by: Alan Wu <alanwu@ruby-lang.org>
YJIT: Support calling bmethods in Ractors
Co-authored-by: Luke Gruber <luke.gru@gmail.com>
Suggestion from Alan
For <https://bugs.ruby-lang.org/issues/21716>, the panic is looking like
some sort of third party memory corruption, with YJIT taking the fall.
At the point of this assert, the assembler has dropped, so there's
nothing in YJIT's code other than JITState that could be holding on to
these transient `PendingBranchRef`.
The strong count being more than a handful or the weak count is non-zero
shows that someone in the process (likely some native extension)
corrupted the Rc's counts.
Previously, the chain_depth>0 version of setlocal blocks did not
update the type of the local variable in the context. This can leave
the context with stale type information and trigger panics like in
[Bug #21772] or lead to miscompilation.
To trigger the issue, YJIT needs to see the same ISEQ before and after
environment escape and have tracked type info before the escape. To
trigger in ISEQs that do not send with a block, it probably requires
Kernel#binding or the use of include/ruby/debug.h APIs.
Fixes https://github.com/Shopify/ruby/issues/902
This pull request adds code generation for dividing fixnums.
Testing confirms the normal case, flooring, and side-exiting on division by zero.
The comptime receiver, which is a proc, is either shareable or from this
ractor so we don't need to assume single-ractor mode. We should never get
the "defined with an un-shareable Proc in a different ractor" error.
Previously because we did a stack_push before ccall, in some cases we
could end up pushing an uninitialized value to the VM stack when
spilling regs as part of the ccall.
Co-authored-by: Luke Gruber <luke.gru@gmail.com>
I'm seeing some memory corruption in the wild on blocks in
`IseqPayload::dead_blocks`. While I unfortunately can't recreate the
issue, (For all I know, it could be some external code corrupting YJIT's
memory.) establishing a link between dead blocks and live blocks seems
fishy enough that we ought to prevent it. When it did happen, it might've
had bad interacts with Code GC and the optimization to immediately
free empty blocks.
* YJIT: Pass iseq pointer to get/set classvariable functions
Since we already have the iseq pointer, we can actually save one
memory read by passing it directly.
We need to wrap the iseq in a VALUE so it can be marked correctly
by GC.
* YJIT: Fix missing GC marking when passing iseq to rb_vm_setinstancevariable
Without wrapping the iseq in a `Operand::Value`, the iseq would not be
marked correctly by GC and when compacting the heap, the iseq would be
lost and cause a crash.
Takashi Kokubun