fix(napi): napi_get_value_uint32 now handles int32s correctly (#16494)

cmake/CompilerFlags.cmake

@@ -230,6 +230,19 @@ else()
   endif()
 endif()
 
+# --- Runtime safety ---
+
+# NOTE: these can get noisy when building with jsc from source. Not a problem when using downloaded objects from gh.
+# NOTE: do not enable shift checks. Many hash functions rely on overflows.
+# NOTE: make sure llvm-symbolizer is in your PATH
+if (ENABLE_ASSERTIONS)
+  register_compiler_flags(
+    DESCRIPTION "Enable undefined behavior sanitizer (UBSan)"
+    -fsanitize=alignment,nullability,bounds,unreachable,pointer-overflow,signed-integer-overflow ${UNIX}
+    /fsanitize:alignment,nullability,bounds,unreachable,pointer-overflow,signed-integer-overflow ${WIN32}
+  )
+endif()
+
 # --- Diagnostics ---
 if(UNIX)
   register_compiler_flags(

src/bun.js/bindings/JSBundlerPlugin.cpp

@@ -300,6 +300,7 @@ int BundlerPlugin::NativePluginList::call(JSC::VM& vm, BundlerPlugin* plugin, in
 
         if (filters[i].match(vm, path)) {
             Bun::NapiExternal* external = callbacks[i].external;
+            ASSERT(onBeforeParseArgs != nullptr);
             if (external) {
                 onBeforeParseArgs->external = external->value();
             } else {

src/bun.js/bindings/napi.cpp

@@ -563,6 +563,7 @@ public:
         ASSERT(jsCast<NAPIFunction*>(callframe->jsCallee()));
         auto* function = static_cast<NAPIFunction*>(callframe->jsCallee());
         auto* env = toNapi(globalObject);
+        ASSERT(function->m_method);
         auto* callback = reinterpret_cast<napi_callback>(function->m_method);
         JSC::VM& vm = globalObject->vm();
 
@@ -2098,7 +2099,7 @@ extern "C" napi_status napi_get_value_int32(napi_env env, napi_value value, int3
     JSC::JSValue jsValue = toJS(value);
     NAPI_RETURN_EARLY_IF_FALSE(env, jsValue.isNumber(), napi_number_expected);
 
-    *result = jsValue.isInt32() ? jsValue.asInt32() : JSC::toInt32(jsValue.asDouble());
+    *result = jsValue.isInt32() ? jsValue.asInt32() : JSC::toInt32(jsValue.asNumber());
     NAPI_RETURN_SUCCESS(env);
 }
 
@@ -2109,8 +2110,8 @@ extern "C" napi_status napi_get_value_uint32(napi_env env, napi_value value, uin
     NAPI_CHECK_ARG(env, value);
     JSC::JSValue jsValue = toJS(value);
     NAPI_RETURN_EARLY_IF_FALSE(env, jsValue.isNumber(), napi_number_expected);
+    *result = jsValue.isUInt32() ? jsValue.asUInt32() : JSC::toUInt32(jsValue.asNumber());
 
-    *result = jsValue.isUInt32() ? jsValue.asUInt32() : JSC::toUInt32(jsValue.asDouble());
     NAPI_RETURN_SUCCESS(env);
 }
 

test/napi/napi-app/module.js

@@ -1,12 +1,7 @@
+const assert = require("node:assert");
 const nativeTests = require("./build/Release/napitests.node");
 const secondAddon = require("./build/Release/second_addon.node");
 
-function assert(ok) {
-  if (!ok) {
-    throw new Error("assertion failed");
-  }
-}
-
 async function gcUntil(fn) {
   const MAX = 100;
   for (let i = 0; i < MAX; i++) {
@@ -157,7 +152,7 @@ nativeTests.test_number_integer_conversions_from_js = () => {
     const actualOutput = nativeTests.double_to_i32(input);
     console.log(`${input} as i32 => ${actualOutput}`);
     if (actualOutput !== expectedOutput) {
-      console.error("wrong");
+      console.error(`${input}: ${actualOutput} != ${expectedOutput}`);
     }
   }
 
@@ -188,7 +183,7 @@ nativeTests.test_number_integer_conversions_from_js = () => {
     const actualOutput = nativeTests.double_to_u32(input);
     console.log(`${input} as u32 => ${actualOutput}`);
     if (actualOutput !== expectedOutput) {
-      console.error("wrong");
+      console.error(`${input}: ${actualOutput} != ${expectedOutput}`);
     }
   }
 
@@ -223,7 +218,7 @@ nativeTests.test_number_integer_conversions_from_js = () => {
       `${typeof input == "number" ? input.toFixed(2) : input} as i64 => ${typeof actualOutput == "number" ? actualOutput.toFixed(2) : actualOutput}`,
     );
     if (actualOutput !== expectedOutput) {
-      console.error("wrong");
+      console.error(`${input}: ${actualOutput} != ${expectedOutput}`);
     }
   }
 };

test/napi/napi.test.ts

@@ -391,11 +391,14 @@ function checkSameOutput(test: string, args: any[] | string) {
   let bunResult = runOn(bunExe(), test, args);
   // remove all debug logs
   bunResult = bunResult.replaceAll(/^\[\w+\].+$/gm, "").trim();
-  expect(bunResult).toBe(nodeResult);
+  expect(bunResult).toEqual(nodeResult);
   return nodeResult;
 }
 
 function runOn(executable: string, test: string, args: any[] | string) {
+  // when the inspector runs (can be due to VSCode extension), there is
+  // a bug that in debug modes the console logs extra stuff
+  const { BUN_INSPECT_CONNECT_TO: _, ...rest } = bunEnv;
   const exec = spawnSync({
     cmd: [
       executable,
@@ -404,7 +407,7 @@ function runOn(executable: string, test: string, args: any[] | string) {
       test,
       typeof args == "string" ? args : JSON.stringify(args),
     ],
-    env: bunEnv,
+    env: rest,
   });
   const errs = exec.stderr.toString();
   if (errs !== "") {