Changeset 295614 in webkit for trunk/Source/JavaScriptCore/jit


Ignore:
Timestamp:
Jun 16, 2022, 4:08:33 PM (3 years ago)
Author:
[email protected]
Message:

[JSC] Always create StructureStubInfo for op_get_by_val
​https://bugs.webkit.org/show_bug.cgi?id=241669
rdar://75146284

Reviewed by Saam Barati and Mark Lam.

DFG OSR exit requires StructureStubInfo for getter / setter calls. However very generic baseline JIT
op_get_by_val does not create StructureStubInfo. It is possible that OSR exit crashes because of this
missing StructureStubInfo. Let's consider the following edge case.

  1. Now, Baseline detects that this is very generic op_get_by_val. So we do not create StructureStubInfo.
  2. This function is inlined in DFG. And DFG emits IC for this GetByVal.
  3. (2)'s DFG function collects information in DFG-level IC. And luckily, in this inlined call path, it was not so generic.
  4. Then, due to different OSR exit or something, we recreate DFG code for this function with (2)'s inlining.
  5. DFG detects that DFG-level IC has more specialized information. So it can inline getter call in this op_get_by_val.
  6. Inside this getter, we perform OSR exit.
  7. Looking into Baseline, and we found that there is no StructureStubInfo!

We always create StructureStubInfo. In very generic op_get_by_val case, we create this with tookSlowPath = true.
And we emit empty inline path to record doneLocation. So, OSR exit can jump to this place.

We also clean up StructureStubInfo code.

  1. "start" is renamed to startLocation. And we do not record it in DataIC case since it is not necessary.
  2. Rename inlineSize to inlineCodeSize.
  3. Add some assertions to ensure that this path is not used for DataIC case.
  4. We also record opcode value in the crashing RELEASE_ASSERT to get more information if this does not fix the issue.
  • Source/JavaScriptCore/bytecode/InlineAccess.cpp:

(JSC::linkCodeInline):
(JSC::InlineAccess::generateArrayLength):
(JSC::InlineAccess::generateStringLength):
(JSC::InlineAccess::rewireStubAsJumpInAccessNotUsingInlineAccess):
(JSC::InlineAccess::rewireStubAsJumpInAccess):
(JSC::InlineAccess::resetStubAsJumpInAccess):

  • Source/JavaScriptCore/bytecode/StructureStubInfo.cpp:

(JSC::StructureStubInfo::initializeFromUnlinkedStructureStubInfo):
(JSC::StructureStubInfo::initializeFromDFGUnlinkedStructureStubInfo):

  • Source/JavaScriptCore/bytecode/StructureStubInfo.h:

(JSC::StructureStubInfo::inlineCodeSize const):
(JSC::StructureStubInfo::inlineSize const): Deleted.

  • Source/JavaScriptCore/dfg/DFGInlineCacheWrapperInlines.h:

(JSC::DFG::InlineCacheWrapper<GeneratorType>::finalize):

  • Source/JavaScriptCore/dfg/DFGJITCode.h:
  • Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:

(JSC::DFG::callerReturnPC):

  • Source/JavaScriptCore/jit/JIT.cpp:

(JSC::JIT::link):

  • Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp:

(JSC::JITInlineCacheGenerator::finalize):
(JSC::JITGetByValGenerator::generateEmptyPath):

  • Source/JavaScriptCore/jit/JITInlineCacheGenerator.h:
  • Source/JavaScriptCore/jit/JITPropertyAccess.cpp:

(JSC::JIT::emit_op_get_by_val):

  • JSTests/stress/get-by-val-generic-structurestubinfo.js: Added.

(let.program):
(runMono.let.o.get x):
(runMono):
(runPoly):

Canonical link: ​https://commits.webkit.org/251619@main

