Skip to content

Commit

Permalink
Fix another crash in Error.captureStackTrace (#3611)
Browse files Browse the repository at this point in the history
Co-authored-by: Jarred Sumner <[email protected]>
  • Loading branch information
Jarred-Sumner and Jarred-Sumner authored Jul 12, 2023
1 parent 666feb3 commit b566573
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 20 deletions.
12 changes: 12 additions & 0 deletions bench/snippets/error-capturestack.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { bench, run } from "./runner.mjs";

var err = new Error();
bench("Error.captureStackTrace(err)", () => {
Error.captureStackTrace(err);
});

bench("Error.prototype.stack", () => {
new Error().stack;
});

await run();
28 changes: 9 additions & 19 deletions src/bun.js/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@ static String computeErrorInfoWithoutPrepareStackTrace(JSC::VM& vm, Vector<Stack

size_t framesCount = stackTrace.size();
ZigStackFrame remappedFrames[framesCount];

bool hasSet = false;
for (size_t i = 0; i < framesCount; i++) {
StackFrame& frame = stackTrace.at(i);
Expand Down Expand Up @@ -418,6 +419,8 @@ static String computeErrorInfoWithoutPrepareStackTrace(JSC::VM& vm, Vector<Stack
unsigned int thisLine = 0;
unsigned int thisColumn = 0;
frame.computeLineAndColumn(thisLine, thisColumn);
memset(remappedFrames + i, 0, sizeof(ZigStackFrame));

remappedFrames[i].position.line = thisLine;
remappedFrames[i].position.column_start = thisColumn;
String sourceURLForFrame = frame.sourceURL(vm);
Expand Down Expand Up @@ -2680,7 +2683,7 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionPerformMicrotaskVariadic, (JSGlobalObject * g
return JSValue::encode(jsUndefined());
}

void GlobalObject::createCallSitesFromFrames(JSC::JSGlobalObject* lexicalGlobalObject, JSC::ObjectInitializationScope& objectScope, JSCStackTrace& stackTrace, JSC::JSArray* callSites)
void GlobalObject::createCallSitesFromFrames(JSC::JSGlobalObject* lexicalGlobalObject, JSCStackTrace& stackTrace, JSC::JSArray* callSites)
{
/* From v8's "Stack Trace API" (https://github.com/v8/v8/wiki/Stack-Trace-API):
* "To maintain restrictions imposed on strict mode functions, frames that have a
Expand All @@ -2691,16 +2694,10 @@ void GlobalObject::createCallSitesFromFrames(JSC::JSGlobalObject* lexicalGlobalO
GlobalObject* globalObject = reinterpret_cast<GlobalObject*>(lexicalGlobalObject);

JSC::Structure* callSiteStructure = globalObject->callSiteStructure();
JSC::IndexingType callSitesIndexingType = callSites->indexingType();
size_t framesCount = stackTrace.size();
for (size_t i = 0; i < framesCount; i++) {
/* Note that we're using initializeIndex and not callSites->butterfly()->contiguous().data()
* directly, since if we're "having a bad time" (globalObject->isHavingABadTime()),
* the array won't be contiguous, but a "slow put" array.
* See https://github.com/WebKit/webkit/commit/1c4a32c94c1f6c6aa35cf04a2b40c8fe29754b8e for more info
* about what's a "bad time". */
CallSite* callSite = CallSite::create(lexicalGlobalObject, callSiteStructure, stackTrace.at(i), encounteredStrictFrame);
callSites->initializeIndex(objectScope, i, callSite, callSitesIndexingType);
callSites->putDirectIndex(lexicalGlobalObject, i, callSite);

if (!encounteredStrictFrame) {
encounteredStrictFrame = callSite->isStrict();
Expand Down Expand Up @@ -2815,28 +2812,20 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalOb
JSC::JSObject* errorObject = objectArg.asCell()->getObject();
JSC::JSValue caller = callFrame->argument(1);

// We cannot use our ErrorInstance::captureStackTrace() fast path here unfortunately.
// We need to return these CallSite array objects which means we need to create them
JSValue errorValue = lexicalGlobalObject->get(lexicalGlobalObject, vm.propertyNames->Error);
auto* errorConstructor = jsDynamicCast<JSC::JSObject*>(errorValue);
size_t stackTraceLimit = globalObject->stackTraceLimit().value();
if (stackTraceLimit == 0) {
stackTraceLimit = DEFAULT_ERROR_STACK_TRACE_LIMIT;
}

JSCStackTrace stackTrace = JSCStackTrace::captureCurrentJSStackTrace(globalObject, callFrame, stackTraceLimit, caller);

// Create an (uninitialized) array for our "call sites"
JSC::GCDeferralContext deferralContext(vm);
JSC::ObjectInitializationScope objectScope(vm);
JSC::JSArray* callSites = JSC::JSArray::tryCreateUninitializedRestricted(objectScope,
&deferralContext,
// Note: we cannot use tryCreateUninitializedRestricted here because we cannot allocate memory inside initializeIndex()
JSC::JSArray* callSites = JSC::JSArray::create(vm,
globalObject->arrayStructureForIndexingTypeDuringAllocation(JSC::ArrayWithContiguous),
stackTrace.size());
RELEASE_ASSERT(callSites);

// Create the call sites (one per frame)
GlobalObject::createCallSitesFromFrames(lexicalGlobalObject, objectScope, stackTrace, callSites);
GlobalObject::createCallSitesFromFrames(lexicalGlobalObject, stackTrace, callSites);

/* Foramt the stack trace.
* Note that v8 won't actually format the stack trace here, but will create a "stack" accessor
Expand All @@ -2847,6 +2836,7 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalOb
size_t framesCount = stackTrace.size();
ZigStackFrame remappedFrames[framesCount];
for (int i = 0; i < framesCount; i++) {
memset(remappedFrames + i, 0, sizeof(ZigStackFrame));
remappedFrames[i].source_url = Bun::toString(lexicalGlobalObject, stackTrace.at(i).sourceURL());
if (JSCStackFrame::SourcePositions* sourcePositions = stackTrace.at(i).getSourcePositions()) {
remappedFrames[i].position.line = sourcePositions->line.zeroBasedInt();
Expand Down
2 changes: 1 addition & 1 deletion src/bun.js/bindings/ZigGlobalObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ class GlobalObject : public JSC::JSGlobalObject {

void clearDOMGuardedObjects();

static void createCallSitesFromFrames(JSC::JSGlobalObject* lexicalGlobalObject, JSC::ObjectInitializationScope& objectScope, JSCStackTrace& stackTrace, JSC::JSArray* callSites);
static void createCallSitesFromFrames(JSC::JSGlobalObject* lexicalGlobalObject, JSCStackTrace& stackTrace, JSC::JSArray* callSites);
JSC::JSValue formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSObject* errorObject, JSC::JSArray* callSites);

static void reportUncaughtExceptionAtEventLoop(JSGlobalObject*, JSC::Exception*);
Expand Down

0 comments on commit b566573

Please sign in to comment.