Skip to content

Commit

Permalink
Cherry-pick 6177152. rdar://122611742
Browse files Browse the repository at this point in the history
    [JSC] Clean up CallLinkInfo::unlinlkOrUpgradeImpl to make `remove` consistent
    https://bugs.webkit.org/show_bug.cgi?id=270004
    rdar://122611742

    Reviewed by Alexey Shvayka.

    1. In CallLinkInfo::unlinkOrUpgradeImpl, let's always start with removing it from the list. If we upgrade, we anyway re-chain it to the new CodeBlock.
       So there is no possible case that we would like to keep the current link.
    2. Let's make linked / unlinked state consistent more by moving `remove` code inside CallLinkInfo itself. And always check `isOnList` before calling it.

    * Source/JavaScriptCore/bytecode/CallLinkInfo.cpp:
    (JSC::CallLinkInfo::unlinkOrUpgradeImpl):
    (JSC::CallLinkInfo::setStub):
    * Source/JavaScriptCore/bytecode/Repatch.cpp:
    (JSC::linkPolymorphicCall):

    Canonical link: https://commits.webkit.org/275256@main

Canonical link: https://commits.webkit.org/272448.634@safari-7618-branch
  • Loading branch information
Constellation authored and Dan Robson committed Feb 28, 2024
1 parent f25738c commit 897c2d7
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 16 deletions.
30 changes: 25 additions & 5 deletions Source/JavaScriptCore/bytecode/CallLinkInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,24 +96,39 @@ void CallLinkInfo::unlinkOrUpgradeImpl(VM& vm, CodeBlock* oldCodeBlock, CodeBloc
{
// We could be called even if we're not linked anymore because of how polymorphic calls
// work. Each callsite within the polymorphic call stub may separately ask us to unlink().
if (isLinked()) {
if (newCodeBlock && isDataIC() && mode() == Mode::Monomorphic && oldCodeBlock == u.dataIC.m_codeBlock) {
if (isOnList())
remove();

dataLogLnIf(Options::dumpDisassembly(), "Unlinking CallLinkInfo: ", RawPointer(this));

Mode mode = this->mode();
switch (mode) {
case Mode::Monomorphic: {
if (newCodeBlock && isDataIC() && oldCodeBlock == u.dataIC.m_codeBlock) {
// Upgrading Monomorphic DataIC with newCodeBlock.
remove();
ArityCheckMode arityCheck = oldCodeBlock->jitCode()->addressForCall(ArityCheckNotRequired) == u.dataIC.m_monomorphicCallDestination ? ArityCheckNotRequired : MustCheckArity;
auto target = newCodeBlock->jitCode()->addressForCall(arityCheck);
u.dataIC.m_codeBlock = newCodeBlock;
u.dataIC.m_monomorphicCallDestination = target;
newCodeBlock->linkIncomingCall(nullptr, this); // This is just relinking. So owner and caller frame can be nullptr.
return;
}
dataLogLnIf(Options::dumpDisassembly(), "Unlinking CallLinkInfo: ", RawPointer(this));
revertCall(vm);
break;
}
case Mode::Polymorphic: {
revertCall(vm);
break;
}
case Mode::Init:
case Mode::Virtual: {
break;
}
}

// Either we were unlinked, in which case we should not have been on any list, or we unlinked
// ourselves so that we're not on any list anymore.
RELEASE_ASSERT(!isOnList());
RELEASE_ASSERT(!isOnList(), static_cast<unsigned>(mode));
}

CodeLocationLabel<JSInternalPtrTag> CallLinkInfo::doneLocation()
Expand Down Expand Up @@ -442,6 +457,11 @@ void CallLinkInfo::setStub(Ref<PolymorphicCallStubRoutine>&& newStub)
MacroAssembler::startOfBranchPtrWithPatchOnRegister(u.codeIC.m_calleeLocation),
CodeLocationLabel<JITStubRoutinePtrTag>(m_stub->code().code()));
}

// The call link info no longer has a call cache apart from the jump to the polymorphic call stub.
if (isOnList())
remove();

m_mode = static_cast<unsigned>(Mode::Polymorphic);
}

Expand Down
11 changes: 0 additions & 11 deletions Source/JavaScriptCore/bytecode/Repatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1924,12 +1924,6 @@ void linkPolymorphicCall(VM& vm, JSCell* owner, CallFrame* callFrame, CallLinkIn
// If there had been a previous stub routine, that one will die as soon as the GC runs and sees
// that it's no longer on stack.
callLinkInfo.setStub(WTFMove(stubRoutine));

// The call link info no longer has a call cache apart from the jump to the polymorphic call
// stub.
if (callLinkInfo.isOnList())
callLinkInfo.remove();

return;
}

Expand Down Expand Up @@ -2050,11 +2044,6 @@ void linkPolymorphicCall(VM& vm, JSCell* owner, CallFrame* callFrame, CallLinkIn
// If there had been a previous stub routine, that one will die as soon as the GC runs and sees
// that it's no longer on stack.
callLinkInfo.setStub(WTFMove(stubRoutine));

// The call link info no longer has a call cache apart from the jump to the polymorphic call
// stub.
if (callLinkInfo.isOnList())
callLinkInfo.remove();
}

void resetGetBy(CodeBlock* codeBlock, StructureStubInfo& stubInfo, GetByKind kind)
Expand Down

0 comments on commit 897c2d7

Please sign in to comment.