Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5626,7 +5626,9 @@ impl VirtualMachine {
let top_source_url = frames[top].source_url.to_utf8();

let already_remapped = frames[top].remapped;
let maybe_lookup: Option<bun_sourcemap::mapping::Lookup> = if already_remapped {
let maybe_lookup: Option<bun_sourcemap::mapping::Lookup> = if frames[top].is_node_vm {
None
} else if already_remapped {
Some(bun_sourcemap::mapping::Lookup {
mapping: bun_sourcemap::mapping::Mapping {
generated: bun_sourcemap::LineColumnOffset::default(),
Expand Down Expand Up @@ -5775,7 +5777,7 @@ impl VirtualMachine {

if frames.len() > 1 {
for i in 0..frames.len() {
if i == top || frames[i].position.is_invalid() {
if i == top || frames[i].position.is_invalid() || frames[i].is_node_vm {
continue;
}
let source_url = frames[i].source_url.to_utf8();
Expand Down
14 changes: 14 additions & 0 deletions src/jsc/ZigStackFrame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,23 @@ pub struct ZigStackFrame {
/// This informs formatters whether to display as a blob URL or not
pub remapped: bool,

/// Set by C++ (`Zig::isNodeVMSource`) for code compiled by `node:vm`. Its
/// `source_url` is whatever filename the caller passed, so it must never be
/// looked up in the source map table even when it names a file Bun transpiled.
pub is_node_vm: bool,

/// -1 means not set.
pub jsc_stack_frame_index: i32,
}

// Mirrors `struct ZigStackFrame` in `src/jsc/bindings/headers-handwritten.h`,
// which C++ populates in place.
bun_core::assert_ffi_layout!(
ZigStackFrame, 72, 8;
function_name @ 0, source_url @ 24, position @ 48, code_type @ 60, is_async @ 61,
remapped @ 62, is_node_vm @ 63, jsc_stack_frame_index @ 64,
);

impl ZigStackFrame {
/// Explicit deref of owned strings.
///
Expand Down Expand Up @@ -72,6 +85,7 @@ impl ZigStackFrame {
position: ZigStackFramePosition::INVALID,
is_async: false,
remapped: false,
is_node_vm: false,
jsc_stack_frame_index: -1,
};

Expand Down
20 changes: 20 additions & 0 deletions src/jsc/bindings/ErrorStackTrace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,26 @@ String sourceURL(JSC::VM& vm, JSC::JSFunction* function)
return Zig::sourceURL(function->jsExecutable()->source());
}

bool isNodeVMSource(JSC::SourceProvider* sourceProvider)
{
if (!sourceProvider) [[unlikely]] {
return false;
}

auto* fetcher = sourceProvider->sourceOrigin().fetcher();
return fetcher && fetcher->fetcherType() == JSC::ScriptFetcher::Type::NodeVM;
}

bool isNodeVMSource(const JSC::StackFrame& frame)
{
auto* codeBlock = frame.codeBlock();
if (!codeBlock || !codeBlock->ownerExecutable()) {
return false;
}

return isNodeVMSource(codeBlock->source().provider());
}

String functionName(JSC::VM& vm, JSC::CodeBlock* codeBlock)
{
auto codeType = codeBlock->codeType();
Expand Down
8 changes: 8 additions & 0 deletions src/jsc/bindings/ErrorStackTrace.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,14 @@ String sourceURL(JSC::VM& vm, const JSC::StackFrame& frame);
String sourceURL(JSC::StackVisitor& visitor);
String sourceURL(JSC::VM& vm, JSC::JSFunction* function);

// True for code compiled by node:vm (every node:vm compile path attaches a
// NodeVMScriptFetcher to its SourceOrigin). node:vm compiles the string it was
// given as-is under a caller-chosen filename, while Bun's source maps are keyed
// by filename, so positions in such code must never be remapped, even when the
// filename is a file Bun transpiled.
bool isNodeVMSource(JSC::SourceProvider* sourceProvider);
bool isNodeVMSource(const JSC::StackFrame& frame);

enum class FinalizerSafety {
NotInFinalizer,
MustNotTriggerGC,
Expand Down
63 changes: 17 additions & 46 deletions src/jsc/bindings/FormatStackTraceForJS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ WTF::String formatStackTrace(
StackFrame& frame = stackTrace.at(i);
ZigStackFrame& remappedFrame = remappedFrames[i];
// Match `ZigStackFramePosition::INVALID` exactly so the Rust batch loop's
// `position.isInvalid()` skips frames we never populate (vm-context
// `position.isInvalid()` skips frames we never populate (node:vm
// frames, frames without line/col info). memset alone leaves
// `line_start_byte = 0` which fails that byte-compare.
remappedFrame.position.line_zero_based = -1;
Expand All @@ -265,26 +265,14 @@ WTF::String formatStackTrace(
if (!frame.hasLineAndColumnInfo()) continue;

originalLineColumns[i] = frame.computeLineAndColumn();

JSC::JSGlobalObject* globalObjectForFrame = lexicalGlobalObject;
if (auto* callee = frame.callee()) {
if (auto* object = callee->getObject()) {
globalObjectForFrame = object->globalObject();
}
}

sourceURLs[i] = Zig::sourceURL(vm, frame);

bool isDefinitelyNotRunninginNodeVMGlobalObject = globalObject == globalObjectForFrame;
bool isDefaultGlobalObjectInAFinalizer = (globalObject && !lexicalGlobalObject && !errorInstance);
if (isDefinitelyNotRunninginNodeVMGlobalObject || isDefaultGlobalObjectInAFinalizer) {
// https://github.com/oven-sh/bun/issues/3595
if (!sourceURLs[i].isEmpty()) {
remappedFrame.position.line_zero_based = OrdinalNumber::fromOneBasedInt(originalLineColumns[i].line).zeroBasedInt();
remappedFrame.position.column_zero_based = OrdinalNumber::fromOneBasedInt(originalLineColumns[i].column).zeroBasedInt();
remappedFrame.source_url = Bun::toStringRef(sourceURLs[i]);
anyRemap = true;
}
// https://github.com/oven-sh/bun/issues/3595
if (!sourceURLs[i].isEmpty() && !Zig::isNodeVMSource(frame)) {
remappedFrame.position.line_zero_based = OrdinalNumber::fromOneBasedInt(originalLineColumns[i].line).zeroBasedInt();
remappedFrame.position.column_zero_based = OrdinalNumber::fromOneBasedInt(originalLineColumns[i].column).zeroBasedInt();
remappedFrame.source_url = Bun::toStringRef(sourceURLs[i]);
anyRemap = true;
}
}

Expand Down Expand Up @@ -440,8 +428,6 @@ static JSValue computeErrorInfoWithPrepareStackTrace(JSC::VM& vm, Zig::GlobalObj
// Create the call sites (one per frame)
Zig::createCallSitesFromFrames(globalObject, lexicalGlobalObject, stackTrace, callSites);

// We need to sourcemap it if it's a GlobalObject.

const int n = stackTrace.size();
WTF::Vector<ZigStackFrame, 8> remappedFrames;
WTF::Vector<WTF::String, 8> sourceURLs;
Expand All @@ -461,33 +447,18 @@ static JSValue computeErrorInfoWithPrepareStackTrace(JSC::VM& vm, Zig::GlobalObj
frame.position.column_zero_based = -1;
frame.position.byte_position = -1;

// When you use node:vm, the global object can be different on a
// per-frame basis. We should sourcemap the frames which are in Bun's
// global object, and not sourcemap the frames which are in a different
// global object.
JSGlobalObject* globalObjectForFrame = lexicalGlobalObject;
if (Zig::isNodeVMSource(stackFrame))
continue;

if (stackFrame.hasLineAndColumnInfo()) {
auto* callee = stackFrame.callee();
// https://github.com/oven-sh/bun/issues/17698
if (callee) {
if (auto* object = callee->getObject()) {
globalObjectForFrame = object->globalObject();
}
}
if (JSCStackFrame::SourcePositions* sourcePositions = stackTrace.at(i).getSourcePositions()) {
frame.position.line_zero_based = sourcePositions->line.zeroBasedInt();
frame.position.column_zero_based = sourcePositions->column.zeroBasedInt();
}

if (globalObjectForFrame == globalObject) {
if (JSCStackFrame::SourcePositions* sourcePositions = stackTrace.at(i).getSourcePositions()) {
frame.position.line_zero_based = sourcePositions->line.zeroBasedInt();
frame.position.column_zero_based = sourcePositions->column.zeroBasedInt();
}

if (!sourceURLs[i].isEmpty()) {
frame.source_url = Bun::toStringRef(sourceURLs[i]);
didRemap[i] = true;
anyRemap = true;
}
if (!sourceURLs[i].isEmpty()) {
frame.source_url = Bun::toStringRef(sourceURLs[i]);
didRemap[i] = true;
anyRemap = true;
}
}

Expand Down Expand Up @@ -616,7 +587,7 @@ WTF::String computeErrorInfoWrapperToString(JSC::VM& vm, Vector<StackFrame>& sta
void computeLineColumnWithSourcemap(JSC::VM& vm, JSC::SourceProvider* _Nonnull sourceProvider, JSC::LineColumn& lineColumn, WTF::String& remappedSourceURL)
{
auto sourceURL = sourceProvider->sourceURL();
if (sourceURL.isEmpty()) {
if (sourceURL.isEmpty() || Zig::isNodeVMSource(sourceProvider)) {
return;
}

Expand Down
1 change: 1 addition & 0 deletions src/jsc/bindings/ZigException.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ static void populateStackFrameMetadata(JSC::VM& vm, JSC::JSGlobalObject* globalO

auto sourceURL = Zig::sourceURL(vm, stackFrame);
frame.source_url = Bun::toStringRef(sourceURL);
frame.is_node_vm = Zig::isNodeVMSource(stackFrame);
auto m_codeBlock = stackFrame.codeBlock();
if (m_codeBlock) {
switch (m_codeBlock->codeType()) {
Expand Down
3 changes: 3 additions & 0 deletions src/jsc/bindings/headers-handwritten.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@ typedef struct ZigStackFrame {
ZigStackFrameCode code_type;
bool is_async;
bool remapped;
// See Zig::isNodeVMSource(). The error printer skips source maps for these.
bool is_node_vm;
int32_t jsc_stack_frame_index;

ZigStackFrame()
Expand All @@ -208,6 +210,7 @@ typedef struct ZigStackFrame {
, code_type {}
, is_async(false)
, remapped(false)
, is_node_vm(false)
, jsc_stack_frame_index(-1)
{
}
Expand Down
1 change: 1 addition & 0 deletions src/runtime/bake/dev_server/error_report_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ impl ErrorReportRequest {
code_type: ZigStackFrameCode::NONE,
is_async: false,
remapped: false,
is_node_vm: false,
jsc_stack_frame_index: -1,
});
}
Expand Down
52 changes: 52 additions & 0 deletions test/cli/run/cpu-prof.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test";
import { readdirSync, readFileSync } from "fs";
import { bunEnv, bunExe, isWindows, tempDir } from "harness";
import { join } from "path";
import { pathToFileURL } from "url";

// Every workload below is time-bounded for 100ms. On Windows JSC's
// SamplingProfiler effectively ticks at the ~15.6ms default timer quantum, and
Expand Down Expand Up @@ -444,4 +445,55 @@ describe.concurrent("--cpu-prof", () => {
const mdContent = readFileSync(join(String(dir), mdFiles[0]), "utf-8");
expect(mdContent).toContain("# CPU Profile");
});

test("vm code compiled under the filename of the profiled file keeps its own positions", async () => {
// Source maps are keyed by filename. `f` is compiled by node:vm on line 5 of
// its source, under the fixture's own filename ("own") or under a name Bun
// never loaded ("other"); both must report the same, physical position. The
// PADDING-line comment is stripped by the transpiler, so any line the
// fixture's source map could produce is > PADDING and never mistaken for 5.
const PADDING = 20;
const padding = ["/*", ...Array.from({ length: PADDING - 2 }, () => " *"), " */"].join("\n") + "\n";
const body = String.raw`
const vm = require("node:vm");
const name = process.argv[2] === "own" ? __filename : "not-a-loaded-file.js";
const source = "\n\n\n\n(function f() { for (const end = performance.now() + 100; performance.now() < end; ) {} })";
const f = vm.runInThisContext(source, { filename: name });
f();
console.log(name);
`;
using dir = tempDir("cpu-prof-vm-filename", { "fixture.js": padding + body });

const profileFor = async (kind: string) => {
await using proc = Bun.spawn({
cmd: [bunExe(), "--cpu-prof", `--cpu-prof-name=${kind}.cpuprofile`, "fixture.js", kind],
cwd: String(dir),
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stderr, exitCode }).toEqual({ stderr: "", exitCode: 0 });
const profile = JSON.parse(readFileSync(join(String(dir), `${kind}.cpuprofile`), "utf8"));
const fNodes = profile.nodes.filter((n: any) => n.callFrame.functionName === "f");
expect(fNodes.length).toBeGreaterThan(0);
return {
name: stdout.trim(),
callFrames: [
...new Set(
fNodes.map((n: any) => JSON.stringify([n.callFrame.url, n.callFrame.lineNumber, n.callFrame.columnNumber])),
),
].map(s => JSON.parse(s as string)),
tickLines: new Set(fNodes.flatMap((n: any) => (n.positionTicks ?? []).map((t: any) => t.line))),
};
};
const [own, other] = await Promise.all([profileFor("own"), profileFor("other")]);

// callFrame.lineNumber is 0-based; positionTicks.line is 1-based.
expect(other.callFrames).toEqual([["not-a-loaded-file.js", 4, expect.any(Number)]]);
const [[, line, column]] = other.callFrames;
expect(own.callFrames).toEqual([[pathToFileURL(own.name).href, line, column]]);
expect(other.tickLines).toEqual(new Set([5]));
expect(own.tickLines).toEqual(new Set([5]));
});
});
Loading