Skip to content

ffi: fix optimized buffer conversions - #64639

Draft
trivikr wants to merge 2 commits into
nodejs:mainfrom
trivikr:ffi-buffer-sig-bigint
Draft

ffi: fix optimized buffer conversions#64639
trivikr wants to merge 2 commits into
nodejs:mainfrom
trivikr:ffi-buffer-sig-bigint

Conversation

@trivikr

@trivikr trivikr commented Jul 21, 2026

Copy link
Copy Markdown
Member

Fixes: #64638

Fast FFI calls with buffer or arraybuffer argument signatures accepted pointer-like values through the generic path, but rejected non-memory-backed values after optimization.

This change keeps the primary optimized entry point compatible with raw pointer conversions while retaining the specialized native fast path for memory-backed arguments. Optimized calls now consistently accept documented representations including bigint, strings, nullish values, buffers, typed arrays, and ArrayBuffer objects.


Assisted-by: openai: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++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 21, 2026
@trivikr trivikr added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jul 21, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 21, 2026
@nodejs-github-bot

This comment was marked as outdated.

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.31034% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.14%. Comparing base (54a5095) to head (c2e566c).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/ffi/fast-api.js 62.50% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #64639   +/-   ##
=======================================
  Coverage   90.14%   90.14%           
=======================================
  Files         744      744           
  Lines      242518   242495   -23     
  Branches    45685    45690    +5     
=======================================
- Hits       218611   218596   -15     
+ Misses      15396    15392    -4     
+ Partials     8511     8507    -4     
Files with missing lines Coverage Δ
lib/ffi.js 94.92% <ø> (-0.04%) ⬇️
src/ffi/fast.cc 68.13% <100.00%> (+1.46%) ⬆️
src/ffi/fast.h 100.00% <ø> (ø)
src/node_ffi.cc 71.12% <100.00%> (+0.03%) ⬆️
lib/internal/ffi/fast-api.js 82.53% <62.50%> (-1.86%) ⬇️

... and 29 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.

@nodejs-github-bot

This comment was marked as outdated.

@trivikr
trivikr force-pushed the ffi-buffer-sig-bigint branch 2 times, most recently from 139053a to d4a75ce Compare July 23, 2026 15:47
@trivikr trivikr removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 23, 2026
@trivikr

This comment was marked as outdated.

@trivikr trivikr added request-ci Add this label to start a Jenkins CI on a PR. ffi Issues and PRs related to experimental Foreign Function Interface support. labels Jul 23, 2026
Preserve pointer-like argument conversions for buffer and arraybuffer
signatures after Fast API optimization. Keep memory-backed arguments
on the specialized native fast path.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.6-sol
@trivikr
trivikr force-pushed the ffi-buffer-sig-bigint branch from d4a75ce to c2e566c Compare July 27, 2026 01:11
@trivikr trivikr added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 27, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 27, 2026
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@trivikr

trivikr commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

The test-ffi-fast-buffer is failing on rhel8-power9le and rhel9-ppc64le

---
duration_ms: 120100.117
exitcode: -15
severity: fail
stack: |-
  timeout
  (node:3758351) internal/test/binding: These APIs are for internal testing only. Do not use them.
  (Use `node --trace-warnings ...` to show where the warning was created)
  (node:3758351) ExperimentalWarning: FFI is an experimental feature and might change at any time
...

@trivikr
trivikr marked this pull request as draft July 27, 2026 18:46
@trivikr

trivikr commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

This PR will be rebased and made ready for review after the fix in ppc64 lands in #64792

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

This comment was marked as outdated.

@trivikr

trivikr commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

CI on node-test-commit-plinux: https://ci.nodejs.org/job/node-test-commit-plinux/65034/

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. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ffi: Fast FFI buffer/arraybuffer signatures reject bigint after optimization

4 participants