Diff
Modified: branches/safari-612-branch/JSTests/ChangeLog (285540 => 285541)
--- branches/safari-612-branch/JSTests/ChangeLog 2021-11-10 00:03:09 UTC (rev 285540)
+++ branches/safari-612-branch/JSTests/ChangeLog 2021-11-10 00:29:58 UTC (rev 285541)
@@ -1,3 +1,13 @@
+2021-11-02 Saam Barati <sbar...@apple.com>
+
+ EnumeratorGetByVal for IndexedMode+OwnStructureMode doesn't always recover the property name
+ https://bugs.webkit.org/show_bug.cgi?id=231321
+ <rdar://problem/84211697>
+
+ Reviewed by Yusuke Suzuki.
+
+ * stress/enumerator-get-by-val-needs-to-recover-property-name.js: Added.
+
2021-11-08 Kocsen Chung <kocsen_ch...@apple.com>
Cherry-pick r283938. rdar://problem/85166798
Added: branches/safari-612-branch/JSTests/stress/enumerator-get-by-val-needs-to-recover-property-name.js (0 => 285541)
--- branches/safari-612-branch/JSTests/stress/enumerator-get-by-val-needs-to-recover-property-name.js (rev 0)
+++ branches/safari-612-branch/JSTests/stress/enumerator-get-by-val-needs-to-recover-property-name.js 2021-11-10 00:29:58 UTC (rev 285541)
@@ -0,0 +1,32 @@
+//@ runDefault("--validateOptions=true", "--useConcurrentJIT=false", "--useConcurrentGC=false", "--thresholdForJITSoon=10", "--thresholdForJITAfterWarmUp=10", "--thresholdForOptimizeAfterWarmUp=100", "--thresholdForOptimizeAfterLongWarmUp=100", "--thresholdForOptimizeSoon=100", "--thresholdForFTLOptimizeAfterWarmUp=1000", "--thresholdForFTLOptimizeSoon=1000", "--validateBCE=true", "--useFTLJIT=true")
+
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+
+function main() {
+ let result;
+ const v35 = [0, 0, {b:"AAAAA"}];
+
+ async function v36(arr) {
+ edenGC(); // This is needed
+ for (let i = 0; i < 2; i++) {
+ const v201 = `
+ var someVar; // this is needed
+
+ for (let j = 0; j < 60000; j++) { }
+
+ const v222 = {"__proto__":[[]], "a":0, "b":0};
+ for (const prop in v222) {
+ result = arr[prop];
+ v222.__proto__ = {};
+ }
+ `;
+ eval(v201); // moving code out of eval breaks differential
+ }
+ }
+ v35.filter(v36);
+ assert(result === "AAAAA");
+}
+main();
Modified: branches/safari-612-branch/Source/_javascript_Core/ChangeLog (285540 => 285541)
--- branches/safari-612-branch/Source/_javascript_Core/ChangeLog 2021-11-10 00:03:09 UTC (rev 285540)
+++ branches/safari-612-branch/Source/_javascript_Core/ChangeLog 2021-11-10 00:29:58 UTC (rev 285541)
@@ -1,3 +1,21 @@
+2021-11-02 Saam Barati <sbar...@apple.com>
+
+ EnumeratorGetByVal for IndexedMode+OwnStructureMode doesn't always recover the property name
+ https://bugs.webkit.org/show_bug.cgi?id=231321
+ <rdar://problem/84211697>
+
+ Reviewed by Yusuke Suzuki.
+
+ When running an EnumeratorGetByVal in IndexedMode+OwnStructureMode, we may
+ go to the slow path. However, we were incorrectly going to the slow path
+ before recovering the actual property name. Instead, we were passing in
+ the integer index value to the get by val.
+
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileEnumeratorGetByVal):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
+
2021-11-08 Kocsen Chung <kocsen_ch...@apple.com>
Cherry-pick r283938. rdar://problem/85166798
Modified: branches/safari-612-branch/Source/_javascript_Core/dfg/DFGOperations.cpp (285540 => 285541)
--- branches/safari-612-branch/Source/_javascript_Core/dfg/DFGOperations.cpp 2021-11-10 00:03:09 UTC (rev 285540)
+++ branches/safari-612-branch/Source/_javascript_Core/dfg/DFGOperations.cpp 2021-11-10 00:29:58 UTC (rev 285541)
@@ -2506,7 +2506,7 @@
return result;
}
-JSC_DEFINE_JIT_OPERATION(operationEnumeratorRecoverNameAndGetByVal, EncodedJSValue, (JSGlobalObject* globalObject, JSCell* base, uint32_t index, JSPropertyNameEnumerator* enumerator))
+JSC_DEFINE_JIT_OPERATION(operationEnumeratorRecoverNameAndGetByVal, EncodedJSValue, (JSGlobalObject* globalObject, EncodedJSValue baseValue, uint32_t index, JSPropertyNameEnumerator* enumerator))
{
VM& vm = globalObject->vm();
CallFrame* callFrame = DECLARE_CALL_FRAME(vm);
@@ -2517,7 +2517,8 @@
PropertyName propertyName = string->toIdentifier(globalObject);
// This should only really return for TerminationException since we know string is backed by a UUID.
RETURN_IF_EXCEPTION(scope, { });
- JSObject* object = base->toObject(globalObject);
+ JSValue base = JSValue::decode(baseValue);
+ JSObject* object = base.toObject(globalObject);
RETURN_IF_EXCEPTION(scope, { });
RELEASE_AND_RETURN(scope, JSValue::encode(object->get(globalObject, propertyName)));
Modified: branches/safari-612-branch/Source/_javascript_Core/dfg/DFGOperations.h (285540 => 285541)
--- branches/safari-612-branch/Source/_javascript_Core/dfg/DFGOperations.h 2021-11-10 00:03:09 UTC (rev 285540)
+++ branches/safari-612-branch/Source/_javascript_Core/dfg/DFGOperations.h 2021-11-10 00:29:58 UTC (rev 285541)
@@ -111,7 +111,7 @@
JSC_DECLARE_JIT_OPERATION(operationEnumeratorNextUpdatePropertyName, JSString*, (JSGlobalObject*, uint32_t, int32_t, JSPropertyNameEnumerator*));
JSC_DECLARE_JIT_OPERATION(operationEnumeratorInByVal, EncodedJSValue, (JSGlobalObject*, EncodedJSValue, EncodedJSValue, uint32_t, int32_t));
JSC_DECLARE_JIT_OPERATION(operationEnumeratorHasOwnProperty, EncodedJSValue, (JSGlobalObject*, EncodedJSValue, EncodedJSValue, uint32_t, int32_t));
-JSC_DECLARE_JIT_OPERATION(operationEnumeratorRecoverNameAndGetByVal, EncodedJSValue, (JSGlobalObject*, JSCell*, uint32_t, JSPropertyNameEnumerator*));
+JSC_DECLARE_JIT_OPERATION(operationEnumeratorRecoverNameAndGetByVal, EncodedJSValue, (JSGlobalObject*, EncodedJSValue, uint32_t, JSPropertyNameEnumerator*));
JSC_DECLARE_JIT_OPERATION(operationNewRegexpWithLastIndex, JSCell*, (JSGlobalObject*, JSCell*, EncodedJSValue));
JSC_DECLARE_JIT_OPERATION(operationNewArray, char*, (JSGlobalObject*, Structure*, void*, size_t));
Modified: branches/safari-612-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (285540 => 285541)
--- branches/safari-612-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2021-11-10 00:03:09 UTC (rev 285540)
+++ branches/safari-612-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2021-11-10 00:29:58 UTC (rev 285541)
@@ -13699,7 +13699,7 @@
JSValueRegs resultRegs;
GPRReg indexGPR;
GPRReg enumeratorGPR;
- MacroAssembler::Jump badStructureSlowPath;
+ MacroAssembler::JumpList recoverGenericCase;
compileGetByVal(node, scopedLambda<std::tuple<JSValueRegs, DataFormat>(DataFormat)>([&] (DataFormat) {
Edge storageEdge = m_graph.varArgChild(node, 2);
@@ -13728,12 +13728,15 @@
storageGPR = storage.gpr();
MacroAssembler::JumpList notFastNamedCases;
+ // FIXME: Maybe we should have a better way to represent IndexedMode+OwnStructureMode?
+ bool indexedAndOwnStructureMode = m_graph.varArgChild(node, 1).node() == m_graph.varArgChild(node, 3).node();
+ MacroAssembler::JumpList& genericOrRecoverCase = indexedAndOwnStructureMode ? recoverGenericCase : notFastNamedCases;
// FIXME: We shouldn't generate this code if we know base is not an object.
notFastNamedCases.append(m_jit.branchTest32(MacroAssembler::NonZero, modeGPR, TrustedImm32(JSPropertyNameEnumerator::IndexedMode | JSPropertyNameEnumerator::GenericMode)));
{
if (!m_state.forNode(baseEdge).isType(SpecCell))
- notFastNamedCases.append(m_jit.branchIfNotCell(baseCellGPR));
+ genericOrRecoverCase.append(m_jit.branchIfNotCell(baseCellGPR));
// Check the structure
// FIXME: If we know there's only one structure for base we can just embed it here.
@@ -13745,11 +13748,7 @@
MacroAssembler::Address(
enumeratorGPR, JSPropertyNameEnumerator::cachedStructureIDOffset()));
- // FIXME: Maybe we should have a better way to represent Indexed+Named?
- if (m_graph.varArgChild(node, 1).node() == m_graph.varArgChild(node, 3).node())
- badStructureSlowPath = badStructure;
- else
- notFastNamedCases.append(badStructure);
+ genericOrRecoverCase.append(badStructure);
// Compute the offset
// If index is less than the enumerator's cached inline storage, then it's an inline access
@@ -13781,8 +13780,8 @@
// FIXME: This is kinda hacky...
ASSERT(generationInfo(node).jsValueRegs() == resultRegs && generationInfo(node).registerFormat() == DataFormatJS);
- if (badStructureSlowPath.isSet())
- addSlowPathGenerator(slowPathCall(badStructureSlowPath, this, operationEnumeratorRecoverNameAndGetByVal, resultRegs, TrustedImmPtr::weakPointer(m_graph, m_graph.globalObjectFor(node->origin.semantic)), baseCellGPR, indexGPR, enumeratorGPR));
+ if (!recoverGenericCase.empty())
+ addSlowPathGenerator(slowPathCall(recoverGenericCase, this, operationEnumeratorRecoverNameAndGetByVal, resultRegs, TrustedImmPtr::weakPointer(m_graph, m_graph.globalObjectFor(node->origin.semantic)), baseCellGPR, indexGPR, enumeratorGPR));
doneCases.link(&m_jit);
};
Modified: branches/safari-612-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (285540 => 285541)
--- branches/safari-612-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2021-11-10 00:03:09 UTC (rev 285540)
+++ branches/safari-612-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2021-11-10 00:29:58 UTC (rev 285541)
@@ -13572,7 +13572,15 @@
LBasicBlock outOfLineLoadBlock = m_out.newBlock();
LBasicBlock genericICBlock = m_out.newBlock();
LBasicBlock continuation = m_out.newBlock();
+ LBasicBlock genericOrRecover;
+ // FIXME: This is not the cleanest way to say we're using IndexedMode+OwnStructureMode mode.
+ bool indexedAndOwnStructureMode = indexEdge.node() == propertyNameEdge.node();
+ if (indexedAndOwnStructureMode)
+ genericOrRecover = m_out.newBlock();
+ else
+ genericOrRecover = genericICBlock;
+
Vector<ValueFromBlock, 4> results;
LValue isNotNamed = m_out.testNonZero32(mode, m_out.constInt32(JSPropertyNameEnumerator::IndexedMode | JSPropertyNameEnumerator::GenericMode));
@@ -13579,7 +13587,7 @@
m_out.branch(isNotNamed, unsure(genericICBlock), unsure(checkIsCellBlock));
m_out.appendTo(checkIsCellBlock);
- m_out.branch(isCell(base, provenType(baseEdge)), usually(checkStructureBlock), rarely(genericICBlock));
+ m_out.branch(isCell(base, provenType(baseEdge)), usually(checkStructureBlock), rarely(genericOrRecover));
m_out.appendTo(checkStructureBlock);
LValue structureID;
@@ -13591,16 +13599,8 @@
LValue hasEnumeratorStructure = m_out.equal(structureID, m_out.load32(enumerator, m_heaps.JSPropertyNameEnumerator_cachedStructureID));
- if (indexEdge.node() == propertyNameEdge.node()) {
- JSGlobalObject* globalObject = m_graph.globalObjectFor(m_origin.semantic);
- LBasicBlock badStructureSlowPath = m_out.newBlock();
- m_out.branch(hasEnumeratorStructure, usually(checkInlineOrOutOfLineBlock), rarely(genericICBlock));
+ m_out.branch(hasEnumeratorStructure, usually(checkInlineOrOutOfLineBlock), rarely(genericOrRecover));
- m_out.appendTo(badStructureSlowPath);
- results.append(m_out.anchor(vmCall(Int64, operationEnumeratorRecoverNameAndGetByVal, weakPointer(globalObject), base, index, enumerator)));
- } else
- m_out.branch(hasEnumeratorStructure, usually(checkInlineOrOutOfLineBlock), rarely(genericICBlock));
-
m_out.appendTo(checkInlineOrOutOfLineBlock);
LValue inlineCapacity = nullptr;
bool hasNoOutOfLineProperties = false;
@@ -13644,6 +13644,12 @@
results.append(m_out.anchor(genericResult));
m_out.jump(continuation);
+ if (indexedAndOwnStructureMode) {
+ m_out.appendTo(genericOrRecover);
+ results.append(m_out.anchor(vmCall(Int64, operationEnumeratorRecoverNameAndGetByVal, weakPointer(m_graph.globalObjectFor(m_origin.semantic)), base, index, enumerator)));
+ m_out.jump(continuation);
+ }
+
m_out.appendTo(continuation);
ASSERT(results.size());
LValue result = m_out.phi(Int64, results);
