YJIT: Fix potential infinite loop when OOM (GH-13186)
Avoid generating an infinite loop in the case where:
1. Block `first` is adjacent to block `second`, and the branch from `first` to
`second` is a fallthrough, and
2. Block `second` immediately exits to the interpreter, and
3. Block `second` is invalidated and YJIT is OOM
While pondering how to fix this, I think I've stumbled on another related edge case:
1. Block `incoming_one` and `incoming_two` both branch to block `second`. Block
`incoming_one` has a fallthrough
2. Block `second` immediately exits to the interpreter (so it starts with its exit)
3. When Block `second` is invalidated, the incoming fallthrough branch from
`incoming_one` might be rewritten first, which overwrites the start of block
`second` with a jump to a new branch stub.
4. YJIT runs of out memory
5. The incoming branch from `incoming_two` is then rewritten, but because we're
OOM we can't generate a new stub, so we use `second`'s exit as the branch
target. However `second`'s exit was already overwritten with a jump to the
branch stub for `incoming_one`, so `incoming_two` will end up jumping to
`incoming_one`'s branch stub.
Fixes [Bug #21257]
Evident with the crash reported in [Bug #20997], the C replacement
codegen functions aren't authored to handle block arguments (nor
should they because the extra code from the complexity defeats
optimization). Filter sites with VM_CALL_ARGS_BLOCKARG.
Co-Authored-By: Alan Wu <alanwu@ruby-lang.org>
If you start Ruby with `--yjit-disable`, the `+YJIT` shouldn't be
added until `RubyVM::YJIT.enable` is actually called. Otherwise
it's confusing in crash reports etc.
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
Do not emit shape transition warnings when YJIT is compiling
[Bug #20522]
If `Warning.warn` is redefined in Ruby, emitting a warning would invoke
Ruby code, which can't safely be done when YJIT is compiling.
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
This is a backport of 6c8ae44a38 with a
test tailored to crash the 3.3.x branch (from GH-10904).
Previously, we read the last element array even when the array was
empty, doing an out-of-bounds access. This sometimes caused a SEGV.
[Bug #20496]
YJIT: Fix unused warnings
```
warning: unused import: `condition::Condition`
--> src/asm/arm64/arg/mod.rs:13:9
|
13 | pub use condition::Condition;
| ^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unused_imports)]` on by default
warning: unused import: `rb_yjit_fix_mul_fix as rb_fix_mul_fix`
--> src/cruby.rs:188:9
|
188 | pub use rb_yjit_fix_mul_fix as rb_fix_mul_fix;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: unused import: `rb_insn_len as raw_insn_len`
--> src/cruby.rs:142:9
|
142 | pub use rb_insn_len as raw_insn_len;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unused_imports)]` on by default
```
Make asm public so it stops warning about unused public stuff in there.
YJIT: Fix ruby2_keywords splat+rest and drop bogus checks
YJIT didn't guard for ruby2_keywords hash in case of splat calls that
land in methods with a rest parameter, creating incorrect results.
The compile-time checks didn't correspond to any actual effects of
ruby2_keywords, so it was masking this bug and YJIT was needlessly
refusing to compile some code. About 16% of fallback reasons in
`lobsters` was due to the ISeq check.
We already handle the tagging part with
exit_if_supplying_kw_and_has_no_kw() and should now have a dynamic guard
for all splat cases.
Note for backporting: You also need 7f51959ff1.
[Bug #20195]
YJIT: Move guard up for a case of splat+rest
Previously, YJIT put the guard for having enough items to extract from
splat array at a place where the side exit is invalid, so if the guard
fails, YJIT could raise something other than ArgumentError. Move the
guard up to a place before any stack manipulation.
[Bug #20204]
* merge revision(s) bbd249e351: [Backport #20192]
YJIT: Properly reject keyword splat with `yield`
We don't have support for keyword splat anywhere, but we tried to
compile these anyways in case of `invokeblock`. This led to bad things
happening such as passing the wrong value and passing a hash into
rb_yjit_array_len(), which raised in the middle of compilation.
[Bug #20192]
* Skip a new test for RJIT
merge revision(s) 2cc7a56e,b0711b1,db5d9429: [Backport #20209]
YJIT: Avoid leaks by skipping objects with a singleton class
For receiver with a singleton class, there are multiple vectors YJIT can
end up retaining the object. There is a path in jit_guard_known_klass()
that bakes the receiver into the code, and the object could also be kept
alive indirectly through a path starting at the CME object baked into
the code.
To avoid these leaks, avoid compiling calls on objects with a singleton
class.
See: https://github.com/Shopify/ruby/issues/552
[Bug #20209]
---
yjit/bindgen/src/main.rs | 1 +
yjit/src/codegen.rs | 17 +++++++++++++++++
yjit/src/cruby_bindings.inc.rs | 1 +
yjit/src/stats.rs | 2 ++
4 files changed, 21 insertions(+)
YJIT: Fix tailcall and JIT entry eating up FINISH frames (#9729)
Suppose YJIT runs a rb_vm_opt_send_without_block()
fallback and the control frame stack looks like:
```
will_tailcall_bar [FINISH]
caller_that_used_fallback
```
will_tailcall_bar() runs in the interpreter and sets up a tailcall.
Right before JIT_EXEC() in the `send` instruction, the stack will look like:
```
bar [FINISH]
caller_that_used_fallback
```
Previously, JIT_EXEC() ran bar() in JIT code, which caused the `FINISH`
flag to return to the interpreter instead of to the JIT code running
caller_that_used_fallback(), causing code to run twice and probably
crash. Recent flaky failures on CI about "each stub expects a particular
iseq" are probably due to leaving methods twice in
`test_optimizations.rb`.
Only run JIT code from the interpreter if a new frame is pushed.
---
test/ruby/test_optimization.rb | 11 +++++++++++
vm_exec.h | 3 ++-
2 files changed, 13 insertions(+), 1 deletion(-)
YJIT: No need to RESTORE_REG now that we reject tailcalls
Thanks to Kokubun for noticing.
Follow-up: b0711b1cf1
---
vm_exec.h | 1 -
1 file changed, 1 deletion(-)
* YJIT: reduce default exec mem size to 48MiB based
Based on user feedback from @jhawthorn and others.
Better for small and memory-constrained deployments.
NOTE: This commit should be included in the next Ruby 3.3.x point
release. @xrxr should we tag someone specific?
* YJIT: Update yjit.md about mem size (#9687)
---------
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
Previously, if the method ID argument happens to be on one below the top
of the stack, we didn't overwrite the type of the stack slot, which
leaves an incorrect type for the stack slot. The included script tripped
asserts both with and without --yjit-verify-ctx.
Previously, for splat callsites that land in methods with optional
parameters, we didn't reject the case where the caller supplies too many
arguments. Accepting those calls previously caused YJIT to construct
corrupted control frames, which leads to crashes if the callee uses
certain stack walking methods such as Kernel#raise and String#gsub (for
setting up the frame-local `$~`).
Example crash in a debug build:
Assertion Failed: ../vm_core.h:1375:VM_ENV_FLAGS:FIXNUM_P(flags)
Counter::guard_send_iseq_has_rest_and_splat_not_equal was using
jump-if-lesser-than, so wasn't checking for equality. Rename function
because moving is destructive in Rust, which is confusing for this function
which doesn't modify the array.
Previously, block.to_proc was called first, by vm_caller_setup_arg_block.
kw.to_hash was called later inside CALLER_SETUP_ARG or setup_parameters_complex.
This adds a splatkw instruction that is inserted before sends with
ARGS_BLOCKARG and KW_SPLAT and without KW_SPLAT_MUT. This is not needed in the
KW_SPLAT_MUT case, because then you know the value is a hash, and you don't
need to call to_hash on it.
The splatkw instruction checks whether the second to top block is a hash,
and if not, replaces it with the value of calling to_hash on it (using
rb_to_hash_type). As it is always before a send with ARGS_BLOCKARG and
KW_SPLAT, second to top is the keyword splat, and top is the passed block.
We've seen quite a few compaction bugs lately, and these assertions
should give clearer symptoms. We only call class_of() on
objects that the Ruby code can see.
We have received a report of `assert!( !cb.has_dropped_bytes())` in
set_page() failing. The only explanation for this seems to be memory
allocation failing in write_byte(). The if condition implies that
`current_write_pos < dst_pos < mem_size`, which rules out failing to
encode the relative jump. The has_capacity() assert above not tripping
implies that we were in a place in the page where write_byte() did
attempt to write the byte and potentially made a syscall in the process.
Remove the assert, since memory allocation could fail. Also, return
failure if the destination is outside of the code region to detect that
out-of-memory situation quicker.
Like in the example given in delayed_deallocation(), stubs can be hit
even if the block housing it is invalidated. Mark them so we don't
work with invalidate ISeqs when hitting these stubs.
* YJIT: Cancel on-stack jit_return on invalidation
Co-authored-by: Alan Wu <alansi.xingwu@shopify.com>
* Use RUBY_VM_CONTROL_FRAME_STACK_OVERFLOW_P
---------
Co-authored-by: Alan Wu <alansi.xingwu@shopify.com>
* YJIT: record num_send_cfunc stat
Also report num_send_known_cfunc as percentage of num_send_cfunc
* Rename num_send_known_cfunc => num_send_cfunc_inline
Name seems more descriptive of what we do with out custom codegen
Randy Stauner