Changeset 295614 in webkit for trunk/Source/JavaScriptCore/jit/JITPropertyAccess.cpp

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

File:

• trunk/Source/JavaScriptCore/jit/JITPropertyAccess.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/jit/JITPropertyAccess.cpp

@@ -63 +63 @@
     emitGetVirtualRegister(property, propertyJSR);
 
+    auto [ stubInfo, stubInfoIndex ] = addUnlinkedStructureStubInfo();
+    JITGetByValGenerator gen(
+        nullptr, stubInfo, JITType::BaselineJIT, CodeOrigin(m_bytecodeIndex), CallSiteIndex(m_bytecodeIndex), AccessType::GetByVal, RegisterSet::stubUnavailableRegisters(),
+        baseJSR, propertyJSR, resultJSR, stubInfoGPR);
+    if (isOperandConstantInt(property))
+        stubInfo->propertyIsInt32 = true;
+    gen.m_unlinkedStubInfoConstantIndex = stubInfoIndex;
+
     if (bytecode.metadata(m_profiledCodeBlock).m_seenIdentifiers.count() > Options::getByValICMaxNumberOfIdentifiers()) {
+        stubInfo->tookSlowPath = true;
+
         auto notCell = branchIfNotCell(baseJSR);
         emitArrayProfilingSiteWithCell(bytecode, baseJSR.payloadGPR(), scratchGPR);
         notCell.link(this);
         loadGlobalObject(scratchGPR);
-        callOperationWithProfile(bytecode, operationGetByVal, dst, scratchGPR, baseJSR, propertyJSR);
+        callOperationWithResult(operationGetByVal, resultJSR, scratchGPR, baseJSR, propertyJSR);
+
+        gen.generateEmptyPath(*this);
     } else {
         emitJumpSlowCaseIfNotJSCell(baseJSR, base);
         emitArrayProfilingSiteWithCell(bytecode, baseJSR.payloadGPR(), scratchGPR);
 
-        auto [ stubInfo, stubInfoIndex ] = addUnlinkedStructureStubInfo();
-        JITGetByValGenerator gen(
-            nullptr, stubInfo, JITType::BaselineJIT, CodeOrigin(m_bytecodeIndex), CallSiteIndex(m_bytecodeIndex), AccessType::GetByVal, RegisterSet::stubUnavailableRegisters(),
-            baseJSR, propertyJSR, resultJSR, stubInfoGPR);
-        if (isOperandConstantInt(property))
-            stubInfo->propertyIsInt32 = true;
-        gen.m_unlinkedStubInfoConstantIndex = stubInfoIndex;
-
         gen.generateBaselineDataICFastPath(*this, stubInfoIndex, stubInfoGPR);
-        resetSP(); // We might OSR exit here, so we need to conservatively reset SP
-
-        addSlowCase();
-        m_getByVals.append(gen);
-
-        setFastPathResumePoint();
-        emitValueProfilingSite(bytecode, resultJSR);
-        emitPutVirtualRegister(dst, resultJSR);
     }
+
+    addSlowCase();
+    m_getByVals.append(gen);
+
+    resetSP(); // We might OSR exit here, so we need to conservatively reset SP
+    setFastPathResumePoint();
+    emitValueProfilingSite(bytecode, resultJSR);
+    emitPutVirtualRegister(dst, resultJSR);
 }
 
@@ -96 +100 @@
 void JIT::generateGetByValSlowCase(const OpcodeType& bytecode, Vector<SlowCaseEntry>::iterator& iter)
 {
-    if (!hasAnySlowCases(iter))
-        return;
+    ASSERT(hasAnySlowCases(iter));
 
     uint32_t bytecodeOffset = m_bytecodeIndex.offset();
@@ -110 +113 @@
     linkAllSlowCases(iter);
 
-    move(TrustedImm32(bytecodeOffset), bytecodeOffsetGPR);
-    loadConstant(gen.m_unlinkedStubInfoConstantIndex, stubInfoGPR);
-    materializePointerIntoMetadata(bytecode, OpcodeType::Metadata::offsetOfArrayProfile(), profileGPR);
-
-    emitNakedNearCall(vm().getCTIStub(slow_op_get_by_val_callSlowOperationThenCheckExceptionGenerator).retaggedCode<NoPtrTag>());
+    if (!gen.m_unlinkedStubInfo->tookSlowPath) {
+        move(TrustedImm32(bytecodeOffset), bytecodeOffsetGPR);
+        loadConstant(gen.m_unlinkedStubInfoConstantIndex, stubInfoGPR);
+        materializePointerIntoMetadata(bytecode, OpcodeType::Metadata::offsetOfArrayProfile(), profileGPR);
+
+        emitNakedNearCall(vm().getCTIStub(slow_op_get_by_val_callSlowOperationThenCheckExceptionGenerator).retaggedCode<NoPtrTag>());
+    }
 
     gen.reportSlowPathCall(coldPathBegin, Call());