mirrors/bun.sh
1
0
Fork 0
mirror of https://github.com/oven-sh/bun synced 2026-02-02 15:08:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: v8::Value::IsInt32()/IsUint32() edge cases (#25548)

Browse Source 
### What does this PR do?

- fixes both functions returning false for double-encoded values (even
if the numeric value is a valid int32/uint32)
- fixes IsUint32() returning false for values that don't fit in int32
- fixes the test from #22462 not testing anything (the native functions
were being passed a callback to run garbage collection as the first
argument, so it was only ever testing what the type check APIs returned
for that function)
- extends the test to cover the first edge case above

### How did you verify your code works?

The new tests fail without these fixes.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This commit is contained in:
190n
2025-12-17 00:52:16 -08:00
committed by GitHub
parent 1e86cebd74
commit fa5a5bbe55
4 changed files with 72 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/bun.js/bindings/v8/V8Value.cpp
View File

@@ -26,7 +26,7 @@ bool Value::IsNumber() const
bool Value::IsUint32() const
{
return localToJSValue().isUInt32();
return localToJSValue().isUInt32AsAnyInt();
}
bool Value::IsUndefined() const
@@ -81,7 +81,7 @@ bool Value::IsArray() const
bool Value::IsInt32() const
{
return localToJSValue().isInt32();
return localToJSValue().isInt32AsAnyInt();
}
bool Value::IsBigInt() const

17
test/v8/v8-module/main.js
View File

