Diff
Modified: trunk/JSTests/ChangeLog (258873 => 258874)
--- trunk/JSTests/ChangeLog 2020-03-23 21:00:01 UTC (rev 258873)
+++ trunk/JSTests/ChangeLog 2020-03-23 21:09:21 UTC (rev 258874)
@@ -1,3 +1,15 @@
+2020-03-23 Yusuke Suzuki <[email protected]>
+
+ [JSC] DFG OSR exit cannot find StructureStubInfo for put_by_val if CodeBlock is once converved from Baseline to LLInt
+ https://bugs.webkit.org/show_bug.cgi?id=209327
+ <rdar://problem/60631061>
+
+ Reviewed by Saam Barati.
+
+ * stress/osr-exit-attempts-to-find-stubinfo-which-is-cleared-by-previous-baseline-to-llint-conversion.js: Added.
+ (setter):
+ (foo):
+
2020-03-23 Ross Kirsling <[email protected]>
Catch parameters must not be lexically redeclared
Added: trunk/JSTests/stress/osr-exit-attempts-to-find-stubinfo-which-is-cleared-by-previous-baseline-to-llint-conversion.js (0 => 258874)
--- trunk/JSTests/stress/osr-exit-attempts-to-find-stubinfo-which-is-cleared-by-previous-baseline-to-llint-conversion.js (rev 0)
+++ trunk/JSTests/stress/osr-exit-attempts-to-find-stubinfo-which-is-cleared-by-previous-baseline-to-llint-conversion.js 2020-03-23 21:09:21 UTC (rev 258874)
@@ -0,0 +1,18 @@
+//@ runDefault("--slowPathAllocsBetweenGCs=50", "--jitPolicyScale=0", "--watchdog=100", "--watchdog-exception-ok")
+let o = {};
+let eff = 'f';
+
+function setter(s) {
+ +s;
+ [arguments];
+}
+
+Object.defineProperty(o, eff, { set: setter });
+
+function foo() {
+ o[eff] = null;
+}
+
+for (let i = 0; i < 100000000; i++) {
+ foo();
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (258873 => 258874)
--- trunk/Source/_javascript_Core/ChangeLog 2020-03-23 21:00:01 UTC (rev 258873)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-03-23 21:09:21 UTC (rev 258874)
@@ -1,3 +1,39 @@
+2020-03-23 Yusuke Suzuki <[email protected]>
+
+ [JSC] DFG OSR exit cannot find StructureStubInfo for put_by_val if CodeBlock is once converved from Baseline to LLInt
+ https://bugs.webkit.org/show_bug.cgi?id=209327
+ <rdar://problem/60631061>
+
+ Reviewed by Saam Barati.
+
+ DFG compiles op_put_by_val as PutById and inlines SetterCall only when DFG found StructureStubInfo for this op_put_by_val.
+ However, it is still possible that DFG OSR exit cannot find StructureStubInfo for SetterCall generated by op_put_by_val.
+ Let's consider the following scenario.
+
+ 1. Baseline CodeBlock (A) is compiled.
+ 2. (A) gets DFG (B).
+ 3. Since (A) collects enough information for put_by_val, (B) can get StructureStubInfo from (A) and compile it as inlined Setter call.
+ 4. (A)'s JITData is destroyed since it is not executed. Then, (A) becomes LLInt.
+ 5. The CodeBlock inlining (A) gets OSR exit. So (A) is executed and (A) eventually gets Baseline CodeBlock again.
+ 6. (B) gets OSR exit. (B) attempts to search for StructureStubInfo in (A) for PutById (originally, put_by_val). But it does not exist since (A)'s JITData is cleared once.
+
+ We should just link to doneTarget of ByValInfo when the SetterCall is generated by `op_put_by_val`. ByValInfo and its doneTarget always exists per op_put_by_val.
+
+ * bytecode/ByValInfo.h:
+ (JSC::ByValInfo::ByValInfo):
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::findByValInfo):
+ * bytecode/CodeBlock.h:
+ * dfg/DFGOSRExitCompilerCommon.cpp:
+ (JSC::DFG::callerReturnPC):
+ * jit/JITOpcodes.cpp:
+ (JSC::JIT::privateCompileHasIndexedProperty):
+ * jit/JITOpcodes32_64.cpp:
+ (JSC::JIT::privateCompileHasIndexedProperty):
+ * jit/JITPropertyAccess.cpp:
+ (JSC::JIT::privateCompilePutByVal):
+ (JSC::JIT::privateCompilePutByValWithCachedId):
+
2020-03-23 Ross Kirsling <[email protected]>
Unreviewed, address Yusuke's feedback on r258801.
Modified: trunk/Source/_javascript_Core/bytecode/ByValInfo.h (258873 => 258874)
--- trunk/Source/_javascript_Core/bytecode/ByValInfo.h 2020-03-23 21:00:01 UTC (rev 258873)
+++ trunk/Source/_javascript_Core/bytecode/ByValInfo.h 2020-03-23 21:09:21 UTC (rev 258874)
@@ -227,11 +227,11 @@
struct ByValInfo {
ByValInfo() { }
- ByValInfo(BytecodeIndex bytecodeIndex, CodeLocationJump<JSInternalPtrTag> notIndexJump, CodeLocationJump<JSInternalPtrTag> badTypeJump, CodeLocationLabel<ExceptionHandlerPtrTag> exceptionHandler, JITArrayMode arrayMode, ArrayProfile* arrayProfile, CodeLocationLabel<JSInternalPtrTag> badTypeDoneTarget, CodeLocationLabel<JSInternalPtrTag> badTypeNextHotPathTarget, CodeLocationLabel<JSInternalPtrTag> slowPathTarget)
+ ByValInfo(BytecodeIndex bytecodeIndex, CodeLocationJump<JSInternalPtrTag> notIndexJump, CodeLocationJump<JSInternalPtrTag> badTypeJump, CodeLocationLabel<ExceptionHandlerPtrTag> exceptionHandler, JITArrayMode arrayMode, ArrayProfile* arrayProfile, CodeLocationLabel<JSInternalPtrTag> doneTarget, CodeLocationLabel<JSInternalPtrTag> badTypeNextHotPathTarget, CodeLocationLabel<JSInternalPtrTag> slowPathTarget)
: notIndexJump(notIndexJump)
, badTypeJump(badTypeJump)
, exceptionHandler(exceptionHandler)
- , badTypeDoneTarget(badTypeDoneTarget)
+ , doneTarget(doneTarget)
, badTypeNextHotPathTarget(badTypeNextHotPathTarget)
, slowPathTarget(slowPathTarget)
, arrayProfile(arrayProfile)
@@ -249,7 +249,7 @@
CodeLocationJump<JSInternalPtrTag> notIndexJump;
CodeLocationJump<JSInternalPtrTag> badTypeJump;
CodeLocationLabel<ExceptionHandlerPtrTag> exceptionHandler;
- CodeLocationLabel<JSInternalPtrTag> badTypeDoneTarget;
+ CodeLocationLabel<JSInternalPtrTag> doneTarget;
CodeLocationLabel<JSInternalPtrTag> badTypeNextHotPathTarget;
CodeLocationLabel<JSInternalPtrTag> slowPathTarget;
ArrayProfile* arrayProfile;
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (258873 => 258874)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2020-03-23 21:00:01 UTC (rev 258873)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2020-03-23 21:09:21 UTC (rev 258874)
@@ -1557,6 +1557,18 @@
return nullptr;
}
+ByValInfo* CodeBlock::findByValInfo(CodeOrigin codeOrigin)
+{
+ ConcurrentJSLocker locker(m_lock);
+ if (auto* jitData = m_jitData.get()) {
+ for (ByValInfo* byValInfo : jitData->m_byValInfos) {
+ if (byValInfo->bytecodeIndex == codeOrigin.bytecodeIndex())
+ return byValInfo;
+ }
+ }
+ return nullptr;
+}
+
ByValInfo* CodeBlock::addByValInfo()
{
ConcurrentJSLocker locker(m_lock);
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (258873 => 258874)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2020-03-23 21:00:01 UTC (rev 258873)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2020-03-23 21:09:21 UTC (rev 258874)
@@ -312,9 +312,10 @@
StructureStubInfo* addStubInfo(AccessType);
- // O(n) operation. Use getStubInfoMap() unless you really only intend to get one
- // stub info.
+ // O(n) operation. Use getICStatusMap() unless you really only intend to get one stub info.
StructureStubInfo* findStubInfo(CodeOrigin);
+ // O(n) operation. Use getICStatusMap() unless you really only intend to get one by-val-info.
+ ByValInfo* findByValInfo(CodeOrigin);
ByValInfo* addByValInfo();
Modified: trunk/Source/_javascript_Core/dfg/DFGOSRExitCompilerCommon.cpp (258873 => 258874)
--- trunk/Source/_javascript_Core/dfg/DFGOSRExitCompilerCommon.cpp 2020-03-23 21:00:01 UTC (rev 258873)
+++ trunk/Source/_javascript_Core/dfg/DFGOSRExitCompilerCommon.cpp 2020-03-23 21:09:21 UTC (rev 258874)
@@ -154,8 +154,8 @@
MacroAssemblerCodePtr<JSEntryPtrTag> jumpTarget;
+ const Instruction& callInstruction = *baselineCodeBlockForCaller->instructions().at(callBytecodeIndex).ptr();
if (callerIsLLInt) {
- const Instruction& callInstruction = *baselineCodeBlockForCaller->instructions().at(callBytecodeIndex).ptr();
#define LLINT_RETURN_LOCATION(name) (callInstruction.isWide16() ? LLInt::getWide16CodePtr<JSEntryPtrTag>(name##_return_location) : (callInstruction.isWide32() ? LLInt::getWide32CodePtr<JSEntryPtrTag>(name##_return_location) : LLInt::getCodePtr<JSEntryPtrTag>(name##_return_location)))
switch (trueCallerCallKind) {
@@ -213,10 +213,23 @@
case InlineCallFrame::GetterCall:
case InlineCallFrame::SetterCall: {
- StructureStubInfo* stubInfo =
- baselineCodeBlockForCaller->findStubInfo(CodeOrigin(callBytecodeIndex));
+ if (callInstruction.opcodeID() == op_put_by_val) {
+ // We compile op_put_by_val as PutById and inlines SetterCall only when we found StructureStubInfo for this op_put_by_val.
+ // But still it is possible that we cannot find StructureStubInfo here. Let's consider the following scenario.
+ // 1. Baseline CodeBlock (A) is compiled.
+ // 2. (A) gets DFG (B).
+ // 3. Since (A) collects enough information for put_by_val, (B) can get StructureStubInfo from (A) and copmile it as inlined Setter call.
+ // 4. (A)'s JITData is destroyed since it is not executed. Then, (A) becomes LLInt.
+ // 5. The CodeBlock inlining (A) gets OSR exit. So (A) is executed and (A) eventually gets Baseline CodeBlock again.
+ // 6. (B) gets OSR exit. (B) attempts to search for StructureStubInfo in (A) for PutById (originally, put_by_val). But it does not exist since (A)'s JITData is cleared once.
+ ByValInfo* byValInfo = baselineCodeBlockForCaller->findByValInfo(CodeOrigin(callBytecodeIndex));
+ RELEASE_ASSERT(byValInfo);
+ jumpTarget = byValInfo->doneTarget.retagged<JSEntryPtrTag>();
+ break;
+ }
+
+ StructureStubInfo* stubInfo = baselineCodeBlockForCaller->findStubInfo(CodeOrigin(callBytecodeIndex));
RELEASE_ASSERT(stubInfo);
-
jumpTarget = stubInfo->doneLocation.retagged<JSEntryPtrTag>();
break;
}
Modified: trunk/Source/_javascript_Core/jit/JITOpcodes.cpp (258873 => 258874)
--- trunk/Source/_javascript_Core/jit/JITOpcodes.cpp 2020-03-23 21:00:01 UTC (rev 258873)
+++ trunk/Source/_javascript_Core/jit/JITOpcodes.cpp 2020-03-23 21:09:21 UTC (rev 258874)
@@ -1285,7 +1285,7 @@
patchBuffer.link(badType, byValInfo->slowPathTarget);
patchBuffer.link(slowCases, byValInfo->slowPathTarget);
- patchBuffer.link(done, byValInfo->badTypeDoneTarget);
+ patchBuffer.link(done, byValInfo->doneTarget);
byValInfo->stubRoutine = FINALIZE_CODE_FOR_STUB(
m_codeBlock, patchBuffer, JITStubRoutinePtrTag,
Modified: trunk/Source/_javascript_Core/jit/JITOpcodes32_64.cpp (258873 => 258874)
--- trunk/Source/_javascript_Core/jit/JITOpcodes32_64.cpp 2020-03-23 21:00:01 UTC (rev 258873)
+++ trunk/Source/_javascript_Core/jit/JITOpcodes32_64.cpp 2020-03-23 21:09:21 UTC (rev 258874)
@@ -1158,7 +1158,7 @@
patchBuffer.link(badType, byValInfo->slowPathTarget);
patchBuffer.link(slowCases, byValInfo->slowPathTarget);
- patchBuffer.link(done, byValInfo->badTypeDoneTarget);
+ patchBuffer.link(done, byValInfo->doneTarget);
byValInfo->stubRoutine = FINALIZE_CODE_FOR_STUB(
m_codeBlock, patchBuffer, JITStubRoutinePtrTag,
Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp (258873 => 258874)
--- trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2020-03-23 21:00:01 UTC (rev 258873)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2020-03-23 21:09:21 UTC (rev 258874)
@@ -1337,7 +1337,7 @@
LinkBuffer patchBuffer(*this, m_codeBlock);
patchBuffer.link(badType, byValInfo->slowPathTarget);
patchBuffer.link(slowCases, byValInfo->slowPathTarget);
- patchBuffer.link(done, byValInfo->badTypeDoneTarget);
+ patchBuffer.link(done, byValInfo->doneTarget);
if (needsLinkForWriteBarrier) {
ASSERT(removeCodePtrTag(m_calls.last().callee.executableAddress()) == removeCodePtrTag(operationWriteBarrierSlowPath));
patchBuffer.link(m_calls.last().from, m_calls.last().callee);
@@ -1377,7 +1377,7 @@
ConcurrentJSLocker locker(m_codeBlock->m_lock);
LinkBuffer patchBuffer(*this, m_codeBlock);
patchBuffer.link(slowCases, byValInfo->slowPathTarget);
- patchBuffer.link(doneCases, byValInfo->badTypeDoneTarget);
+ patchBuffer.link(doneCases, byValInfo->doneTarget);
if (!m_exceptionChecks.empty())
patchBuffer.link(m_exceptionChecks, byValInfo->exceptionHandler);
