Skip to content

Commit

Permalink
[JSC] Make a bit defensive on jettisoning
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=274672
rdar://126591959

Reviewed by Yijia Huang.

Speculatively making a bit defensive against jettisoning.

1. Ensure jettisoning does not add dead CodeBlocks.
2. Loop finalizeCodeBlockEdge until we ensure that edge is cleared or edge is alive.
3. Clear CallSlot for dead cells in PolymorphicCallStubRoutine (not necessary, but just for defensive change).

* Source/JavaScriptCore/jit/PolymorphicCallStubRoutine.cpp:
(JSC::PolymorphicCallStubRoutine::visitWeakImpl):
* Source/JavaScriptCore/runtime/ScriptExecutable.cpp:
(JSC::ScriptExecutable::installCode):
* Source/JavaScriptCore/runtime/ScriptExecutableInlines.h:
(JSC::ScriptExecutable::finalizeCodeBlockEdge):

Canonical link: https://commits.webkit.org/279292@main
  • Loading branch information
Constellation committed May 24, 2024
1 parent dd1da9b commit fc1a727
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 20 deletions.
15 changes: 12 additions & 3 deletions Source/JavaScriptCore/jit/PolymorphicCallStubRoutine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,18 @@ void PolymorphicCallStubRoutine::clearCallNodesFor(CallLinkInfo*)
bool PolymorphicCallStubRoutine::visitWeakImpl(VM& vm)
{
bool isStillLive = true;
forEachDependentCell([&](JSCell* cell) {
isStillLive &= vm.heap.isMarked(cell);
});
for (unsigned i = 0, size = std::size(trailingSpan()) - 1; i < size; ++i) {
auto& slot = trailingSpan()[i];
if (!slot.m_calleeOrExecutable) {
isStillLive = false;
continue;
}
if (!vm.heap.isMarked(slot.m_calleeOrExecutable)) {
slot = CallSlot();
isStillLive = false;
continue;
}
}
return isStillLive;
}

Expand Down
24 changes: 12 additions & 12 deletions Source/JavaScriptCore/runtime/ScriptExecutable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,19 @@ void ScriptExecutable::installCode(CodeBlock* codeBlock)

void ScriptExecutable::installCode(VM& vm, CodeBlock* genericCodeBlock, CodeType codeType, CodeSpecializationKind kind, Profiler::JettisonReason reason)
{
if (genericCodeBlock)
if (genericCodeBlock) {
CODEBLOCK_LOG_EVENT(genericCodeBlock, "installCode", ());
switch (reason) {
case Profiler::JettisonReason::JettisonDueToWeakReference:
case Profiler::JettisonReason::JettisonDueToOldAge: {
if (genericCodeBlock && !vm.heap.isMarked(genericCodeBlock))
genericCodeBlock = nullptr;
break;
}
default:
break;
}
}

CodeBlock* oldCodeBlock = nullptr;

Expand Down Expand Up @@ -198,17 +209,6 @@ void ScriptExecutable::installCode(VM& vm, CodeBlock* genericCodeBlock, CodeType
debugger->registerCodeBlock(genericCodeBlock);
}

switch (reason) {
case Profiler::JettisonReason::JettisonDueToWeakReference:
case Profiler::JettisonReason::JettisonDueToOldAge: {
if (genericCodeBlock && !vm.heap.isMarked(genericCodeBlock))
genericCodeBlock = nullptr;
break;
}
default:
break;
}

if (oldCodeBlock)
oldCodeBlock->unlinkOrUpgradeIncomingCalls(vm, genericCodeBlock);

Expand Down
16 changes: 11 additions & 5 deletions Source/JavaScriptCore/runtime/ScriptExecutableInlines.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,23 @@ namespace JSC {

inline void ScriptExecutable::finalizeCodeBlockEdge(VM& vm, WriteBarrier<CodeBlock>& codeBlockEdge)
{
auto* codeBlock = codeBlockEdge.get();
if (!codeBlock)
return;
for (;;) {
auto* codeBlock = codeBlockEdge.get();
if (!codeBlock)
return;

if (vm.heap.isMarked(codeBlock))
return;

if (!vm.heap.isMarked(codeBlock)) {
if (codeBlock->shouldJettisonDueToWeakReference(vm))
codeBlock->jettison(Profiler::JettisonDueToWeakReference);
else
codeBlock->jettison(Profiler::JettisonDueToOldAge);
if (codeBlock == codeBlockEdge.get())

if (codeBlock == codeBlockEdge.get()) {
codeBlockEdge.clear();
return;
}
}
}

Expand Down

0 comments on commit fc1a727

Please sign in to comment.