Revert to getString() - view() was throwing exceptions

The view() approach was failing in x64 ASAN builds. The logs showed: - WriteUtf8 returning 0 (no bytes written) - Buffer filled with garbage (0x02 bytes) - This happens because view() throws an exception on line 168 The view() call was hitting RETURN_IF_EXCEPTION and returning 0, leaving the buffer unmodified. Using getString() which returns a WTF::String is the correct and safe approach. Changes from main: - Use unsigned_length in comparisons (not length) - Reorder surrogate handling before null terminator - Increase test timeout to 120s - Initialize and enlarge test buffers to 32 bytes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Update main.cpp
2026-02-16 22:01:47 +00:00 · 2025-11-03 07:12:38 +00:00 · 2025-11-02 22:53:11 -08:00 · 2025-11-03 06:02:45 +00:00 · 2025-11-03 05:13:02 +00:00 · 2025-11-03 04:04:25 +00:00
3 changed files with 74 additions and 34 deletions
--- a/src/bun.js/bindings/v8/V8String.cpp
+++ b/src/bun.js/bindings/v8/V8String.cpp
@@ -162,18 +162,15 @@ int String::WriteUtf8(Isolate* isolate, char* buffer, int length, int* nchars_re
    auto jsString = localToObjectPointer<JSString>();
    WTF::String string = jsString->getString(isolate->globalObject());

-    size_t unsigned_length = length < 0 ? SIZE_MAX : length;
+    size_t unsigned_length = length < 0 ? SIZE_MAX : static_cast<size_t>(length);

    uint64_t result = string.is8Bit() ? TextEncoder__encodeInto8(string.span8().data(), string.span8().size(), buffer, unsigned_length)
                                      : TextEncoder__encodeInto16(string.span16().data(), string.span16().size(), buffer, unsigned_length);
    uint32_t read = static_cast<uint32_t>(result);
    uint32_t written = static_cast<uint32_t>(result >> 32);

-    if (written < length && read == string.length()) {
-        buffer[written] = 0;
-        written++;
-    }
-    if (read < string.length() && U16_IS_SURROGATE(string[read]) && written + 3 <= length) {
+    // Handle unpaired surrogate before adding null terminator
+    if (read < string.length() && U16_IS_SURROGATE(string[read]) && written + 3 <= unsigned_length) {
        // encode unpaired surrogate
        char16_t surrogate = string[read];
        buffer[written + 0] = 0xe0 | (surrogate >> 12);
@@ -182,6 +179,11 @@ int String::WriteUtf8(Isolate* isolate, char* buffer, int length, int* nchars_re
        written += 3;
        read += 1;
    }
+
+    if (written < unsigned_length && read == string.length()) {
+        buffer[written] = 0;
+        written++;
+    }
    if (nchars_ref) {
        *nchars_ref = read;
    }
--- a/test/v8/v8-module/main.cpp
+++ b/test/v8/v8-module/main.cpp
@@ -439,15 +439,21 @@ static Local<Object> setup_object_with_string_field(Isolate *isolate,

 static void examine_object_fields(Isolate *isolate, Local<Object> o,
                                  int expected_field0, int expected_field1) {
-  char buf[16];
+  char buf[32] = {0};
  HandleScope hs(isolate);
-  o->GetInternalField(0).As<String>()->WriteUtf8(isolate, buf);
-  assert(atoi(buf) == expected_field0);
+  o->GetInternalField(0).As<String>()->WriteUtf8(isolate, buf, sizeof(buf));
+  if (atoi(buf) != expected_field0) {
+    printf("expected_field0 = %d, got %s\n", expected_field0, buf);
+    assert(false);
+  }

  Local<Value> field1 = o->GetInternalField(1).As<Value>();
  if (field1->IsString()) {
-    field1.As<String>()->WriteUtf8(isolate, buf);
-    assert(atoi(buf) == expected_field1);
+    field1.As<String>()->WriteUtf8(isolate, buf, sizeof(buf));
+    if (atoi(buf) != expected_field1) {
+      printf("expected_field1 = %d, got %s\n", expected_field1, buf);
+      assert(false);
+    }
  } else {
    assert(field1->IsUndefined());
  }
@@ -503,9 +509,12 @@ void test_handle_scope_gc(const FunctionCallbackInfo<Value> &info) {

    // try to use all mini strings
    for (size_t j = 0; j < num_small_allocs; j++) {
-      char buf[16];
-      mini_strings[j]->WriteUtf8(isolate, buf);
-      assert(atoi(buf) == (int)j);
+      char buf[32] = {0};
+      mini_strings[j]->WriteUtf8(isolate, buf, sizeof(buf));
+      if (atoi(buf) != (int)j) {
+        printf("mini_strings[%zu] = %d\n", j, atoi(buf));
+        assert(false);
+      }
    }

    for (size_t j = 0; j < num_small_allocs; j++) {
@@ -533,7 +542,10 @@ void test_handle_scope_gc(const FunctionCallbackInfo<Value> &info) {
  for (size_t i = 0; i < num_strings; i++) {
    huge_strings[i]->WriteUtf8(isolate, string_data);
    for (size_t j = 0; j < string_size - 1; j++) {
-      assert(string_data[j] == (char)(i + 1));
+      if (string_data[j] != (char)(i + 1)) {
+        printf("string_data[%zu] = %c\n", j, string_data[j]);
+        assert(false);
+      }
    }
  }

@@ -573,7 +585,7 @@ void test_v8_escapable_handle_scope(const FunctionCallbackInfo<Value> &info) {
  LOG_VALUE_KIND(t);

  char buf[16];
-  s->WriteUtf8(isolate, buf);
+  s->WriteUtf8(isolate, buf, sizeof(buf));
  LOG_EXPR(buf);
  LOG_EXPR(n->Value());
 }
--- a/test/v8/v8.test.ts
+++ b/test/v8/v8.test.ts
@@ -103,24 +103,50 @@ async function build(
  console.log(err);
 }

-describe.todoIf(isBroken && isMusl)("node:v8", () => {
-  beforeAll(async () => {
-    // set up clean directories for our 4 builds
-    directories.bunRelease = tmpdirSync();
-    directories.bunDebug = tmpdirSync();
-    directories.node = tmpdirSync();
-    directories.badModules = tmpdirSync();
+describe.concurrent.todoIf(isBroken && isMusl)("node:v8", () => {
+  beforeAll(
+    async () => {
+      // set up clean directories for our 4 builds
+      directories.bunRelease = tmpdirSync();
+      directories.bunDebug = tmpdirSync();
+      directories.node = tmpdirSync();
+      directories.badModules = tmpdirSync();

-    await install(srcDir, directories.bunRelease, Runtime.bun);
-    await install(srcDir, directories.bunDebug, Runtime.bun);
-    await install(srcDir, directories.node, Runtime.node);
-    await install(join(__dirname, "bad-modules"), directories.badModules, Runtime.node);
-
-    await build(srcDir, directories.bunRelease, Runtime.bun, BuildMode.release);
-    await build(srcDir, directories.bunDebug, Runtime.bun, BuildMode.debug);
-    await build(srcDir, directories.node, Runtime.node, BuildMode.release);
-    await build(join(__dirname, "bad-modules"), directories.badModules, Runtime.node, BuildMode.release);
-  });
+      if (isWindows) {
+        let queue = [
+          () => install(srcDir, directories.bunRelease, Runtime.bun),
+          () => build(srcDir, directories.bunRelease, Runtime.bun, BuildMode.release),
+          () => install(srcDir, directories.bunDebug, Runtime.bun),
+          () => build(srcDir, directories.bunDebug, Runtime.bun, BuildMode.debug),
+          () => install(srcDir, directories.node, Runtime.node),
+          () => build(srcDir, directories.node, Runtime.node, BuildMode.release),
+          () => install(join(__dirname, "bad-modules"), directories.badModules, Runtime.node),
+          () => build(join(__dirname, "bad-modules"), directories.badModules, Runtime.node, BuildMode.release),
+        ];
+        for (const task of queue) {
+          await task();
+        }
+      } else {
+        await Promise.all([
+          install(srcDir, directories.bunRelease, Runtime.bun).then(() =>
+            build(srcDir, directories.bunRelease, Runtime.bun, BuildMode.release),
+          ),
+          install(srcDir, directories.bunDebug, Runtime.bun).then(() =>
+            build(srcDir, directories.bunDebug, Runtime.bun, BuildMode.debug),
+          ),
+          install(srcDir, directories.node, Runtime.node).then(() =>
+            build(srcDir, directories.node, Runtime.node, BuildMode.release),
+          ),
+          install(join(__dirname, "bad-modules"), directories.badModules, Runtime.node).then(() =>
+            build(join(__dirname, "bad-modules"), directories.badModules, Runtime.node, BuildMode.release),
+          ),
+        ]);
+      }
+    },
+    {
+      timeout: 1000000,
+    },
+  );

  describe("module lifecycle", () => {
    it("can call a basic native function", async () => {
@@ -261,7 +287,7 @@ describe.todoIf(isBroken && isMusl)("node:v8", () => {
    });
    it("keeps GC objects alive", async () => {
      await checkSameOutput("test_handle_scope_gc", []);
-    }, 10000);
+    }, 120000); // 2 minutes for ASAN/debug builds
  });

  describe("EscapableHandleScope", () => {
Author	SHA1	Message	Date
Claude Bot	0dcb535df5	Revert to getString() - view() was throwing exceptions The view() approach was failing in x64 ASAN builds. The logs showed: - WriteUtf8 returning 0 (no bytes written) - Buffer filled with garbage (0x02 bytes) - This happens because view() throws an exception on line 168 The view() call was hitting RETURN_IF_EXCEPTION and returning 0, leaving the buffer unmodified. Using getString() which returns a WTF::String is the correct and safe approach. Changes from main: - Use unsigned_length in comparisons (not length) - Reorder surrogate handling before null terminator - Increase test timeout to 120s - Initialize and enlarge test buffers to 32 bytes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>	2025-11-03 07:12:38 +00:00
Jarred Sumner	0d0ced14d9	Update main.cpp	2025-11-02 22:53:11 -08:00
Claude Bot	19474111ba	Initialize and enlarge test buffers for WriteUtf8 - Initialize buffers to zero to avoid UBSAN issues with uninitialized memory - Increase buffer size from 16 to 32 bytes to ensure ample room for strings plus null terminator The test was using 16-byte buffers and passing sizeof(buf) to WriteUtf8, but this leaves no guaranteed space for the null terminator if the encoder uses the full buffer. With 32 bytes we have plenty of headroom. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>	2025-11-03 06:02:45 +00:00
Claude Bot	bdb8d9058d	Fix WriteUtf8 surrogate handling to avoid buffer overflow The unpaired surrogate encoding was happening AFTER incrementing written to include the null terminator. This meant the bounds check 'written + 3 <= unsigned_length' was using the wrong value, potentially allowing writes past the buffer boundary. This UB was caught by UBSAN in CI but not locally. By handling the surrogate encoding before adding the null terminator, the bounds check correctly uses the pre-increment value of written. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>	2025-11-03 05:13:02 +00:00
Claude Bot	12cc2a5b1f	Revert: WriteUtf8 should include null terminator in return value The previous commit incorrectly removed written++ after adding the null terminator. The V8 API actually DOES include the null terminator in the return count, as verified by Node.js behavior and all other string tests. The ASAN fix that resolved test_handle_scope_gc was actually the combination of: - Using view() with EnsureStillAliveScope (earlier commits) - Using unsigned_length in comparisons (previous commit) - Increasing test timeout to 120s All 34 v8 tests now pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>	2025-11-03 04:04:25 +00:00
Claude Bot	8d9f8ba804	Use unsigned_length in buffer bounds checks for clarity Changed comparisons from using the potentially negative signed 'length' parameter to using 'unsigned_length' in the null terminator and surrogate encoding bounds checks. This makes the code clearer and avoids any potential issues with signed/unsigned comparison semantics. No functional change since the values being compared (written) are already unsigned, but this is more explicit and maintainable. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>	2025-11-03 04:00:33 +00:00
Claude Bot	d9e205d1c7	Increase timeout for test_handle_scope_gc to 2 minutes The test was timing out at 10 seconds, but with ASAN/debug builds it needs more time. The test allocates 500 small strings and 50 large 20MB strings (1GB total), which is slow with ASAN overhead. Changed from 10s (or 60s for ASAN) to a flat 120s timeout since the isASAN check doesn't detect bun-debug builds. Test now passes in ~58 seconds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>	2025-11-02 12:46:16 +00:00
Claude Bot	c92da7cad4	Fix WriteUtf8 to not include null terminator in return value The V8 WriteUtf8 API returns the number of bytes written excluding the null terminator. We were incorrectly incrementing 'written' after adding the null byte, causing the return value to be off by one. This fixes the ASAN failure in test_handle_scope_gc where the assertion atoi(buf) == (int)j was failing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>	2025-11-02 12:41:20 +00:00
Jarred Sumner	a92b664bbe	Update V8String.cpp	2025-11-01 23:34:36 -07:00
Jarred Sumner	a69c6c0e61	Try this	2025-11-01 23:34:17 -07:00
Jarred Sumner	8da9f1b668	Update v8.test.ts	2025-11-01 23:14:05 -07:00
Jarred Sumner	ca07146df2	Update v8.test.ts	2025-11-01 22:10:42 -07:00
Jarred Sumner	e3d4e0475e	Try to deflake v8 test	2025-11-01 22:07:16 -07:00