Skip to content

ffi: preserve link register in ppc64 trampoline - #64792

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:ffi-link-register-ppc64-trampoline
Open

ffi: preserve link register in ppc64 trampoline#64792
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:ffi-link-register-ppc64-trampoline

Conversation

@trivikr

@trivikr trivikr commented Jul 27, 2026

Copy link
Copy Markdown
Member

Refs: #64639 (comment)

The PPC64LE fast FFI trampoline uses bl .+4 to obtain the address
needed to load its target literal. The bl instruction overwrites the
caller's link register, but the trampoline did not restore it before
tail-branching to the native target.

As a result, optimized FFI calls could return into the trampoline
instead of returning to V8, causing an infinite loop. This manifested
as 120-second timeouts on rhel8-power9le and rhel9-ppc64le.

Preserve the caller's link register in r0 and restore it before the
tail branch. The target literal padding is also selected according to
the GP argument count so that the literal remains 8-byte aligned.


Assisted-by: codex:gpt-5.6-sol

The ppc64 fast FFI trampoline uses `bl` to obtain the address used to
load its target literal. This overwrites the caller's link register, so
optimized FFI calls return into the trampoline and loop indefinitely.

Save and restore the caller's link register around the branch. Also
align the target literal for both even and odd GP argument counts.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: codex:gpt-5.6-sol
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Jul 27, 2026
@trivikr trivikr added ffi Issues and PRs related to experimental Foreign Function Interface support. request-ci Add this label to start a Jenkins CI on a PR. labels Jul 27, 2026
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.14%. Comparing base (6a3d80f) to head (b39984e).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64792      +/-   ##
==========================================
- Coverage   90.16%   90.14%   -0.02%     
==========================================
  Files         744      744              
  Lines      242518   242518              
  Branches    45705    45693      -12     
==========================================
- Hits       218670   218622      -48     
- Misses      15357    15401      +44     
- Partials     8491     8495       +4     

see 28 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@trivikr

trivikr commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

Verified that the ppc64 test failure which detected this was successful in #64639 (comment)

@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 28, 2026
@nodejs-github-bot

This comment was marked as outdated.

@trivikr trivikr added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 28, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. ffi Issues and PRs related to experimental Foreign Function Interface support. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants