Skip to content

ffi: fix optimized buffer conversions - #64639

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
trivikr:ffi-buffer-sig-bigint
Jul 30, 2026
Merged

ffi: fix optimized buffer conversions#64639
nodejs-github-bot merged 1 commit 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 (e7904d4) to head (9e1a804).
⚠️ Report is 6 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%   -0.01%     
==========================================
  Files         746      746              
  Lines      242764   242741      -23     
  Branches    45739    45735       -4     
==========================================
- Hits       218848   218819      -29     
+ Misses      15426    15422       -4     
- Partials     8490     8500      +10     
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 26 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
@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

This comment was marked as outdated.

@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

@nodejs-github-bot

This comment was marked as outdated.

@trivikr

This comment was marked as outdated.

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 700494d to 9e1a804 Compare July 30, 2026 02:52
@trivikr
trivikr marked this pull request as ready for review July 30, 2026 02:53
@trivikr trivikr added request-ci Add this label to start a Jenkins CI on a PR. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jul 30, 2026
@trivikr

trivikr commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

@ShogunPanda @avivkeller Can one of you please provide your approval again?
The bug which was causing tests to fail was fixed in #64792, and this PR was rebased.

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

Copy link
Copy Markdown
Collaborator

@trivikr trivikr added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 30, 2026
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 30, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/64639
βœ”  Done loading data for nodejs/node/pull/64639
----------------------------------- PR info ------------------------------------
Title      ffi: fix optimized buffer conversions (#64639)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     trivikr:ffi-buffer-sig-bigint -> nodejs:main
Labels     c++, lib / src, needs-ci, ffi
Commits    1
 - ffi: fix optimized buffer conversions
Committers 1
 - Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/64639
Fixes: https://github.com/nodejs/node/issues/64638
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Aviv Keller <me@aviv.sh>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/64639
Fixes: https://github.com/nodejs/node/issues/64638
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Aviv Keller <me@aviv.sh>
--------------------------------------------------------------------------------
   β„Ή  This PR was created on Tue, 21 Jul 2026 03:43:06 GMT
   βœ”  Approvals: 2
   βœ”  - Paolo Insogna (@ShogunPanda) (TSC): https://github.com/nodejs/node/pull/64639#pullrequestreview-4819398365
   βœ”  - Aviv Keller (@avivkeller): https://github.com/nodejs/node/pull/64639#pullrequestreview-4783382497
   ✘  2 GitHub CI job(s) cancelled:
   ✘    - lint-readme: CANCELLED (https://github.com/nodejs/node/actions/runs/30509655206/job/90766758617)
   ✘    - lint-pr-url: CANCELLED (https://github.com/nodejs/node/actions/runs/30509655206/job/90766758661)
   β„Ή  Last Full PR CI on 2026-07-30T04:42:36Z: https://ci.nodejs.org/job/node-test-pull-request/75302/
- Querying data for job/node-test-pull-request/75302/
βœ”  Build data downloaded
   βœ”  Last Jenkins CI successful
--------------------------------------------------------------------------------
   βœ”  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/30567472928

@trivikr trivikr added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jul 30, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 30, 2026
@nodejs-github-bot
nodejs-github-bot merged commit f85fdab into nodejs:main Jul 30, 2026
107 of 109 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in f85fdab

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

Labels

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