Skip to content

Commit 59c35e2

Browse files
author
Mark Lam
committed
Replace uses of CallFrame::iterate() with StackVisitor::visit().
https://bugs.webkit.org/show_bug.cgi?id=250751 <rdar://problem/104363671> Reviewed by Yusuke Suzuki. An assertion in CallFrame::iterate() is blocking StackVisitor from dumping the JS stack when we encounter Wasm frames. The assertion was from back when Wasm frames didn't exist. This patch removes CallFrame::iterate() completely, and changes all callsites to call StackVisitor::visit() instead. * Source/JavaScriptCore/API/JSContextRef.cpp: (JSContextCreateBacktrace): * Source/JavaScriptCore/bytecode/CodeBlock.cpp: (JSC::CodeBlock::noticeIncomingCall): * Source/JavaScriptCore/inspector/ScriptCallStackFactory.cpp: (Inspector::createScriptCallStack): (Inspector::createScriptCallStackForConsole): * Source/JavaScriptCore/interpreter/CallFrame.h: (JSC::CallFrame::iterate): Deleted. * Source/JavaScriptCore/jsc.cpp: (JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/runtime/FunctionPrototype.cpp: (JSC::retrieveArguments): (JSC::retrieveCallerFunction): * Source/JavaScriptCore/runtime/JSGlobalObject.cpp: * Source/JavaScriptCore/runtime/NullSetterFunction.cpp: (JSC::callerIsStrict): * Source/JavaScriptCore/tools/JSDollarVM.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/tools/VMInspector.cpp: (JSC::VMInspector::codeBlockForFrame): (JSC::VMInspector::dumpCallFrame): (JSC::VMInspector::dumpRegisters): (JSC::VMInspector::dumpStack): * Source/WebCore/bindings/js/JSDOMGlobalObject.cpp: (WebCore::callerGlobalObject): * Source/WebCore/testing/Internals.cpp: (WebCore::Internals::parserMetaData): Canonical link: https://commits.webkit.org/259027@main
1 parent 01831b1 commit 59c35e2

12 files changed

Lines changed: 22 additions & 34 deletions

File tree

‎Source/JavaScriptCore/API/JSContextRef.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ JSStringRef JSContextCreateBacktrace(JSContextRef ctx, unsigned maxStackSize)
367367

368368
ASSERT(maxStackSize);
369369
BacktraceFunctor functor(builder, maxStackSize);
370-
frame->iterate(vm, functor);
370+
StackVisitor::visit(frame, vm, functor);
371371

372372
return OpaqueJSString::tryCreate(builder.toString()).leakRef();
373373
}

‎Source/JavaScriptCore/bytecode/CodeBlock.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2425,8 +2425,9 @@ void CodeBlock::noticeIncomingCall(CallFrame* callerFrame)
24252425
}
24262426

24272427
// Recursive calls won't be inlined.
2428+
VM& vm = this->vm();
24282429
RecursionCheckFunctor functor(callerFrame, this, Options::maximumInliningDepth());
2429-
vm().topCallFrame->iterate(vm(), functor);
2430+
StackVisitor::visit(vm.topCallFrame, vm, functor);
24302431