@@ -5,28 +5,15 @@ const buildMode = process.argv[5];
const tests = require("./module")(buildMode === "debug");
// Custom JSON reviver to handle BigInt
function parseArgs(str) {
return JSON.parse(str, (_, value) =>
value && typeof value === "object" && "__bigint__" in value ? BigInt(value.__bigint__) : value,
);
}
const testName = process.argv[2];
const args = parseArgs(process.argv[3] ?? "[]");
const args = eval(process.argv[3] ?? "[]");
const thisValue = JSON.parse(process.argv[4] ?? "null");
function runGC() {
if (typeof Bun !== "undefined") {
Bun.gc(true);
}
}
const fn = tests[testName];
if (typeof fn !== "function") {
throw new Error("Unknown test:", testName);
}
const result = fn.apply(thisValue, [runGC, ...args]);
const result = fn.apply(thisValue, [...args]);
if (result) {
console.log(result == global);
throw new Error(result);

122
test/v8/v8.test.ts
View File

@@ -1,4 +1,5 @@
import { spawn } from "bun";
import { jscDescribe } from "bun:jsc";
import { beforeAll, describe, expect, it } from "bun:test";
import { bunEnv, bunExe, isASAN, isBroken, isMusl, isWindows, nodeExe, tmpdirSync } from "harness";
import assert from "node:assert";
@@ -124,135 +125,141 @@ describe.todoIf(isBroken && isMusl)("node:v8", () => {
describe("module lifecycle", () => {
it("can call a basic native function", async () => {
await checkSameOutput("test_v8_native_call", []);
await checkSameOutput("test_v8_native_call");
});
});
describe("primitives", () => {
it("can create and distinguish between null, undefined, true, and false", async () => {
await checkSameOutput("test_v8_primitives", []);
await checkSameOutput("test_v8_primitives");
});
});
describe("Value type checks", () => {
it("matches Node for IsMap/IsArray/IsInt32/IsBigInt on representative values", async () => {
// Each entry is an argument list; we wrap each value so the addon receives it as info[0].
const cases: any[][] = [
[new Map()],
[[]],
[42],
[2147483647], // INT32_MAX
[2147483648], // INT32_MAX + 1 (should not be Int32)
[-2147483648], // INT32_MIN
[-2147483649], // INT32_MIN - 1 (should not be Int32)
[123n],
[3.14],
["string"],
[{}],
];
for (const args of cases) {
it("Math.fround returns a double-encoded value", () => {
// If this fails, you need to find a new way to make a JSValue which uses the double encoding
// but holds an int32 value (maybe Float64Array?)
expect(jscDescribe(Math.fround(1))).toBe("Double: 4607182418800017408, 1.000000");
});
it.each([
// Each entry should eval() to an array of arguments
"[new Map()]",
"[[]]",
"[42]",
"[2 ** 31 - 1]", // INT32_MAX
"[2 ** 31]", // INT32_MAX + 1 (should not be Int32)
"[-(2 ** 31)]", // INT32_MIN
"[-(2 ** 31) - 1]", // INT32_MIN - 1 (should not be Int32)
"[2 ** 32 - 1]", // UINT32_MAX
"[2 ** 32]", // UINT32_MAX + 1
"[Math.fround(1)]", // Value represented as a double but whose numeric value fits in the int32 range (should be int32)
"[123n]",
"[3.14]",
"['string']",
"[{}]",
])("matches Node for IsMap/IsArray/IsInt32/IsBigInt on %s", async args => {
await checkSameOutput("test_v8_value_type_checks", args);
}
});
});
describe("Number", () => {
it("can create small integer", async () => {
await checkSameOutput("test_v8_number_int", []);
await checkSameOutput("test_v8_number_int");
});
// non-i32 v8::Number is not implemented yet
it("can create large integer", async () => {
await checkSameOutput("test_v8_number_large_int", []);
await checkSameOutput("test_v8_number_large_int");
});
it("can create fraction", async () => {
await checkSameOutput("test_v8_number_fraction", []);
await checkSameOutput("test_v8_number_fraction");
});
});
describe("String", () => {
it("can create and read back strings with only ASCII characters", async () => {
await checkSameOutput("test_v8_string_ascii", []);
await checkSameOutput("test_v8_string_ascii");
});
// non-ASCII strings are not implemented yet
it("can create and read back strings with UTF-8 characters", async () => {
await checkSameOutput("test_v8_string_utf8", []);
await checkSameOutput("test_v8_string_utf8");
});
it("handles replacement correctly in strings with invalid UTF-8 sequences", async () => {
await checkSameOutput("test_v8_string_invalid_utf8", []);
await checkSameOutput("test_v8_string_invalid_utf8");
});
it("can create strings from null-terminated Latin-1 data", async () => {
await checkSameOutput("test_v8_string_latin1", []);
await checkSameOutput("test_v8_string_latin1");
});
describe("WriteUtf8", () => {
it("truncates the string correctly", async () => {
await checkSameOutput("test_v8_string_write_utf8", []);
await checkSameOutput("test_v8_string_write_utf8");
});
});
});
describe("External", () => {
it("can create an external and read back the correct value", async () => {
await checkSameOutput("test_v8_external", []);
await checkSameOutput("test_v8_external");
});
});
describe("Value", () => {
it("can compare values using StrictEquals", async () => {
await checkSameOutput("test_v8_strict_equals", []);
await checkSameOutput("test_v8_strict_equals");
});
});
describe("Object", () => {
it("can create an object and set properties", async () => {
await checkSameOutput("test_v8_object", []);
await checkSameOutput("test_v8_object");
});
it("can get properties by key using Object::Get(context, key)", async () => {
await checkSameOutput("test_v8_object_get_by_key", []);
await checkSameOutput("test_v8_object_get_by_key");
});
it("can get array elements by index using Object::Get(context, index)", async () => {
await checkSameOutput("test_v8_object_get_by_index", []);
await checkSameOutput("test_v8_object_get_by_index");
});
it("correctly handles exceptions from get and set", async () => {
await checkSameOutput("test_v8_object_get_set_exceptions", []);
await checkSameOutput("test_v8_object_get_set_exceptions");
});
});
describe("Array", () => {
it("can create an array from a C array of Locals", async () => {
await checkSameOutput("test_v8_array_new", []);
await checkSameOutput("test_v8_array_new");
});
it("can create an array with a specific length", async () => {
await checkSameOutput("test_v8_array_new_with_length", []);
await checkSameOutput("test_v8_array_new_with_length");
});
it("can create an array from a callback", async () => {
await checkSameOutput("test_v8_array_new_with_callback", []);
await checkSameOutput("test_v8_array_new_with_callback");
});
it("correctly reports array length", async () => {
await checkSameOutput("test_v8_array_length", []);
await checkSameOutput("test_v8_array_length");
});
it("can iterate over array elements with callbacks", async () => {
await checkSameOutput("test_v8_array_iterate", []);
await checkSameOutput("test_v8_array_iterate");
});
});
describe("ObjectTemplate", () => {
it("creates objects with internal fields", async () => {
await checkSameOutput("test_v8_object_template", []);
await checkSameOutput("test_v8_object_template");
});
});
describe("FunctionTemplate", () => {
it("keeps the data parameter alive", async () => {
await checkSameOutput("test_v8_function_template", []);
await checkSameOutput("test_v8_function_template");
});
});
describe("Function", () => {
it("correctly receives all its arguments from JS", async () => {
await checkSameOutput("print_values_from_js", [5.0, true, null, false, "async meow", {}]);
await checkSameOutput("print_native_function", []);
await checkSameOutput("print_values_from_js", "[5.0, true, null, false, 'async meow', {}]");
await checkSameOutput("print_native_function");
});
it("correctly receives the this value from JS", async () => {
await checkSameOutput("call_function_with_weird_this_values", []);
await checkSameOutput("call_function_with_weird_this_values");
});
});
@@ -272,20 +279,20 @@ describe.todoIf(isBroken && isMusl)("node:v8", () => {
describe("Global", () => {
it("can create, modify, and read the value from global handles", async () => {
await checkSameOutput("test_v8_global", []);
await checkSameOutput("test_v8_global");
});
});
describe("HandleScope", () => {
it("can hold a lot of locals", async () => {
await checkSameOutput("test_many_v8_locals", []);
await checkSameOutput("test_many_v8_locals");
});
// Skip on ASAN: false positives due to dynamic library boundary crossing where
// Bun is built with ASAN+UBSAN but the native addon is not
it.skipIf(isASAN)(
"keeps GC objects alive",
async () => {
await checkSameOutput("test_handle_scope_gc", []);
await checkSameOutput("test_handle_scope_gc");
},
10000,
);
@@ -293,30 +300,30 @@ describe.todoIf(isBroken && isMusl)("node:v8", () => {
describe("EscapableHandleScope", () => {
it("keeps handles alive in the outer scope", async () => {
await checkSameOutput("test_v8_escapable_handle_scope", []);
await checkSameOutput("test_v8_escapable_handle_scope");
});
});
describe("MaybeLocal", () => {
it("correctly handles ToLocal and ToLocalChecked operations", async () => {
await checkSameOutput("test_v8_maybe_local", []);
await checkSameOutput("test_v8_maybe_local");
});
});
describe("uv_os_getpid", () => {
it.skipIf(isWindows)("returns the same result as getpid on POSIX", async () => {
await checkSameOutput("test_uv_os_getpid", []);
await checkSameOutput("test_uv_os_getpid");
});
});
describe("uv_os_getppid", () => {
it.skipIf(isWindows)("returns the same result as getppid on POSIX", async () => {
await checkSameOutput("test_uv_os_getppid", []);
await checkSameOutput("test_uv_os_getppid");
});
});
});
async function checkSameOutput(testName: string, args: any[], thisValue?: any) {
async function checkSameOutput(testName: string, args?: string, thisValue?: any) {
const [nodeResultResolution, bunReleaseResultResolution, bunDebugResultResolution] = await Promise.allSettled([
runOn(Runtime.node, BuildMode.release, testName, args, thisValue),
runOn(Runtime.bun, BuildMode.release, testName, args, thisValue),
@@ -344,12 +351,11 @@ async function checkSameOutput(testName: string, args: any[], thisValue?: any) {
return nodeResult;
}
// Custom JSON serialization that handles BigInt
function serializeArgs(args: any[]): string {
return JSON.stringify(args, (_, value) => (typeof value === "bigint" ? { __bigint__: value.toString() } : value));
}
async function runOn(runtime: Runtime, buildMode: BuildMode, testName: string, jsArgs: any[], thisValue?: any) {
/**
* @param jsArgs should eval() to an array
* @param thisValue will be JSON stringified
*/
async function runOn(runtime: Runtime, buildMode: BuildMode, testName: string, jsArgs?: string, thisValue?: any) {
if (runtime == Runtime.node) {
assert(buildMode == BuildMode.release);
}
@@ -366,7 +372,7 @@ async function runOn(runtime: Runtime, buildMode: BuildMode, testName: string, j
...(runtime == Runtime.bun ? ["--smol"] : []),
join(baseDir, "main.js"),
testName,
serializeArgs(jsArgs),
jsArgs ?? "[]",
JSON.stringify(thisValue ?? null),
];
if (buildMode == BuildMode.debug) {