Skip to content

Commit 617e082

Browse files
authored
ffi: accept safe integer numbers for 64-bit arguments
Allow safe integer numbers as int64 and uint64 arguments alongside bigint values. Reject negative numbers for uint64 and numbers outside the safe integer range. Keep 64-bit return values as bigint. Apply validation and conversion across the Fast API, shared-buffer, and generic argument conversion paths. Add coverage for Number and BigInt boundaries, invalid inputs, and single-argument calls before and after optimization. Signed-off-by: HoonDongKang <d159123@naver.com> Assisted-by: Codex:Astra-medium PR-URL: #66197 Fixes: #66198 Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent e21f812 commit 617e082

8 files changed

Lines changed: 290 additions & 41 deletions

File tree

‎doc/api/ffi.md‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,16 @@ For 8-, 16-, and 32-bit integer types and for floating-point types, pass
548548
JavaScript `number` values that match the declared type.
549549

550550
For 64-bit integer types (`int64` and `uint64`), pass JavaScript `bigint`
551-
values.
551+
values within the declared type's range or safe integer `number` values.
552+
For `int64`, numbers must be between `Number.MIN_SAFE_INTEGER` and
553+
`Number.MAX_SAFE_INTEGER`, inclusive. For `uint64`, numbers must be between
554+
`0` and `Number.MAX_SAFE_INTEGER`, inclusive. This allows buffer lengths such
555+
as `buffer.byteLength` to be passed without an explicit `BigInt()` conversion.
556+
Use `bigint` for integers outside JavaScript's safe integer range.
557+
558+
Invalid arguments, including fractional numbers, `NaN`, infinities, and values
559+
outside these ranges, throw `ERR_INVALID_ARG_VALUE`. Return values for 64-bit
560+
integer types are always exposed as `bigint` values.
552561

553562
For pointer-like arguments:
554563

‎lib/internal/ffi-shared-buffer.js‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