Location:
trunk/Source/JavaScriptCore/jit
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/jit/JIT.cpp

    r294873 r295614  
    947947    auto finalizeICs = [&] (auto& generators) {
    948948        for (auto& gen : generators) {
    949             gen.m_unlinkedStubInfo->start = patchBuffer.locationOf<JITStubRoutinePtrTag>(gen.m_start);
    950949            gen.m_unlinkedStubInfo->doneLocation = patchBuffer.locationOf<JSInternalPtrTag>(gen.m_done);
    951950            gen.m_unlinkedStubInfo->slowPathStartLocation = patchBuffer.locationOf<JITStubRoutinePtrTag>(gen.m_slowPathBegin);

  • trunk/Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp

    r294873 r295614  
    6464{
    6565    ASSERT(m_stubInfo);
    66     m_stubInfo->start = start;
     66    m_stubInfo->startLocation = start;
    6767    m_stubInfo->doneLocation = fastPath.locationOf<JSInternalPtrTag>(m_done);
    6868
    … …  
    535535}
    536536
     537void JITGetByValGenerator::generateEmptyPath(CCallHelpers& jit)
     538{
     539    m_start = jit.label();
     540    m_done = jit.label();
     541}
     542
    537543void JITGetByValGenerator::finalize(LinkBuffer& fastPath, LinkBuffer& slowPath)
    538544{

  • trunk/Source/JavaScriptCore/jit/JITInlineCacheGenerator.h

    r294873 r295614  
    559559    void generateFastPath(CCallHelpers&);
    560560
     561    void generateEmptyPath(CCallHelpers&);
     562
    561563    template<typename StubInfo>
    562564    static void setUpStubInfo(StubInfo& stubInfo,

  • trunk/Source/JavaScriptCore/jit/JITPropertyAccess.cpp

    r295008 r295614  
    6363    emitGetVirtualRegister(property, propertyJSR);
    6464
     65    auto [ stubInfo, stubInfoIndex ] = addUnlinkedStructureStubInfo();
     66    JITGetByValGenerator gen(
     67        nullptr, stubInfo, JITType::BaselineJIT, CodeOrigin(m_bytecodeIndex), CallSiteIndex(m_bytecodeIndex), AccessType::GetByVal, RegisterSet::stubUnavailableRegisters(),
     68        baseJSR, propertyJSR, resultJSR, stubInfoGPR);
     69    if (isOperandConstantInt(property))
     70        stubInfo->propertyIsInt32 = true;
     71    gen.m_unlinkedStubInfoConstantIndex = stubInfoIndex;
     72
    6573    if (bytecode.metadata(m_profiledCodeBlock).m_seenIdentifiers.count() > Options::getByValICMaxNumberOfIdentifiers()) {
     74        stubInfo->tookSlowPath = true;
     75
    6676        auto notCell = branchIfNotCell(baseJSR);
    6777        emitArrayProfilingSiteWithCell(bytecode, baseJSR.payloadGPR(), scratchGPR);
    6878        notCell.link(this);
    6979        loadGlobalObject(scratchGPR);
    70         callOperationWithProfile(bytecode, operationGetByVal, dst, scratchGPR, baseJSR, propertyJSR);
     80        callOperationWithResult(operationGetByVal, resultJSR, scratchGPR, baseJSR, propertyJSR);
     81
     82        gen.generateEmptyPath(*this);
    7183    } else {
    7284        emitJumpSlowCaseIfNotJSCell(baseJSR, base);
    7385        emitArrayProfilingSiteWithCell(bytecode, baseJSR.payloadGPR(), scratchGPR);
    7486
    75         auto [ stubInfo, stubInfoIndex ] = addUnlinkedStructureStubInfo();
    76         JITGetByValGenerator gen(
    77             nullptr, stubInfo, JITType::BaselineJIT, CodeOrigin(m_bytecodeIndex), CallSiteIndex(m_bytecodeIndex), AccessType::GetByVal, RegisterSet::stubUnavailableRegisters(),
    78             baseJSR, propertyJSR, resultJSR, stubInfoGPR);
    79         if (isOperandConstantInt(property))
    80             stubInfo->propertyIsInt32 = true;
    81         gen.m_unlinkedStubInfoConstantIndex = stubInfoIndex;
    82 
    8387        gen.generateBaselineDataICFastPath(*this, stubInfoIndex, stubInfoGPR);
    84         resetSP(); // We might OSR exit here, so we need to conservatively reset SP
    85 
    86         addSlowCase();
    87         m_getByVals.append(gen);
    88 
    89         setFastPathResumePoint();
    90         emitValueProfilingSite(bytecode, resultJSR);
    91         emitPutVirtualRegister(dst, resultJSR);
    9288    }
     89
     90    addSlowCase();
     91    m_getByVals.append(gen);
     92
     93    resetSP(); // We might OSR exit here, so we need to conservatively reset SP
     94    setFastPathResumePoint();
     95    emitValueProfilingSite(bytecode, resultJSR);
     96    emitPutVirtualRegister(dst, resultJSR);
    9397}
    9498
    … …  
    96100void JIT::generateGetByValSlowCase(const OpcodeType& bytecode, Vector<SlowCaseEntry>::iterator& iter)
    97101{
    98     if (!hasAnySlowCases(iter))
    99         return;
     102    ASSERT(hasAnySlowCases(iter));
    100103
    101104    uint32_t bytecodeOffset = m_bytecodeIndex.offset();
    … …  
    110113    linkAllSlowCases(iter);
    111114
    112     move(TrustedImm32(bytecodeOffset), bytecodeOffsetGPR);
    113     loadConstant(gen.m_unlinkedStubInfoConstantIndex, stubInfoGPR);
    114     materializePointerIntoMetadata(bytecode, OpcodeType::Metadata::offsetOfArrayProfile(), profileGPR);
    115 
    116     emitNakedNearCall(vm().getCTIStub(slow_op_get_by_val_callSlowOperationThenCheckExceptionGenerator).retaggedCode<NoPtrTag>());
     115    if (!gen.m_unlinkedStubInfo->tookSlowPath) {
     116        move(TrustedImm32(bytecodeOffset), bytecodeOffsetGPR);
     117        loadConstant(gen.m_unlinkedStubInfoConstantIndex, stubInfoGPR);
     118        materializePointerIntoMetadata(bytecode, OpcodeType::Metadata::offsetOfArrayProfile(), profileGPR);
     119
     120        emitNakedNearCall(vm().getCTIStub(slow_op_get_by_val_callSlowOperationThenCheckExceptionGenerator).retaggedCode<NoPtrTag>());
     121    }
    117122
    118123    gen.reportSlowPathCall(coldPathBegin, Call());
Note: See TracChangeset for help on using the changeset viewer.