Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YJIT: Fix autosplat miscomp for blocks with optionals #8006

Merged
merged 2 commits into from Jul 4, 2023

Conversation

XrXr
Copy link
Member

@XrXr XrXr commented Jun 30, 2023

When passing an array as the sole argument to yield, and the yieldee
takes more than 1 optional parameter, the array is expanded similar
to *array splat calls. This is called "autosplat" in
setup_parameters_complex().

Previously, YJIT did not detect this autosplat condition. It passed the
array without expanding it, deviating from interpreter behavior.
Detect this conditon and refuse to compile it.

Fixes: Shopify/yjit#313


The setup_parameters_complex() logic, for reference:

ruby/vm_args.c

Lines 643 to 649 in d49a92d

if (given_argc == (NIL_P(keyword_hash) ? 1 : 2) &&
allow_autosplat &&
(min_argc > 0 || ISEQ_BODY(iseq)->param.opt_num > 1) &&
!ISEQ_BODY(iseq)->param.flags.ambiguous_param0 &&
!((ISEQ_BODY(iseq)->param.flags.has_kw ||
ISEQ_BODY(iseq)->param.flags.has_kwrest)
&& max_argc == 1) &&

XrXr added 2 commits June 30, 2023 13:13
When passing an array as the sole argument to `yield`, and the yieldee
takes more than 1 optional parameter, the array is expanded similar
to `*array` splat calls. This is called "autosplat" in
`setup_parameters_complex()`.

Previously, YJIT did not detect this autosplat condition. It passed the
array without expanding it, deviating from interpreter behavior.
Detect this conditon and refuse to compile it.

Fixes: Shopify/yjit#313
This is mirrors the same issue as YJIT. See previous commit.
@XrXr XrXr marked this pull request as ready for review June 30, 2023 22:07
@matzbot matzbot requested a review from a team June 30, 2023 22:07
@tomstuart
Copy link
Contributor

Thank you Alan! ❤️

@maximecb maximecb merged commit 296782a into ruby:master Jul 4, 2023
88 checks passed
@XrXr XrXr deleted the yjit-autosplat-opts branch July 4, 2023 14:57
github-merge-queue bot pushed a commit that referenced this pull request Jul 4, 2023
YJIT: Fix autosplat miscomp for blocks with optionals

When passing an array as the sole argument to `yield`, and the yieldee
takes more than 1 optional parameter, the array is expanded similar
to `*array` splat calls. This is called "autosplat" in
`setup_parameters_complex()`.

Previously, YJIT did not detect this autosplat condition. It passed the
array without expanding it, deviating from interpreter behavior.
Detect this conditon and refuse to compile it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants