Handle 16-bit module sources in the error preview, the new-position fix-up and the REPL

src/jsc/bindings/ErrorStackFrame.cpp

@@ -8,9 +8,10 @@
 namespace Bun {
 using namespace JSC;
 
-/// Adjust a `ZigStackFramePosition` by a number of bytes. This accounts for when the adjustment
-/// crosses line boundaries, and thus requires the source code in order to properly compute
-/// the result.
+/// Adjust a `ZigStackFramePosition` by a number of code units. This accounts for when the
+/// adjustment crosses line boundaries, and thus requires the source code in order to properly
+/// compute the result. Works on 8-bit and 16-bit sources alike: offsets from JSC are code units
+/// either way.
 void adjustPositionBackwards(ZigStackFramePosition& pos, int amount, CodeBlock* code)
 {
     if (pos.byte_position - amount < 0) {
@@ -31,20 +32,12 @@ void adjustPositionBackwards(ZigStackFramePosition& pos, int amount, CodeBlock*
         }
 
         auto source = provider->source();
-        if (!source.is8Bit()) {
-            // Debug-only assertion
-            // Bun does not yet use 16-bit sources anywhere. The transpiler ensures everything
-            // fit's into latin1 / 8-bit strings for on-average lower memory usage.
-            ASSERT_NOT_REACHED("16-bit source re-mapping is not implemented here.");
-
-            pos.line_zero_based = 0;
-            pos.column_zero_based = 0;
-            pos.byte_position = 0;
-            return;
-        }
+        unsigned length = source.length();
 
         for (int i = 0; i < amount; i++) {
-            if (source[pos.byte_position - i] == '\n') {
+            // The position is the end of the text when the callee ends there.
+            unsigned index = pos.byte_position - i;
+            if (index < length && source[index] == '\n') {
                 pos.line_zero_based = pos.line_zero_based - 1;
             }
         }

src/jsc/bindings/ZigException.cpp

@@ -148,22 +148,22 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr
     if (flags == PopulateStackTraceFlags::OnlyPosition)
         return;
 
-    if (source_lines_count > 1 && source_lines != nullptr && sourceString.is8Bit()) {
-        // Search for the beginning of the line
-        unsigned int lineStart = location.byte_position;
+    if (source_lines_count > 1 && source_lines != nullptr && !sourceString.isEmpty()) {
+        unsigned int maxSearch = sourceString.length();
+
+        // Search for the beginning of the line. The position is the end of the
+        // text when the expression it belongs to ends there.
+        unsigned int lineStart = std::min<unsigned int>(location.byte_position, maxSearch - 1);
         while (lineStart > 0 && sourceString[lineStart] != '\n') {
             lineStart--;
         }
 
         // Search for the end of the line
         unsigned int lineEnd = location.byte_position;
-        unsigned int maxSearch = sourceString.length();
         while (lineEnd < maxSearch && sourceString[lineEnd] != '\n') {
             lineEnd++;
         }
 
-        const unsigned char* bytes = sourceString.span8().data();
-
         // Most of the time, when you look at a stack trace, you want a couple lines above.
 
         // It is key to not clone this data because source code strings are large.
@@ -177,36 +177,34 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr
         source_line_numbers[0] = location.line();
 
         if (lineStart > 0) {
-            auto byte_offset_in_source_string = lineStart - 1;
+            auto offset_in_source_string = lineStart - 1;
             uint8_t source_line_i = 1;
             auto remaining_lines_to_grab = source_lines_count - 1;
 
             {
-                // This should probably be code points instead of newlines
-                while (byte_offset_in_source_string > 0 && bytes[byte_offset_in_source_string] != '\n') {
-                    byte_offset_in_source_string--;
+                while (offset_in_source_string > 0 && sourceString[offset_in_source_string] != '\n') {
+                    offset_in_source_string--;
                 }
 
-                byte_offset_in_source_string -= byte_offset_in_source_string > 0;
+                offset_in_source_string -= offset_in_source_string > 0;
             }
 
-            while (byte_offset_in_source_string > 0 && remaining_lines_to_grab > 0) {
-                unsigned int end_of_line_offset = byte_offset_in_source_string;
+            while (offset_in_source_string > 0 && remaining_lines_to_grab > 0) {
+                unsigned int end_of_line_offset = offset_in_source_string;
 
-                // This should probably be code points instead of newlines
-                while (byte_offset_in_source_string > 0 && bytes[byte_offset_in_source_string] != '\n') {
-                    byte_offset_in_source_string--;
+                while (offset_in_source_string > 0 && sourceString[offset_in_source_string] != '\n') {
+                    offset_in_source_string--;
                 }
 
                 // We are at the beginning of the line
-                source_lines[source_line_i] = Bun::toStringView(sourceString.substring(byte_offset_in_source_string, end_of_line_offset - byte_offset_in_source_string + 1));
+                source_lines[source_line_i] = Bun::toStringView(sourceString.substring(offset_in_source_string, end_of_line_offset - offset_in_source_string + 1));
 
                 source_line_numbers[source_line_i] = location.line().fromZeroBasedInt(location.line().zeroBasedInt() - source_line_i);
                 source_line_i++;
 
                 remaining_lines_to_grab--;
 
-                byte_offset_in_source_string -= byte_offset_in_source_string > 0;
+                offset_in_source_string -= offset_in_source_string > 0;
             }
         }
     }

src/jsc/bindings/bindings.cpp

@@ -6806,7 +6806,12 @@ extern "C" JSC::EncodedJSValue Bun__REPL__evaluate(
     auto& vm = JSC::getVM(globalObject);
     auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
 
-    WTF::String source = WTF::String::fromUTF8(std::span { sourcePtr, sourceLen });
+    // Same decoder as the module loader: text the transpiler passes through
+    // verbatim (preserved comments) can be ill-formed UTF-8, which becomes U+FFFD
+    // here; WTF::String::fromUTF8 returned a null string that evaluated to nothing.
+    WTF::String source = sourceLen > 0
+        ? Zig::convertUTF8ToString(std::span { sourcePtr, sourceLen })
+        : WTF::emptyString();
     WTF::String filename = filenameLen > 0
         ? WTF::String::fromUTF8(std::span { filenamePtr, filenameLen })
         : "[repl]"_s;

test/bundler/bundler_compile.test.ts

@@ -1360,6 +1360,38 @@ error: Hello World`,
       },
     },
   });
+  // The non-ASCII comment makes JSC hold the module as a 16-bit string, and the
+  // argument list below `new` makes the position fix-up read that string.
+  itBundled("compile/NoSourceMapNonAsciiSource", {
+    target: "bun",
+    compile: true,
+    files: {
+      "/entry.ts": /* js */ `
+        /*! © café 中 */
+        function code() {
+          throw new (class Boom extends Error {
+            constructor(message: string) {
+              super(message);
+            }
+          })("boom");
+        }
+        code();
+      `,
+    },
+    run: {
+      exitCode: 1,
+      validate({ stderr }) {
+        expect(stderr).toInclude("| /*! © café 中 */\n");
+        expect(stderr).toInclude(
+          `5 |   throw new class Boom extends Error {
+            ^
+error: boom
+`,
+        );
+        expect(stderr).toMatch(/at code \(.*:5:9\)\n/);
+      },
+    },
+  });
   itBundled("compile/SourceMapBigFile", {
     target: "bun",
     compile: true,

test/js/bun/repl/repl.test.ts

@@ -431,6 +431,28 @@ describe.concurrent("Bun REPL", () => {
       expect(exitCode).toBe(0);
     });
 
+    test(".load of a file with an ill-formed byte in a preserved comment still evaluates it", async () => {
+      // The transpiler passes `/*! */` comments through verbatim, so the program
+      // handed to the evaluator can contain ill-formed UTF-8.
+      using dir = tempDir("repl-load-ill-formed", {
+        "bad.js": Buffer.concat([
+          Buffer.from("function tagged() {\n  /*! "),
+          Buffer.from([0xe9]),
+          Buffer.from(" */\n}\nvar loadedWithComment = 1;\n"),
+        ]),
+      });
+      const filePath = path.join(String(dir), "bad.js");
+      const { outputs, stderr, exitCode } = await runRepl([
+        `.load ${filePath}`,
+        'JSON.stringify([loadedWithComment, tagged.toString().split("\\uFFFD").length])',
+        ".exit",
+      ]);
+      // `.load` echoes the value of the file's last statement.
+      expect(outputs).toEqual([`Loading ${filePath}...\n1`, '"[1,2]"']);
+      expect(stderr).toBe("");
+      expect(exitCode).toBe(0);
+    });
+
     test(".load with a nonexistent file shows the error and keeps running", async () => {
       // A relative path: Windows rejects a forward-slash absolute path with EINVAL.
       const { outputs, stderr, exitCode } = await runRepl([

test/js/bun/util/inspect-error.test.js

@@ -510,3 +510,29 @@ describe.concurrent("AggregateError whose errors cannot be walked", () => {
     expect(exitCode).toBe(1);
   });
 });
+
+// The position of `xyz is not defined` is the end of the identifier, which is
+// the end of the text here. `Malloc=1` lets ASAN see a read past the source.
+describe("source excerpt for an error at the end of the source text", () => {
+  test.concurrent.each([
+    ["8-bit", "xyz"],
+    ["16-bit", "中文"],
+  ])("%s source", async (_, source) => {
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "-e", `(0, eval)(${JSON.stringify(source)});`],
+      env: { ...bunEnv, NO_COLOR: "1", Malloc: "1" },
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect({ stdout, excerpt: stderr.split("\n").slice(0, 3) }).toEqual({
+      stdout: "",
+      excerpt: [
+        `1 | ${source}`,
+        Buffer.alloc(4 + source.length, " ").toString() + "^",
+        `ReferenceError: ${source} is not defined`,
+      ],
+    });
+    expect(exitCode).toBe(1);
+  });
+});