diff --git a/src/jsc/bindings/CallSite.cpp b/src/jsc/bindings/CallSite.cpp index dbfee33bc406..42f201c32d20 100644 --- a/src/jsc/bindings/CallSite.cpp +++ b/src/jsc/bindings/CallSite.cpp @@ -20,7 +20,7 @@ namespace Zig { const JSC::ClassInfo CallSite::s_info = { "CallSite"_s, &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(CallSite) }; -void CallSite::finishCreation(VM& vm, JSC::JSGlobalObject* globalObject, JSCStackFrame& stackFrame, bool encounteredStrictFrame) +void CallSite::finishCreation(VM& vm, JSCStackFrame& stackFrame, bool encounteredStrictFrame) { Base::finishCreation(vm); @@ -39,20 +39,12 @@ void CallSite::finishCreation(VM& vm, JSC::JSGlobalObject* globalObject, JSCStac } } - // Initialize "this" and "function" (and set the "IsStrict" flag if needed) - JSC::CallFrame* callFrame = stackFrame.callFrame(); + // JSC::StackFrame has no receiver, so getThis() is always undefined. + m_thisValue.set(vm, this, JSC::jsUndefined()); if (isStrictFrame) { - m_thisValue.set(vm, this, JSC::jsUndefined()); m_function.set(vm, this, JSC::jsUndefined()); m_flags |= static_cast(Flags::IsStrict); } else { - if (callFrame && callFrame->thisValue()) { - // We know that we're not in strict mode - m_thisValue.set(vm, this, callFrame->thisValue().toThis(globalObject, JSC::ECMAMode::sloppy())); - } else { - m_thisValue.set(vm, this, JSC::jsUndefined()); - } - m_function.set(vm, this, stackFrame.callee()); } diff --git a/src/jsc/bindings/CallSite.h b/src/jsc/bindings/CallSite.h index 2920e9fe0bce..d4086e59976c 100644 --- a/src/jsc/bindings/CallSite.h +++ b/src/jsc/bindings/CallSite.h @@ -48,7 +48,7 @@ class CallSite final : public JSC::JSNonFinalObject { { auto& vm = JSC::getVM(globalObject); CallSite* callSite = new (NotNull, JSC::allocateCell(vm)) CallSite(vm, structure); - callSite->finishCreation(vm, globalObject, stackFrame, encounteredStrictFrame); + callSite->finishCreation(vm, stackFrame, encounteredStrictFrame); return callSite; } @@ -101,7 +101,7 @@ class CallSite final : public JSC::JSNonFinalObject { { } - void finishCreation(VM& vm, JSC::JSGlobalObject* globalObject, JSCStackFrame& stackFrame, bool encounteredStrictFrame); + void finishCreation(VM& vm, JSCStackFrame& stackFrame, bool encounteredStrictFrame); DECLARE_VISIT_CHILDREN; }; diff --git a/src/jsc/bindings/ErrorStackTrace.cpp b/src/jsc/bindings/ErrorStackTrace.cpp index 7afd14d4b236..18308c3ddb8b 100644 --- a/src/jsc/bindings/ErrorStackTrace.cpp +++ b/src/jsc/bindings/ErrorStackTrace.cpp @@ -182,27 +182,6 @@ void JSCStackTrace::getFramesForCaller(JSC::VM& vm, JSC::CallFrame* callFrame, J stackTrace.shrink(stackTraceLimit); } -JSCStackTrace JSCStackTrace::getStackTraceForThrownValue(JSC::VM& vm, JSC::JSValue thrownValue) -{ - const WTF::Vector* jscStackTrace = nullptr; - - JSC::Exception* currentException = DECLARE_TOP_EXCEPTION_SCOPE(vm).exception(); - if (currentException && currentException->value() == thrownValue) { - jscStackTrace = ¤tException->stack(); - } else { - JSC::ErrorInstance* error = dynamicDowncast(thrownValue); - if (error) { - jscStackTrace = error->stackTrace(); - } - } - - if (!jscStackTrace) { - return JSCStackTrace(); - } - - return fromExisting(vm, *jscStackTrace); -} - static bool isVisibleBuiltinFunction(JSC::CodeBlock* codeBlock) { if (!codeBlock->ownerExecutable()) { @@ -213,57 +192,9 @@ static bool isVisibleBuiltinFunction(JSC::CodeBlock* codeBlock) return !Zig::sourceURL(source).isEmpty(); } -JSCStackFrame::JSCStackFrame(JSC::VM& vm, JSC::StackVisitor& visitor) - : m_vm(vm) - , m_codeBlock(nullptr) - , m_bytecodeIndex(JSC::BytecodeIndex()) - , m_sourceURL() - , m_functionName() - , m_isWasmFrame(false) - , m_isAsync(false) - , m_sourcePositionsState(SourcePositionsState::NotCalculated) -{ - m_callee = visitor->callee().asCell(); - m_callFrame = visitor->callFrame(); - - if (auto* codeBlock = visitor->codeBlock()) { - auto codeType = codeBlock->codeType(); - if (codeType == JSC::FunctionCode || codeType == JSC::EvalCode) { - m_isFunctionOrEval = true; - } - } - - // Based on JSC's GetStackTraceFunctor (Interpreter.cpp) - if (visitor->isNativeCalleeFrame()) { - auto* nativeCallee = visitor->callee().asNativeCallee(); - switch (nativeCallee->category()) { - case NativeCallee::Category::Wasm: { - m_wasmFunctionIndexOrName = visitor->wasmFunctionIndexOrName(); - m_isWasmFrame = true; - break; - } - case NativeCallee::Category::InlineCache: { - break; - } - } - } else if (auto* codeBlock = visitor->codeBlock()) { - auto* unlinkedCodeBlock = codeBlock->unlinkedCodeBlock(); - if (!unlinkedCodeBlock->isBuiltinFunction() || isVisibleBuiltinFunction(codeBlock)) { - m_codeBlock = codeBlock; - m_bytecodeIndex = visitor->bytecodeIndex(); - } - } - - if (!m_bytecodeIndex && visitor->hasLineAndColumnInfo()) { - auto lineColumn = visitor->computeLineAndColumn(); - m_sourcePositions = { OrdinalNumber::fromOneBasedInt(lineColumn.line), OrdinalNumber::fromOneBasedInt(lineColumn.column) }; - m_sourcePositionsState = SourcePositionsState::Calculated; - } -} - JSCStackFrame::JSCStackFrame(JSC::VM& vm, const JSC::StackFrame& frame) : m_vm(vm) - , m_callFrame(nullptr) + , m_stackFrame(&frame) , m_codeBlock(nullptr) , m_bytecodeIndex(JSC::BytecodeIndex()) , m_sourceURL() diff --git a/src/jsc/bindings/ErrorStackTrace.h b/src/jsc/bindings/ErrorStackTrace.h index 17c9dc6822ea..6a30a99bd293 100644 --- a/src/jsc/bindings/ErrorStackTrace.h +++ b/src/jsc/bindings/ErrorStackTrace.h @@ -16,8 +16,7 @@ using namespace WebCore; namespace Zig { -/* JSCStackFrame is an alternative to JSC::StackFrame, which provides the following advantages\changes: - * - Also hold the call frame (ExecState). This is mainly used by CallSite to get "this value". +/* JSCStackFrame is a view over a JSC::StackFrame, which provides the following advantages\changes: * - More detailed and v8 compatible "source offsets" calculations: JSC::StackFrame only provides the * line number and column numbers. It's column calculation seems to be different than v8's column. * According to v8's unit tests, it seems that their column number points to the beginning of @@ -46,11 +45,10 @@ class JSCStackFrame { private: JSC::VM& m_vm; + // Points into the vector this frame was built from (JSCStackTrace::fromExisting). + const JSC::StackFrame* m_stackFrame; JSC::JSCell* m_callee { nullptr }; - // May be null - JSC::CallFrame* m_callFrame; - // May be null JSC::CodeBlock* m_codeBlock { nullptr }; JSC::BytecodeIndex m_bytecodeIndex; @@ -77,11 +75,10 @@ class JSCStackFrame { SourcePositionsState m_sourcePositionsState; public: - JSCStackFrame(JSC::VM& vm, JSC::StackVisitor& visitor); JSCStackFrame(JSC::VM& vm, const JSC::StackFrame& frame); + const JSC::StackFrame& stackFrame() const { return *m_stackFrame; } JSC::JSCell* callee() const { return m_callee; } - JSC::CallFrame* callFrame() const { return m_callFrame; } JSC::CodeBlock* codeBlock() const { return m_codeBlock; } intptr_t sourceID() const; @@ -179,28 +176,11 @@ class JSCStackTrace { WTF::Vector&& frames() { return WTF::move(m_frames); } + // Drops private-visibility frames (present when Options::showPrivateScriptsInStackTraces() is on), so the result does not index like existingFrames. static JSCStackTrace fromExisting(JSC::VM& vm, const WTF::Vector& existingFrames); static void getFramesForCaller(JSC::VM& vm, JSC::CallFrame* callFrame, JSC::JSCell* owner, JSC::JSValue caller, WTF::Vector& stackTrace, size_t stackTraceLimit); - /* In JSC, JSC::Exception points to the actual value that was thrown, usually - * a JSC::ErrorInstance (but could be any JSValue). In v8, on the other hand, - * TryCatch::Exception returns the thrown value, and we follow the same rule in jscshim. - * This is a problem, since JSC::Exception is the one that holds the stack trace. - * ErrorInstances might also hold the stack trace (until the error properties are - * "materialized" and it is no longer needed). So, to try to get the stack trace for a thrown JSValue, - * we'll try two things: - * - If the current JSC (vm) exception points to our value, it means our value is probably the current - * exception and we could take the stack trace from the vm's current JSC::Exception. The downside - * of doing this is that we'll get the last stack trace of the thrown value, meaning that if the value - * was thrown, stored in the api and than re-thrown, we'll get the latest stack trace and not the one - * that was available when we stored it. For now it'll do. - * - If that failed and our thrown value is a JSC::ErrorInstance, we'll try to use it's stack trace, - * if it currently has one. - * - * Return value must remain stack allocated */ - static JSCStackTrace getStackTraceForThrownValue(JSC::VM& vm, JSC::JSValue thrownValue); - private: JSCStackTrace(WTF::Vector& frames) : m_frames(WTF::move(frames)) diff --git a/src/jsc/bindings/FormatStackTraceForJS.cpp b/src/jsc/bindings/FormatStackTraceForJS.cpp index d622679f31aa..ccbfc31bef99 100644 --- a/src/jsc/bindings/FormatStackTraceForJS.cpp +++ b/src/jsc/bindings/FormatStackTraceForJS.cpp @@ -454,7 +454,8 @@ static JSValue computeErrorInfoWithPrepareStackTrace(JSC::VM& vm, Zig::GlobalObj for (int i = 0; i < n; i++) { ZigStackFrame& frame = remappedFrames[i]; - auto& stackFrame = stackFrames.at(i); + JSCStackFrame& visibleFrame = stackTrace.at(i); + const JSC::StackFrame& stackFrame = visibleFrame.stackFrame(); sourceURLs[i] = Zig::sourceURL(vm, stackFrame); didRemap[i] = false; frame.position.line_zero_based = -1; @@ -478,7 +479,7 @@ static JSValue computeErrorInfoWithPrepareStackTrace(JSC::VM& vm, Zig::GlobalObj } if (globalObjectForFrame == globalObject) { - if (JSCStackFrame::SourcePositions* sourcePositions = stackTrace.at(i).getSourcePositions()) { + if (JSCStackFrame::SourcePositions* sourcePositions = visibleFrame.getSourcePositions()) { frame.position.line_zero_based = sourcePositions->line.zeroBasedInt(); frame.position.column_zero_based = sourcePositions->column.zeroBasedInt(); } diff --git a/test/js/node/v8/capture-stack-trace.test.js b/test/js/node/v8/capture-stack-trace.test.js index 94e4bef75b63..21b1ddd39fbd 100644 --- a/test/js/node/v8/capture-stack-trace.test.js +++ b/test/js/node/v8/capture-stack-trace.test.js @@ -1,7 +1,7 @@ import { nativeFrameForTesting } from "bun:internal-for-testing"; import { noInline } from "bun:jsc"; import { afterEach, expect, mock, test } from "bun:test"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, tempDir } from "harness"; const origPrepareStackTrace = Error.prepareStackTrace; afterEach(() => { Error.prepareStackTrace = origPrepareStackTrace; @@ -1121,3 +1121,76 @@ test("lazy error-info materialization does not store an empty stack value when t }); expect(exitCode).toBe(0); }); + +test("Error.prepareStackTrace call sites keep their own file when a hidden frame is on the stack", async () => { + // A bound function call is a frame with private implementation visibility. JSC omits it from the + // trace unless showPrivateScriptsInStackTraces is on (debug builds turn it on); Bun then has to + // drop it while building the CallSites, and the frames after it must keep their own file. + using dir = tempDir("prepare-stack-trace-hidden-frame", { + "main.cjs": [ + `const { outer, viaAsyncLocalStorage } = require("./callers.cjs");`, + `function inner() {`, + ` const error = new Error("boom");`, + ` return error;`, + `}`, + `function main() {`, + ` const error = outer(inner.bind(null));`, + ` return error;`, + `}`, + `const { basename } = require("node:path");`, + `const wanted = new Set(["inner", "outer", "main", "run", "viaAsyncLocalStorage"]);`, + `Error.prepareStackTrace = (_error, callSites) =>`, + ` callSites`, + ` .filter(callSite => wanted.has(callSite.getFunctionName()))`, + ` .map(callSite => ({`, + ` name: callSite.getFunctionName(),`, + ` file: basename(callSite.getFileName()),`, + ` line: callSite.getLineNumber(),`, + ` }));`, + `console.log(JSON.stringify({ bound: main().stack, asyncLocalStorage: viaAsyncLocalStorage(inner.bind(null)).stack }));`, + ].join("\n"), + "callers.cjs": [ + `const { AsyncLocalStorage } = require("node:async_hooks");`, + `exports.outer = function outer(callback) {`, + ` const error = callback();`, + ` return error;`, + `};`, + `const storage = new AsyncLocalStorage();`, + `exports.viaAsyncLocalStorage = function viaAsyncLocalStorage(callback) {`, + ` const error = storage.run("store", callback);`, + ` return error;`, + `};`, + ].join("\n"), + }); + + const results = await Promise.all( + ["0", "1"].map(async showPrivateScriptsInStackTraces => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.cjs"], + cwd: String(dir), + env: { ...bunEnv, BUN_JSC_showPrivateScriptsInStackTraces: showPrivateScriptsInStackTraces }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { showPrivateScriptsInStackTraces, callSites: stdout && JSON.parse(stdout), stderr, exitCode }; + }), + ); + + const expected = { + bound: [ + { name: "inner", file: "main.cjs", line: 3 }, + { name: "outer", file: "callers.cjs", line: 3 }, + { name: "main", file: "main.cjs", line: 7 }, + ], + asyncLocalStorage: [ + { name: "inner", file: "main.cjs", line: 3 }, + { name: "run", file: "node:async_hooks", line: expect.any(Number) }, + { name: "viaAsyncLocalStorage", file: "callers.cjs", line: 8 }, + ], + }; + expect(results).toEqual([ + { showPrivateScriptsInStackTraces: "0", callSites: expected, stderr: "", exitCode: 0 }, + { showPrivateScriptsInStackTraces: "1", callSites: expected, stderr: "", exitCode: 0 }, + ]); +});