33
const {
4+
BigInt,
45
DataView,
56
DataViewPrototypeGetBigInt64,
67
DataViewPrototypeGetBigUint64,
@@ -23,6 +24,7 @@ const {
2324
DataViewPrototypeSetUint32,
2425
DataViewPrototypeSetUint8,
2526
NumberIsInteger,
27+
NumberIsSafeInteger,
2628
ObjectDefineProperty,
2729
ReflectApply,
2830
TypeError,
@@ -132,14 +134,25 @@ function writeNumericArg(view, info, offset, arg, index) {
132134
return;
133135
}
134136
if (kind === 'i64') {
135-
if (typeof arg !== 'bigint' || arg < I64_MIN || arg > I64_MAX) {
137+
if (typeof arg === 'number') {
138+
if (!NumberIsSafeInteger(arg)) {
139+
throwFFIArgError(`Argument ${index} must be ${info.label}`);
140+
}
141+
arg = BigInt(arg);
142+
} else if (typeof arg !== 'bigint' || arg < I64_MIN || arg > I64_MAX) {
136143
throwFFIArgError(`Argument ${index} must be ${info.label}`);
137144
}
138145
sI64(view, offset, arg, true);
139146
return;
140147
}
148+
141149
if (kind === 'u64') {
142-
if (typeof arg !== 'bigint' || arg < 0n || arg > U64_MAX) {
150+
if (typeof arg === 'number') {
151+
if (!NumberIsSafeInteger(arg) || arg < 0) {
152+
throwFFIArgError(`Argument ${index} must be ${info.label}`);
153+
}
154+
arg = BigInt(arg);
155+
} else if (typeof arg !== 'bigint' || arg < 0n || arg > U64_MAX) {
143156
throwFFIArgError(`Argument ${index} must be ${info.label}`);
144157
}
145158
sU64(view, offset, arg, true);

‎lib/internal/ffi/fast-api.js‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
const {
44
ArrayBufferPrototypeGetDetached,
55
ArrayPrototypeIncludes,
6+
BigInt,
67
DataViewPrototypeGetBuffer,
78
NumberIsInteger,
9+
NumberIsSafeInteger,
810
ObjectDefineProperty,
911
ReflectApply,
1012
SafeWeakMap,
@@ -83,15 +85,26 @@ function throwFFIArgCountError(expected, actual) {
8385
`Invalid argument count: expected ${expected}, got ${actual}`);
8486
}
8587

86-
function validateFastIntegerArg(type, value, index) {
88+
function validateAndConvertFastIntegerArg(type, value, index) {
8789
const info = fastIntegerTypeInfo[type];
88-
if (info === undefined) return;
90+
if (info === undefined) return value;
91+
92+
// The native Fast API expects BigInt for 64-bit integer arguments.
93+
if (info.kind === 'bigint' && typeof value === 'number') {
94+
if (!NumberIsSafeInteger(value) || (info.min === 0n && value < 0)) {
95+
throwFFIArgError(`Argument ${index} must be ${info.label}`);
96+
}
97+
return BigInt(value);
98+
}
99+
89100
const validType = info.kind === 'number' ?
90101
typeof value === 'number' && NumberIsInteger(value) :
91102
typeof value === 'bigint';
92103
if (!validType || value < info.min || value > info.max) {
93104
throwFFIArgError(`Argument ${index} must be ${info.label}`);
94105
}
106+
107+
return value;
95108
}
96109

97110
function needsRawPointerConversion(type) {
@@ -223,7 +236,8 @@ function getFastArgumentIndexes(argumentsTypes) {
223236
}
224237

225238
function convertFastArg(type, value, stringState, index) {
226-
validateFastIntegerArg(type, value, index);
239+
value = validateAndConvertFastIntegerArg(type, value, index);
240+
227241
return needsPointerConversion(type) ?
228242
convertPointerArg(type, value, stringState, index) : value;
229243
}
@@ -295,9 +309,8 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
295309
if (arguments.length !== 1) {
296310
throwFFIArgCountError(1, arguments.length);
297311
}
298-
validateFastIntegerArg(t0, a0, 0);
312+
let arg = validateAndConvertFastIntegerArg(t0, a0, 0);
299313
validateFastPointerArg(t0, a0, 0);
300-
let arg = a0;
301314
if (needsNullPointerConversion(t0) &&
302315
(arg === null || arg === undefined)) {
303316
arg = 0n;

‎src/ffi/types.cc‎

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -648,28 +648,43 @@ Maybe<FFIArgumentCategory> ToFFIArgument(Environment* env,
648648

649649
*static_cast<uint32_t*>(ret) = arg->Uint32Value(context).FromJust();
650650
} else if (type == &ffi_type_sint64) {
651-
if (!arg->IsBigInt()) {
652-
THROW_ERR_INVALID_ARG_VALUE(env, "Argument %u must be an int64", index);
653-
return {};
654-
}
651+
if (arg->IsBigInt()) {
652+
bool lossless;
653+
int64_t value = arg.As<BigInt>()->Int64Value(&lossless);
654+
if (!lossless) {
655+
THROW_ERR_INVALID_ARG_VALUE(env, "Argument %u must be an int64", index);
656+
return {};
657+
}
655658

656-
bool lossless;
657-
*static_cast<int64_t*>(ret) = arg.As<BigInt>()->Int64Value(&lossless);
658-
if (!lossless) {
659-
THROW_ERR_INVALID_ARG_VALUE(env, "Argument %u must be an int64", index);
660-
return {};
659+
*static_cast<int64_t*>(ret) = value;
660+
} else {
661+
int64_t value;
662+
if (!GetStrictSignedInteger(
663+
arg, -kMaxSafeJsInteger, kMaxSafeJsInteger, &value)) {
664+
THROW_ERR_INVALID_ARG_VALUE(env, "Argument %u must be an int64", index);
665+
return {};
666+
}
667+
668+
*static_cast<int64_t*>(ret) = value;
661669
}
662670
} else if (type == &ffi_type_uint64) {
663-
if (!arg->IsBigInt()) {
664-
THROW_ERR_INVALID_ARG_VALUE(env, "Argument %u must be a uint64", index);
665-
return {};
666-
}
671+
if (arg->IsBigInt()) {
672+
bool lossless;
673+
uint64_t value = arg.As<BigInt>()->Uint64Value(&lossless);
674+
if (!lossless) {
675+
THROW_ERR_INVALID_ARG_VALUE(env, "Argument %u must be a uint64", index);
676+
return {};
677+
}
667678

668-
bool lossless;
669-
*static_cast<uint64_t*>(ret) = arg.As<BigInt>()->Uint64Value(&lossless);
670-
if (!lossless) {
671-
THROW_ERR_INVALID_ARG_VALUE(env, "Argument %u must be a uint64", index);
672-
return {};
679+
*static_cast<uint64_t*>(ret) = value;
680+
} else {
681+
uint64_t value;
682+
if (!GetStrictUnsignedInteger(arg, kMaxSafeJsInteger, &value)) {
683+
THROW_ERR_INVALID_ARG_VALUE(env, "Argument %u must be a uint64", index);
684+
return {};
685+
}
686+
687+
*static_cast<uint64_t*>(ret) = value;
673688
}
674689
} else if (type == &ffi_type_float) {
675690
if (!arg->IsNumber()) {

‎test/ffi/fixture_library/ffi_test_library.c‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,30 @@ FFI_EXPORT int32_t identity_i32(int32_t value) {
4949
return value;
5050
}
5151

52+
FFI_EXPORT int64_t identity_i64(int64_t value) {
53+
return value;
54+
}
55+
56+
FFI_EXPORT uint64_t identity_u64(uint64_t value) {
57+
return value;
58+
}
59+
60+
FFI_EXPORT int64_t identity_i64_fallback(void* pointer,
61+
int64_t value,
62+
void (*callback)(void)) {
63+
(void)pointer;
64+
(void)callback;
65+
return value;
66+
}
67+
68+
FFI_EXPORT uint64_t identity_u64_fallback(void* pointer,
69+
uint64_t value,
70+
void (*callback)(void)) {
71+
(void)pointer;
72+
(void)callback;
73+
return value;
74+
}
75+
5276
FFI_EXPORT char identity_char(char value) {
5377
return value;
5478
}

‎test/ffi/test-ffi-calls.js‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ test('ffi calls support integer arithmetic and char semantics', () => {
2222
assert.strictEqual(symbols.add_u32(0xFFFFFFFF, 1), 0);
2323
assert.strictEqual(symbols.add_i64(20n, 22n), 42n);
2424
assert.strictEqual(symbols.add_u64(20n, 22n), 42n);
25+
assert.strictEqual(symbols.add_i64(20, 22), symbols.add_i64(20n, 22n));
26+
assert.strictEqual(symbols.add_u64(20, 22), symbols.add_u64(20n, 22n));
27+
assert.strictEqual(symbols.add_i64(-20, 22n), 2n);
28+
assert.strictEqual(symbols.add_u64(20n, 22), 42n);
2529

2630
if (symbols.char_is_signed()) {
2731
assert.strictEqual(symbols.identity_char(-1), -1);
@@ -104,15 +108,18 @@ test('ffi strings and buffers cross the boundary correctly', () => {
104108
symbols.free_string(duplicated);
105109

106110
const buffer = Buffer.from([1, 2, 3, 4]);
107-
assert.strictEqual(symbols.sum_buffer(buffer, BigInt(buffer.length)), 10n);
108-
symbols.reverse_buffer(buffer, BigInt(buffer.length));
111+
assert.strictEqual(
112+
symbols.sum_buffer(buffer, buffer.length),
113+
symbols.sum_buffer(buffer, BigInt(buffer.length)),
114+
);
115+
symbols.reverse_buffer(buffer, buffer.length);
109116
assert.deepStrictEqual([...buffer], [4, 3, 2, 1]);
110117

111118
const typed = new Uint8Array([5, 6, 7, 8]);
112-
assert.strictEqual(symbols.sum_buffer(typed, BigInt(typed.byteLength)), 26n);
119+
assert.strictEqual(symbols.sum_buffer(typed, typed.byteLength), 26n);
113120

114121
const arrayBuffer = new Uint8Array([9, 10, 11, 12]).buffer;
115-
assert.strictEqual(symbols.sum_buffer(arrayBuffer, BigInt(arrayBuffer.byteLength)), 42n);
122+
assert.strictEqual(symbols.sum_buffer(arrayBuffer, arrayBuffer.byteLength), 42n);
116123
} finally {
117124
lib.close();
118125
}
@@ -204,13 +211,20 @@ test('ffi validates invalid arguments', () => {
204211
assert.throws(() => symbols.add_i16(40_000, 1), /Argument 0 must be an int16/);
205212
assert.throws(() => symbols.add_u16(Number.NaN, 1), /Argument 0 must be a uint16/);
206213
assert.throws(() => symbols.add_u16(70_000, 1), /Argument 0 must be a uint16/);
207-
assert.throws(() => symbols.add_i64(1, 2n), /Argument 0 must be an int64/);
208-
assert.throws(() => symbols.add_i64(1.5, 2n), /Argument 0 must be an int64/);
214+
assert.throws(() => symbols.add_i64(1.5, 2), /Argument 0 must be an int64/);
215+
assert.throws(() => symbols.add_i64(Number.NaN, 2), /Argument 0 must be an int64/);
216+
assert.throws(() => symbols.add_i64(Number.POSITIVE_INFINITY, 2), /Argument 0 must be an int64/);
217+
assert.throws(() => symbols.add_i64(Number.NEGATIVE_INFINITY, 2), /Argument 0 must be an int64/);
218+
assert.throws(() => symbols.add_i64(Number.MAX_SAFE_INTEGER + 1, 2), /Argument 0 must be an int64/);
219+
assert.throws(() => symbols.add_i64(Number.MIN_SAFE_INTEGER - 1, 2), /Argument 0 must be an int64/);
209220
assert.throws(() => symbols.add_i64(2n ** 63n, 2n), /Argument 0 must be an int64/);
210221
assert.throws(() => symbols.add_i64(-(2n ** 63n) - 1n, 2n), /Argument 0 must be an int64/);
211222
assert.throws(() => symbols.add_u64('1', 2n), /Argument 0 must be a uint64/);
212-
assert.throws(() => symbols.add_u64(1, 2n), /Argument 0 must be a uint64/);
213-
assert.throws(() => symbols.add_u64(Number.NaN, 2n), /Argument 0 must be a uint64/);
223+
assert.throws(() => symbols.add_u64(-1, 2), /Argument 0 must be a uint64/);
224+
assert.throws(() => symbols.add_u64(1.5, 2), /Argument 0 must be a uint64/);
225+
assert.throws(() => symbols.add_u64(Number.NaN, 2), /Argument 0 must be a uint64/);
226+
assert.throws(() => symbols.add_u64(Number.POSITIVE_INFINITY, 2), /Argument 0 must be a uint64/);
227+
assert.throws(() => symbols.add_u64(Number.MAX_SAFE_INTEGER + 1, 2), /Argument 0 must be a uint64/);
214228
assert.throws(() => symbols.add_u64(-1n, 2n), /Argument 0 must be a uint64/);
215229
assert.throws(() => symbols.add_u64(2n ** 64n, 2n), /Argument 0 must be a uint64/);
216230
assert.throws(() => symbols.identity_pointer(-1n), /Argument 0 must be a non-negative pointer bigint/);

‎test/ffi/test-ffi-fast-integer-validation.js‎

Lines changed: 76 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ test('fast FFI validates integer argument ranges', () => {
3232

3333
function callU32(value) { return functions.add_u32(value, 0); }
3434

35-
function callI64(value) { return functions.add_i64(value, 0n); }
35+
function callI64(value) { return functions.add_i64(value, 0); }
3636

37-
function callU64(value) { return functions.add_u64(value, 0n); }
37+
function callU64(value) { return functions.add_u64(value, 0); }
3838

3939
for (const [fn, value] of [
4040
[callI8, 0],
@@ -43,8 +43,8 @@ test('fast FFI validates integer argument ranges', () => {
4343
[callU16, 0],
4444
[callI32, 0],
4545
[callU32, 0],
46-
[callI64, 0n],
47-
[callU64, 0n],
46+
[callI64, 0],
47+
[callU64, 0],
4848
]) {
4949
optimize(fn, value);
5050
}
@@ -62,6 +62,24 @@ test('fast FFI validates integer argument ranges', () => {
6262
assert.throws(() => callU32(-1), expect);
6363
assert.throws(() => callU32(1.5), expect);
6464
assert.throws(() => callU32('1'), expect);
65+
assert.strictEqual(callI64(Number.MAX_SAFE_INTEGER),
66+
BigInt(Number.MAX_SAFE_INTEGER));
67+
assert.strictEqual(callI64(Number.MIN_SAFE_INTEGER),
68+
BigInt(Number.MIN_SAFE_INTEGER));
69+
assert.strictEqual(callU64(Number.MAX_SAFE_INTEGER),
70+
BigInt(Number.MAX_SAFE_INTEGER));
71+
assert.strictEqual(callI64((2n ** 63n) - 1n), (2n ** 63n) - 1n);
72+
assert.strictEqual(callU64((2n ** 64n) - 1n), (2n ** 64n) - 1n);
73+
assert.throws(() => callI64(Number.MAX_SAFE_INTEGER + 1), expect);
74+
assert.throws(() => callI64(Number.MIN_SAFE_INTEGER - 1), expect);
75+
assert.throws(() => callI64(1.5), expect);
76+
assert.throws(() => callI64(Number.NaN), expect);
77+
assert.throws(() => callI64(Number.POSITIVE_INFINITY), expect);
78+
assert.throws(() => callU64(-1), expect);
79+
assert.throws(() => callU64(Number.MAX_SAFE_INTEGER + 1), expect);
80+
assert.throws(() => callU64(1.5), expect);
81+
assert.throws(() => callU64(Number.NaN), expect);
82+
assert.throws(() => callU64(Number.POSITIVE_INFINITY), expect);
6583
assert.throws(() => callI64(2n ** 63n), expect);
6684
assert.throws(() => callU64(2n ** 64n), expect);
6785
} finally {
@@ -70,6 +88,60 @@ test('fast FFI validates integer argument ranges', () => {
7088
}
7189
});
7290

91+
test('fast FFI converts single i64/u64 Number arguments before and after optimization', () => {
92+
const { lib, functions } = ffi.dlopen(libraryPath, {
93+
identity_i64: { return: 'int64', arguments: ['int64'] },
94+
identity_u64: { return: 'uint64', arguments: ['uint64'] },
95+
});
96+
97+
try {
98+
// The native signature must have one argument to exercise the single-argument wrapper.
99+
function callI64(value) { return functions.identity_i64(value); }
100+
101+
function callU64(value) { return functions.identity_u64(value); }
102+
103+
for (const optimized of [false, true]) {
104+
if (optimized) {
105+
optimize(callI64, -42);
106+
optimize(callU64, 42);
107+
}
108+
109+
for (const value of [0, -0, 42, Number.MAX_SAFE_INTEGER, 0n, 42n]) {
110+
assert.strictEqual(callI64(value), BigInt(value));
111+
assert.strictEqual(callU64(value), BigInt(value));
112+
}
113+
for (const value of [-42, Number.MIN_SAFE_INTEGER,
114+
-(2n ** 63n), (2n ** 63n) - 1n]) {
115+
assert.strictEqual(callI64(value), BigInt(value));
116+
}
117+
assert.strictEqual(callU64((2n ** 64n) - 1n), (2n ** 64n) - 1n);
118+
119+
const signedError = {
120+
code: 'ERR_INVALID_ARG_VALUE',
121+
message: 'Argument 0 must be an int64',
122+
};
123+
const unsignedError = {
124+
code: 'ERR_INVALID_ARG_VALUE',
125+
message: 'Argument 0 must be a uint64',
126+
};
127+
for (const value of [Number.MAX_SAFE_INTEGER + 1, Number.MIN_SAFE_INTEGER - 1,
128+
1.5, NaN, Infinity, -Infinity, '1', null, undefined, true, {}]) {
129+
assert.throws(() => callI64(value), signedError);
130+
assert.throws(() => callU64(value), unsignedError);
131+
}
132+
for (const value of [-(2n ** 63n) - 1n, 2n ** 63n]) {
133+
assert.throws(() => callI64(value), signedError);
134+
}
135+
for (const value of [-1, -1n, 2n ** 64n]) {
136+
assert.throws(() => callU64(value), unsignedError);
137+
}
138+
}
139+
} finally {
140+
eval('%WaitForBackgroundOptimization()');
141+
lib.close();
142+
}
143+
});
144+
73145
test('fast FFI validates pointer BigInt ranges', () => {
74146
const lib = new ffi.DynamicLibrary(libraryPath);
75147
try {

0 commit comments

Comments
 (0)