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.

Sign up for GitHub

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

Jump to bottom

YJIT: Bump ec->cfp after setting cfp->jit_return #9072

Merged
merged 1 commit into from
Nov 30, 2023
Merged

YJIT: Bump ec->cfp after setting cfp->jit_return #9072

merged 1 commit into from
Nov 30, 2023

Conversation

k0kubun
Copy link
Member

@k0kubun k0kubun commented Nov 30, 2023

There's a race condition that, when stackprof runs rb_profile_frames() on its signal handler, ec->cfp could be pointing to a JIT frame without jit_return because we set cfp->jit_return after bumping ec->cfp, which trips the safeguard added by #8415.

This PR reorders these operations to make it stackprof-safe. It also optimizes lea mov into just sub for ISEQ calls.

Before

  0x563bdb0260e2: lea rax, [r13 - 0x38]
  # switch to new CFP
  0x563bdb0260e6: mov r13, rax
  0x563bdb0260e9: mov qword ptr [r12   0x10], r13
  # update cfp->jit_return
  0x563bdb0260ee: movabs rax, 0x563bdb0280ca
  0x563bdb0260f8: mov qword ptr [r13   0x30], rax

After

  # update cfp->jit_return                                                                                                                                            
  0x55b192d380e2: movabs rax, 0x55b192d3a0ca                                                                                                                          
  0x55b192d380ec: mov qword ptr [r13 - 8], rax                                                                                                                        
  # switch to new CFP
  0x55b192d380f0: sub r13, 0x38                                                    
  0x55b192d380f4: mov qword ptr [r12   0x10], r13

@k0kubun k0kubun marked this pull request as ready for review November 30, 2023 01:14
@matzbot matzbot requested a review from a team November 30, 2023 01:14
@maximecb maximecb merged commit d048bae into ruby:master Nov 30, 2023
98 checks passed
@XrXr XrXr deleted the yjit-jit-return branch November 30, 2023 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@k0kubun @maximecb