24312432
if (functor.didRecurse()) {
24322433
dataLogLnIf(Options::verboseCallLink(), " Clearing SABI because recursion was detected.");

‎Source/JavaScriptCore/inspector/ScriptCallStackFactory.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ Ref<ScriptCallStack> createScriptCallStack(JSC::JSGlobalObject* globalObject, si
116116
return ScriptCallStack::create();
117117

118118
CreateScriptCallStackFunctor functorIncludingFirstFrame(globalObject, false, maxStackSize);
119-
frame->iterate(vm, functorIncludingFirstFrame);
119+
StackVisitor::visit(frame, vm, functorIncludingFirstFrame);
120120
return functorIncludingFirstFrame.takeStack();
121121
}
122122

@@ -133,11 +133,11 @@ Ref<ScriptCallStack> createScriptCallStackForConsole(JSC::JSGlobalObject* global
133133
return ScriptCallStack::create();
134134

135135
CreateScriptCallStackFunctor functorSkippingFirstFrame(globalObject, true, maxStackSize);
136-
frame->iterate(vm, functorSkippingFirstFrame);
136+
StackVisitor::visit(frame, vm, functorSkippingFirstFrame);
137137
auto stack = functorSkippingFirstFrame.takeStack();
138138
if (!stack->size()) {
139139
CreateScriptCallStackFunctor functorIncludingFirstFrame(globalObject, false, maxStackSize);
140-
frame->iterate(vm, functorIncludingFirstFrame);
140+
StackVisitor::visit(frame, vm, functorIncludingFirstFrame);
141141
stack = functorIncludingFirstFrame.takeStack();
142142
}
143143
return stack;

‎Source/JavaScriptCore/interpreter/CallFrame.h‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -319,19 +319,6 @@ using JSInstruction = BaseInstruction<JSOpcodeTraits>;
319319
JS_EXPORT_PRIVATE static JSGlobalObject* globalObjectOfClosestCodeBlock(VM&, CallFrame*);
320320
String friendlyFunctionName();
321321

322-
// CallFrame::iterate() expects a Functor that implements the following method:
323-
// IterationStatus operator()(StackVisitor&) const;
324-
// FIXME: This method is improper. We rely on the fact that we can call it with a null
325-
// receiver. We should always be using StackVisitor directly.
326-
// It's only valid to call this from a non-wasm top frame.
327-
template <StackVisitor::EmptyEntryFrameAction action = StackVisitor::ContinueIfTopEntryFrameIsEmpty, typename Functor> void iterate(VM& vm, const Functor& functor)
328-
{
329-
void* rawThis = this;
330-
if (!!rawThis)
331-
RELEASE_ASSERT(callee().isCell());
332-
StackVisitor::visit<action, Functor>(this, vm, functor);
333-
}
334-
335322
void dump(PrintStream&) const;
336323

337324
// This function is used in LLDB btjs.

‎Source/JavaScriptCore/jsc.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1508,7 +1508,7 @@ JSC_DEFINE_HOST_FUNCTION(functionJSCStack, (JSGlobalObject* globalObject, CallFr
15081508
trace.append("--> Stack trace:\n");
15091509

15101510
FunctionJSCStackFunctor functor(trace);
1511-
callFrame->iterate(vm, functor);
1511+
StackVisitor::visit(callFrame, vm, functor);
15121512
fprintf(stderr, "%s", trace.toString().utf8().data());
15131513
return JSValue::encode(jsUndefined());
15141514
}

‎Source/JavaScriptCore/runtime/FunctionPrototype.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ static JSValue retrieveArguments(VM& vm, CallFrame* callFrame, JSFunction* funct
148148
{
149149
RetrieveArgumentsFunctor functor(vm, functionObj);
150150
if (callFrame)
151-
callFrame->iterate(vm, functor);
151+
StackVisitor::visit(callFrame, vm, functor);
152152
return functor.result();
153153
}
154154

@@ -217,7 +217,7 @@ static JSValue retrieveCallerFunction(VM& vm, CallFrame* callFrame, JSFunction*
217217
{
218218
RetrieveCallerFunctionFunctor functor(functionObj);
219219
if (callFrame)
220-
callFrame->iterate(vm, functor);
220+
StackVisitor::visit(callFrame, vm, functor);
221221
return functor.result();
222222
}
223223

‎Source/JavaScriptCore/runtime/JSGlobalObject.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ JSC_DEFINE_HOST_FUNCTION(assertCall, (JSGlobalObject* globalObject, CallFrame* c
442442
bool iteratedOnce = false;
443443
CodeBlock* codeBlock = nullptr;
444444
unsigned line;
445-
callFrame->iterate(globalObject->vm(), [&] (StackVisitor& visitor) {
445+
StackVisitor::visit(callFrame, globalObject->vm(), [&] (StackVisitor& visitor) {
446446
if (!iteratedOnce) {
447447
iteratedOnce = true;
448448
return IterationStatus::Continue;

‎Source/JavaScriptCore/runtime/NullSetterFunction.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ class GetCallerStrictnessFunctor {
6767
static bool callerIsStrict(VM& vm, CallFrame* callFrame)
6868
{
6969
GetCallerStrictnessFunctor iter;
70-
callFrame->iterate(vm, iter);
70+
StackVisitor::visit(callFrame, vm, iter);
7171
return iter.callerIsStrict();
7272
}
7373

‎Source/JavaScriptCore/tools/JSDollarVM.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ class JSDollarVMCallFrame : public JSNonFinalObject {
171171

172172
unsigned frameIndex = 0;
173173
bool isValid = false;
174-
callFrame->iterate(vm, [&] (StackVisitor& visitor) {
174+
StackVisitor::visit(callFrame, vm, [&] (StackVisitor& visitor) {
175175
DollarVMAssertScope assertScope;
176176

177177
if (frameIndex++ != requestedFrameIndex)
@@ -2439,7 +2439,7 @@ JSC_DEFINE_HOST_FUNCTION(functionLLintTrue, (JSGlobalObject* globalObject, CallF
24392439
if (!callFrame)
24402440
return JSValue::encode(jsUndefined());
24412441
CallerFrameJITTypeFunctor functor;
2442-
callFrame->iterate(vm, functor);
2442+
StackVisitor::visit(callFrame, vm, functor);
24432443
return JSValue::encode(jsBoolean(functor.jitType() == JITType::InterpreterThunk));
24442444
}
24452445

@@ -2452,7 +2452,7 @@ JSC_DEFINE_HOST_FUNCTION(functionBaselineJITTrue, (JSGlobalObject* globalObject,
24522452
if (!callFrame)
24532453
return JSValue::encode(jsUndefined());
24542454
CallerFrameJITTypeFunctor functor;
2455-
callFrame->iterate(vm, functor);
2455+
StackVisitor::visit(callFrame, vm, functor);
24562456
return JSValue::encode(jsBoolean(functor.jitType() == JITType::BaselineJIT));
24572457
}
24582458

@@ -2697,7 +2697,7 @@ JSC_DEFINE_HOST_FUNCTION(functionDumpRegisters, (JSGlobalObject* globalObject, C
26972697
}
26982698

26992699
unsigned frameIndex = 0;
2700-
callFrame->iterate(vm, [&] (StackVisitor& visitor) {
2700+
StackVisitor::visit(callFrame, vm, [&] (StackVisitor& visitor) {
27012701
DollarVMAssertScope assertScope;
27022702
if (frameIndex++ != requestedFrameIndex)
27032703
return IterationStatus::Continue;

‎Source/JavaScriptCore/tools/VMInspector.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ CodeBlock* VMInspector::codeBlockForFrame(VM* vm, CallFrame* topCallFrame, unsig
315315
};
316316

317317
FetchCodeBlockFunctor functor(frameNumber);
318-
topCallFrame->iterate(*vm, functor);
318+
StackVisitor::visit(topCallFrame, *vm, functor);
319319
return functor.codeBlock;
320320
}
321321

@@ -356,7 +356,7 @@ void VMInspector::dumpCallFrame(VM* vm, CallFrame* callFrame, unsigned framesToS
356356
if (!ensureCurrentThreadOwnsJSLock(vm))
357357
return;
358358
DumpFrameFunctor functor(DumpFrameFunctor::DumpOne, framesToSkip);
359-
callFrame->iterate(*vm, functor);
359+
StackVisitor::visit(callFrame, *vm, functor);
360360
}
361361

362362
void VMInspector::dumpRegisters(CallFrame* callFrame)
@@ -394,7 +394,7 @@ void VMInspector::dumpRegisters(CallFrame* callFrame)
394394
dataLogF("-----------------------------------------------------------------------------\n");
395395
dataLogF("[ArgumentCount] | %10p | %lu \n", it, (unsigned long) callFrame->argumentCount());
396396

397-
callFrame->iterate(vm, [&] (StackVisitor& visitor) {
397+
StackVisitor::visit(callFrame, vm, [&] (StackVisitor& visitor) {
398398
if (visitor->callFrame() == callFrame) {
399399
unsigned line = 0;
400400
unsigned unusedColumn = 0;
@@ -443,7 +443,7 @@ void VMInspector::dumpRegisters(CallFrame* callFrame)
443443
CallFrame* nextCallFrame = nullptr;
444444

445445
if (topCallFrame) {
446-
topCallFrame->iterate(vm, [&] (StackVisitor& visitor) {
446+
StackVisitor::visit(topCallFrame, vm, [&] (StackVisitor& visitor) {
447447
if (callFrame == visitor->callFrame())
448448
return IterationStatus::Done;
449449
nextCallFrame = visitor->callFrame();
@@ -478,7 +478,7 @@ void VMInspector::dumpStack(VM* vm, CallFrame* topCallFrame, unsigned framesToSk
478478
if (!topCallFrame)
479479
return;
480480
DumpFrameFunctor functor(DumpFrameFunctor::DumpAll, framesToSkip);
481-
topCallFrame->iterate(*vm, functor);
481+
StackVisitor::visit(topCallFrame, *vm, functor);
482482
}
483483

484484
void VMInspector::dumpValue(JSValue value)

0 commit comments

Comments
 (0)