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.
```
.../tool/lib/colorize.rb:56:in `resolve_color': undefined method `gsub' for an instance of Symbol (NoMethodError)
color.gsub(/\b[a-z][\w ]+/) do |n|
^^^^^
from .../tool/lib/colorize.rb:47:in `decorate'
from .../tool/lib/test/unit.rb:1012:in `block in failed'
```
For now, on a small rails app I have hanging around:
```
==> memprof.after.txt <==
Total allocated: 872.51 MB (465330 objects)
Total retained: 40.48 kB (326 objects)
==> memprof.before.txt <==
Total allocated: 890.79 MB (1494026 objects)
Total retained: 40.40 kB (328 objects)
```
Not a huge difference in memory usage, but it's a drastic improvement
in total number of allocations.
Additionally, this will pay huge dividends once
https://github.com/ruby/zlib/pull/61 is merged, as it will allow us to
completely avoid allocations in the repeated calls to readpartial,
which currently accounts for most of the memory usage shown above.
https://github.com/rubygems/rubygems/commit/f78d45d927
If there are MultiTargetNodes within parameters, we need to
iterate over them and compile them individually correctly, once
the locals are all in the correct spaces. We need to add one
getlocal for the hidden variable, and then can recurse into the
MultiTargetNodes themselves
This commit completely restructures how we handle parameters. The
motivation for this commit was the fix compilation of MultiTargetNodes
within parameters, including nested MultiTargetNodes. A subsequent
commit will actually do the compilation for the MultiTargetNodes.
This commit's main accomplishment is restructuring the locals table
and how we account for it on the ScopeNode, specifically with regards
to hidden variables.
It has multiple steps, all commented within the code, to calculate
the locals table correctly and compile the parameters:
- Step 1: Caculate the table size for the locals
- Step 2: Populate iv index table and local table
- Step 3: Fill in parameter names of MultiTargetNodes on local table
- Step 4: Fill in method body locals on local table
- Step 5: Compile any locals
Prior to this commit, we were using `add_ensure_iseq` which compiled
a node as if it was a CRuby node. This commit defines
`pm_add_ensure_iseq` which compiles the Prism node appropriately.
* YJIT: implement call fuzzer script
Attempt to detect bugs in YJIT call implementation.
* Add basic checks for rest, kwrest. Impprove formatting.
* Refactor call fuzzer to make it more powerful and maintainable
Compute checksum of arguments
* Fix checksum computation. Add useless locals as sussged by Alan.
* Add some useless if statements
* Add arguments of different types
* Pass object arguments as well. Force different shapes.
* Compute fuzzing time/speed
* Make use of block param
In this commit we're splitting up the call nodes that were in target
positions (that is, for loop indices, rescue error captures, and
multi assign targets).
Previously, we would simply leave the call nodes in place. This had
the benefit of keeping the AST relatively simple, but had the
downside of not being very explicit. If a static analysis tool wanted
to only look at call nodes, it could easily be confused because the
method would have 1 fewer argument than it would actually be called
with.
This also brings some consistency to the AST. All of the nodes in
a target position are now *TargetNode nodes. These should all be
treated the same, and the call nodes can now be treated the same.
Finally, there is benefit to memory. Because being in a target
position ensures we don't have some fields, we can strip down the
number of fields on these nodes.
So this commit introduces two new nodes: CallTargetNode and
IndexTargetNode. For CallTargetNode we get to drop the opening_loc,
closing_loc, arguments, and block. Those can never be present. We
also get to mark their fields as non-null, so they will always be
seen as present.
The IndexTargetNode keeps around most of its fields but gets to
drop both the name (because it will always be []=) and the
message_loc (which was always super confusing because it included
the arguments by virtue of being inside the []).
Overall, this adds complexity to the AST at the expense of memory
savings and explicitness. I believe this tradeoff is worth it in
this case, especially because these are very much not common nodes
in the first place.
https://github.com/ruby/prism/commit/3ef71cdb45
The logic within the consequent for the CaseNodes in popped cases
was incorrect as it wouldn't emit consequent instructions for a
popped CaseNode. This commit fixes that.
Arguments that are passed as a hash need special consideration since in certain case they are not treated as keyword arguments. For example, if a call is passing `"a" => 1` as an argument, this will be turned into an implicit hash argument and not a keyword argument.
The existing compiler checks to see if all hash nodes can be treated as keyword arguments. If they can, then it will treat them as keyword arguments. If not, then it will treat them as implicit hash arguments.
This commit implements the same logic inside the Prism compiler.
Uses Open3.popen2e in place of Open3.capture2e in Gem::Ext::Builder.
This change aims to stream stdout/stderr of ext builds when in verbose
mode, instead of printing everything at once when the build completes.
Nice for debugging gem builds that consumes longer times.
https://github.com/rubygems/rubygems/commit/dcdcb5adda
Our current implementation of rb_postponed_job_register suffers from
some safety issues that can lead to interpreter crashes (see bug #1991).
Essentially, the issue is that jobs can be called with the wrong
arguments.
We made two attempts to fix this whilst keeping the promised semantics,
but:
* The first one involved masking/unmasking when flushing jobs, which
was believed to be too expensive
* The second one involved a lock-free, multi-producer, single-consumer
ringbuffer, which was too complex
The critical insight behind this third solution is that essentially the
only user of these APIs are a) internal, or b) profiling gems.
For a), none of the usages actually require variable data; they will
work just fine with the preregistration interface.
For b), generally profiling gems only call a single callback with a
single piece of data (which is actually usually just zero) for the life
of the program. The ringbuffer is complex because it needs to support
multi-word inserts of job & data (which can't be atomic); but nobody
actually even needs that functionality, really.
So, this comit:
* Introduces a pre-registration API for jobs, with a GVL-requiring
rb_postponed_job_prereigster, which returns a handle which can be
used with an async-signal-safe rb_postponed_job_trigger.
* Deprecates rb_postponed_job_register (and re-implements it on top of
the preregister function for compatability)
* Moves all the internal usages of postponed job register
pre-registration
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.
There seems to be another manifestation of bug #20021, where some of the
compaction tests are failing on i686 for unrelated PR's because of fake
"live" references to moved objects on the machine stack.
We _could_ solve this by counting how many objects are pinned during
compaction, but doing that involves pushing down the mark & pin bitset
merge into gc_compact_plane and out of gc_compact_page, which I thought
was pretty ugly.
Now that we've solved bug #20022 though, we're able to compact
arbitrarily many objects with GC.verify_compaction_references, so the
number of objects we're moving is now 50,000 instead of 500. Since
that's now much larger than the number of objects likely to be pinned, I
think it's safe enough to just add a fudge-factor to the tests.
Any _other_ change in GC.verify_compaction_references that breaks
compaction is now highly likely to break the assertion by more than 10
objects, since it's operating on so many more in the first place.
[Bug #20021]
It seems that the Ractor sleep GVL event arrives very slightly after the
value becomes available and other threads wake (which makes sense) so we
need a little additional time to ensure we end up in a consisteny state.
This doesn't actually fix the encodings for symbols and regex,
unfortunately. But I wanted to get this change in because it is
the last AST change we're going to make before 3.3 is released.
So, if consumers want, they can start to check these flags to
determine the encoding, even though it will be wrong. Then once we
actually set them correctly, everything should work.
https://github.com/ruby/prism/commit/9b35f7e891
Alan Wu
Kazuhiro NISHIYAMA
Samuel Giddins
hogelog
Jemma Issroff
Burdette Lamar
Koichi Sasada
Maxime Chevalier-Boisvert
Kevin Newton
Martin Emde
Ufuk Kayserilioglu
TSUYUSATO Kitsune
Daisuke Aritomo
Nobuyoshi Nakada
git
Hsing-Hui Hsu
KJ Tsanaktsidis
Stan Lo
Alex Koval
Jeremy Evans
John Hawthorn
Yusuke Endoh
Victor Shepelev
eileencodes
Benoit Daloze
Schneems