From 6b80bea23085ae7d78f8255371f613ac3d670cdc Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 21:50:55 +0000 Subject: [PATCH 1/3] Handle 16-bit module sources in the error preview, the new-position fix-up and the REPL A module whose text contains any non-ASCII character (a preserved comment is enough) is held by JSC as a 16-bit string. Three readers assumed 8-bit: - ZigException.cpp only rendered the source preview for 8-bit sources, reading them through span8(); it now indexes the string, which works for both widths, so such modules get a preview again. - adjustPositionBackwards in ErrorStackFrame.cpp gave up on 16-bit sources and reset the frame to 1:1 whenever moving a constructor call's position back to its new keyword crossed a line; the offsets it works with are code units in either width, so the 8-bit-only check is removed. - Bun__REPL__evaluate decoded the program with WTF::String::fromUTF8, which returns a null string for ill-formed input; a preserved comment passes through the transpiler verbatim, so a stray byte in one made the whole input evaluate to nothing. It now uses the same decoder as the module loader, which substitutes U+FFFD. --- src/jsc/bindings/ErrorStackFrame.cpp | 18 +++----------- src/jsc/bindings/ZigException.cpp | 26 +++++++++---------- src/jsc/bindings/bindings.cpp | 7 +++++- test/bundler/bundler_compile.test.ts | 37 ++++++++++++++++++++++++++++ test/js/bun/repl/repl.test.ts | 25 +++++++++++++++++++ 5 files changed, 83 insertions(+), 30 deletions(-) diff --git a/src/jsc/bindings/ErrorStackFrame.cpp b/src/jsc/bindings/ErrorStackFrame.cpp index 1e7baf4c2055..d2b31d84e9de 100644 --- a/src/jsc/bindings/ErrorStackFrame.cpp +++ b/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,17 +32,6 @@ 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; - } for (int i = 0; i < amount; i++) { if (source[pos.byte_position - i] == '\n') { diff --git a/src/jsc/bindings/ZigException.cpp b/src/jsc/bindings/ZigException.cpp index cfd1059c2e78..d9bfa26715c4 100644 --- a/src/jsc/bindings/ZigException.cpp +++ b/src/jsc/bindings/ZigException.cpp @@ -148,7 +148,7 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr if (flags == PopulateStackTraceFlags::OnlyPosition) return; - if (source_lines_count > 1 && source_lines != nullptr && sourceString.is8Bit()) { + if (source_lines_count > 1 && source_lines != nullptr) { // Search for the beginning of the line unsigned int lineStart = location.byte_position; while (lineStart > 0 && sourceString[lineStart] != '\n') { @@ -162,8 +162,6 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr 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 +175,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; } } } diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index e08008192fa1..53edd4926bc7 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/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; diff --git a/test/bundler/bundler_compile.test.ts b/test/bundler/bundler_compile.test.ts index ac147a1d9dbd..1a707b5f2151 100644 --- a/test/bundler/bundler_compile.test.ts +++ b/test/bundler/bundler_compile.test.ts @@ -1360,6 +1360,43 @@ error: Hello World`, }, }, }); + // A module containing any non-ASCII text (here a preserved comment) is held + // by JSC as a 16-bit string. Without a source map the error preview is + // rendered from that string, and so is the fix-up that moves a constructor + // call's position back to its new keyword when the two are on different + // lines. Both used to give up on 16-bit sources: no preview, and a frame + // position of 1:1. The ASCII twin of this fixture has always printed the + // output asserted here. + 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, diff --git a/test/js/bun/repl/repl.test.ts b/test/js/bun/repl/repl.test.ts index 5947a8d30a3f..4569259515f6 100644 --- a/test/js/bun/repl/repl.test.ts +++ b/test/js/bun/repl/repl.test.ts @@ -431,6 +431,31 @@ 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. It has to be decoded + // the way module sources are (the bad byte becomes U+FFFD); decoding it with + // WTF::String::fromUTF8 turned the whole program into a null string, so + // nothing in the file was defined. + 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([ From a86543152e87aac0bf244b1d1db8dbbb8cff047e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 22:19:19 +0000 Subject: [PATCH 2/3] Do not read past the source when an error position is the end of the text The excerpt scan in populateStackFramePosition starts at the error's position, which is the length of the source when the expression it belongs to ends the text (an identifier that is not defined, for example). The scan then read one code unit past the source. Start it at the last code unit, and skip the excerpt for an empty source. adjustPositionBackwards gets the same bound for the same position. 8-bit sources had this read before. 16-bit sources reach it now that they get an excerpt. --- src/jsc/bindings/ErrorStackFrame.cpp | 5 ++++- src/jsc/bindings/ZigException.cpp | 10 ++++++---- test/bundler/bundler_compile.test.ts | 9 ++------- test/js/bun/repl/repl.test.ts | 5 +---- test/js/bun/util/inspect-error.test.js | 26 ++++++++++++++++++++++++++ 5 files changed, 39 insertions(+), 16 deletions(-) diff --git a/src/jsc/bindings/ErrorStackFrame.cpp b/src/jsc/bindings/ErrorStackFrame.cpp index d2b31d84e9de..e88b20d9732e 100644 --- a/src/jsc/bindings/ErrorStackFrame.cpp +++ b/src/jsc/bindings/ErrorStackFrame.cpp @@ -32,9 +32,12 @@ void adjustPositionBackwards(ZigStackFramePosition& pos, int amount, CodeBlock* } auto source = provider->source(); + 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; } } diff --git a/src/jsc/bindings/ZigException.cpp b/src/jsc/bindings/ZigException.cpp index d9bfa26715c4..bd3946236646 100644 --- a/src/jsc/bindings/ZigException.cpp +++ b/src/jsc/bindings/ZigException.cpp @@ -148,16 +148,18 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr if (flags == PopulateStackTraceFlags::OnlyPosition) return; - if (source_lines_count > 1 && source_lines != nullptr) { - // 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(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++; } diff --git a/test/bundler/bundler_compile.test.ts b/test/bundler/bundler_compile.test.ts index 1a707b5f2151..fc9694b2bc1c 100644 --- a/test/bundler/bundler_compile.test.ts +++ b/test/bundler/bundler_compile.test.ts @@ -1360,13 +1360,8 @@ error: Hello World`, }, }, }); - // A module containing any non-ASCII text (here a preserved comment) is held - // by JSC as a 16-bit string. Without a source map the error preview is - // rendered from that string, and so is the fix-up that moves a constructor - // call's position back to its new keyword when the two are on different - // lines. Both used to give up on 16-bit sources: no preview, and a frame - // position of 1:1. The ASCII twin of this fixture has always printed the - // output asserted here. + // 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, diff --git a/test/js/bun/repl/repl.test.ts b/test/js/bun/repl/repl.test.ts index 4569259515f6..4f3e5ca9b075 100644 --- a/test/js/bun/repl/repl.test.ts +++ b/test/js/bun/repl/repl.test.ts @@ -433,10 +433,7 @@ describe.concurrent("Bun REPL", () => { 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. It has to be decoded - // the way module sources are (the bad byte becomes U+FFFD); decoding it with - // WTF::String::fromUTF8 turned the whole program into a null string, so - // nothing in the file was defined. + // 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 /*! "), diff --git a/test/js/bun/util/inspect-error.test.js b/test/js/bun/util/inspect-error.test.js index 355ea17b2683..eff3aed5fb04 100644 --- a/test/js/bun/util/inspect-error.test.js +++ b/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); + }); +}); From aa48a4a056cfc65d227dff529a02be00c17e3f0f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 22:02:43 +0000 Subject: [PATCH 3/3] Build the coverage line table in UTF-16 code units JSC reports the offsets of executed and unexecuted blocks in code units of the source string it holds, but ByteRangeMapping looked them up in a table of byte offsets built from the UTF-8 form of that string, so line attribution drifted after any non-ASCII text in the module. Today a non-ASCII preserved comment is enough: every line after it is misattributed and functions that never ran are reported as covered. The table now stores each line start in code units (the byte offsets are kept as-is while the text is ASCII), and the per-row column tables the line offset table allocates, which were never used here, are freed instead of being kept for the life of the mapping. --- src/sourcemap_jsc/CodeCoverage.rs | 43 +++++++++++++--- test/cli/test/coverage.test.ts | 84 +++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 7 deletions(-) diff --git a/src/sourcemap_jsc/CodeCoverage.rs b/src/sourcemap_jsc/CodeCoverage.rs index 72308d3e3f87..3e617e2e577b 100644 --- a/src/sourcemap_jsc/CodeCoverage.rs +++ b/src/sourcemap_jsc/CodeCoverage.rs @@ -6,11 +6,10 @@ use std::borrow::Cow; use bun_ast::Loc; use bun_collections::VecExt; use bun_collections::bit_set::DynamicBitSet; -use bun_core::{self, Utf8Bytes}; +use bun_core::{self, Utf8Bytes, strings}; use bun_jsc::{JSGlobalObject, JSValue, VM, bun_string_jsc}; use bun_sourcemap::{ LineOffsetTable, LineOffsetTableColumns as _, Ordinal, ParsedSourceMap, internal_source_map, - line_offset_table, }; type LinesHits = Vec; @@ -641,10 +640,13 @@ pub struct BasicBlockRange { } pub struct ByteRangeMapping { - pub(crate) line_offset_table: line_offset_table::List, + /// Offset of the start of each line, in UTF-16 code units: the unit JSC + /// reports `BasicBlockRange` offsets in. Equal to the byte offset only + /// while the source text is pure ASCII. + pub(crate) line_starts: Box<[u32]>, /// JSC records coverage per `SourceProvider`, and each load of the file makes one. pub(crate) source_ids: Vec, - /// Of the text `line_offset_table` was built from. + /// Of the text `line_starts` was built from. source_hash: u64, pub source_url: Utf8Bytes<'static>, } @@ -710,7 +712,7 @@ impl ByteRangeMapping { ignore_sourcemap: bool, ) -> Result, bun_alloc::AllocError> { let source_url = self.source_url.slice(); - let line_starts = self.line_offset_table.items_byte_offset_to_start_of_line(); + let line_starts = &*self.line_starts; let mut executable_lines: Bitset; let mut lines_which_have_executed: Bitset; @@ -1037,9 +1039,36 @@ impl ByteRangeMapping { source_id: i32, source_url: Utf8Bytes<'static>, ) -> ByteRangeMapping { + let mut line_offset_table = LineOffsetTable::generate(source_contents, 0) + .unwrap_or_else(|_| bun_alloc::out_of_memory()); + let byte_starts = line_offset_table.items_byte_offset_to_start_of_line(); + + let line_starts: Box<[u32]> = if strings::is_all_ascii(source_contents) { + Box::from(byte_starts) + } else { + // `source_contents` is the UTF-8 form of the string JSC holds + // (8-bit or 16-bit), and JSC's offsets count that string's code + // units, so re-measure each line in those. + let mut previous_byte_start: usize = 0; + let mut code_units: u32 = 0; + byte_starts + .iter() + .map(|&byte_start| { + let byte_start = byte_start as usize; + let line = &source_contents[previous_byte_start..byte_start]; + code_units += u32::try_from(strings::element_length_utf8_into_utf16(line)) + .expect("int cast"); + previous_byte_start = byte_start; + code_units + }) + .collect() + }; + // `MultiArrayList`'s own `Drop` frees the slab only; this drops each + // row's `columns_for_non_ascii` box. + line_offset_table.drop_elements(); + ByteRangeMapping { - line_offset_table: LineOffsetTable::generate(source_contents, 0) - .unwrap_or_else(|_| bun_alloc::out_of_memory()), + line_starts, source_ids: vec![source_id], source_hash, source_url, diff --git a/test/cli/test/coverage.test.ts b/test/cli/test/coverage.test.ts index 3be7a44e6635..41015a470be3 100644 --- a/test/cli/test/coverage.test.ts +++ b/test/cli/test/coverage.test.ts @@ -1006,3 +1006,87 @@ test("b", () => { }); }); }); + +// JSC reports the offsets of executed and unexecuted blocks in UTF-16 code +// units of the source text it holds, and the line table those offsets are +// looked up in was built in bytes, so line attribution drifted after any +// non-ASCII text. Each non-ASCII variant below has exactly the same number of +// code units on every line as the ASCII twin (an astral character is two code +// units and four bytes), so its lcov record must be identical to the twin's; +// counting bytes or code points shifts it. +// +// Both lengths are in UTF-16 code units. +const coverageBannerUnits = 30; +const coverageTextUnits = 40; +function coverageDemo(banner: string, text: string) { + return `/*! ${banner} */ +export const raw = String.raw\`${text}\`; +export const re = /${text}/u; +export function covered() { + return raw.length + re.source.length; +} +export function uncovered() { + return 1; +} +export function alsoUncovered() { + return 2; +} +`; +} +const coverageDemoTest = ` +import { expect, test } from "bun:test"; +import { covered } from "./demo"; + +test("source text has the expected length", () => { + expect(covered()).toBe(${2 * coverageTextUnits}); +}); +`; +function repeatUnits(piece: string, units: number) { + return Buffer.alloc((Buffer.byteLength(piece) * units) / piece.length, piece).toString(); +} + +// lcov records of `demo.ts` for each variant, in order. +async function coverageRecords(variants: Record) { + using dir = tempDir( + "cov-non-ascii", + Object.fromEntries( + Object.entries(variants).flatMap(([name, source]) => [ + [`${name}/demo.ts`, source], + [`${name}/demo.test.ts`, coverageDemoTest], + ]), + ), + ); + // `await` here keeps `dir` alive until the processes have finished. + return await Promise.all( + Object.keys(variants).map(async variant => { + const cwd = path.join(String(dir), variant); + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "--coverage", "--coverage-reporter=lcov", "./demo.test.ts"], + cwd, + env: bunEnv, + stdout: "ignore", + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderr).toContain(" 1 pass\n"); + expect(exitCode).toBe(0); + const record = readFileSync(path.join(cwd, "coverage", "lcov.info"), "utf8") + .split("end_of_record") + .find(record => record.includes("\nSF:demo.ts\n")); + expect(record).toBeDefined(); + return record!; + }), + ); +} + +test("a non-ASCII preserved comment does not shift coverage lines", async () => { + const asciiText = repeatUnits("x", coverageTextUnits); + const [ascii, legalComment] = await coverageRecords({ + ascii: coverageDemo(repeatUnits("x", coverageBannerUnits), asciiText), + legalComment: coverageDemo(repeatUnits("©🐰", coverageBannerUnits), asciiText), + }); + // `uncovered` and `alsoUncovered` start on lines 7 and 10. + expect(ascii).toContain("\nDA:7,0\n"); + expect(ascii).toContain("\nDA:10,0\n"); + expect(legalComment).toBe(ascii); +});