- Revision
- 264857
- Author
- [email protected]
- Date
- 2020-07-24 14:29:40 -0700 (Fri, 24 Jul 2020)
Log Message
[JSC] DFG::AbstractValue::filterByValue should re-filter configured m_value via m_type
https://bugs.webkit.org/show_bug.cgi?id=214721
<rdar://problem/65882837>
Reviewed by Mark Lam.
JSTests:
* stress/speculation-from-structure-and-classinfo-should-agree.js: Added.
(bar):
(foo):
Source/_javascript_Core:
Let's consider the following case.
1. Input AbstractValue is saying SpecObjectOther.
2. We have CheckIsConstant with StringPrototype (which is SpecObjectOther | SpecStringObject in the current SpeculatedType).
3. We call filterByValue, which filters m_type by SpecObjectOther | SpecStringObject. But the filtered m_type is SpecObjectOther since (2)'s SpeculatedType is broader.
4. We store the given constant (StringPrototype) to m_value.
5. This AbstractValue's m_type is SpecObjectOther while its m_value's SpeculatedType is SpecObjectOther | SpecStringObject. Contradiction!
When setting m_value by filterByValue, we should filter m_value with m_type to ensure that m_value is the expected one.
This patch also avoid using SpecObjectOther | SpecStringObject for StringPrototype since we can return narrower type for that.
* bytecode/SpeculatedType.cpp:
(JSC::speculationFromStructure):
* dfg/DFGAbstractValue.cpp:
(JSC::DFG::AbstractValue::filterByValue):
(JSC::DFG::AbstractValue::filter):
* dfg/DFGPredictionPropagationPhase.cpp:
* runtime/StringObject.cpp:
(JSC::StringObject::finishCreation):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (264856 => 264857)
--- trunk/JSTests/ChangeLog 2020-07-24 21:03:23 UTC (rev 264856)
+++ trunk/JSTests/ChangeLog 2020-07-24 21:29:40 UTC (rev 264857)
@@ -1,3 +1,15 @@
+2020-07-24 Yusuke Suzuki <[email protected]>
+
+ [JSC] DFG::AbstractValue::filterByValue should re-filter configured m_value via m_type
+ https://bugs.webkit.org/show_bug.cgi?id=214721
+ <rdar://problem/65882837>
+
+ Reviewed by Mark Lam.
+
+ * stress/speculation-from-structure-and-classinfo-should-agree.js: Added.
+ (bar):
+ (foo):
+
2020-07-24 Alexey Shvayka <[email protected]>
JSON.parse should not modify non-configurable properties.
Added: trunk/JSTests/stress/speculation-from-structure-and-classinfo-should-agree.js (0 => 264857)
--- trunk/JSTests/stress/speculation-from-structure-and-classinfo-should-agree.js (rev 0)
+++ trunk/JSTests/stress/speculation-from-structure-and-classinfo-should-agree.js 2020-07-24 21:29:40 UTC (rev 264857)
@@ -0,0 +1,14 @@
+//@ runDefault("--forceEagerCompilation=1", "--minimumOptimizationDelay=10", "--maximumOptimizationDelay=10", "--useConcurrentJIT=0")
+
+function bar(a0) {
+ '' instanceof a0;
+}
+
+function foo() {
+ optimizeNextInvocation(foo);
+ bar(String);
+ bar(Symbol);
+}
+
+for (let i=0; i<11; i++)
+ foo();
Modified: trunk/Source/_javascript_Core/ChangeLog (264856 => 264857)
--- trunk/Source/_javascript_Core/ChangeLog 2020-07-24 21:03:23 UTC (rev 264856)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-07-24 21:29:40 UTC (rev 264857)
@@ -1,3 +1,31 @@
+2020-07-24 Yusuke Suzuki <[email protected]>
+
+ [JSC] DFG::AbstractValue::filterByValue should re-filter configured m_value via m_type
+ https://bugs.webkit.org/show_bug.cgi?id=214721
+ <rdar://problem/65882837>
+
+ Reviewed by Mark Lam.
+
+ Let's consider the following case.
+
+ 1. Input AbstractValue is saying SpecObjectOther.
+ 2. We have CheckIsConstant with StringPrototype (which is SpecObjectOther | SpecStringObject in the current SpeculatedType).
+ 3. We call filterByValue, which filters m_type by SpecObjectOther | SpecStringObject. But the filtered m_type is SpecObjectOther since (2)'s SpeculatedType is broader.
+ 4. We store the given constant (StringPrototype) to m_value.
+ 5. This AbstractValue's m_type is SpecObjectOther while its m_value's SpeculatedType is SpecObjectOther | SpecStringObject. Contradiction!
+
+ When setting m_value by filterByValue, we should filter m_value with m_type to ensure that m_value is the expected one.
+ This patch also avoid using SpecObjectOther | SpecStringObject for StringPrototype since we can return narrower type for that.
+
+ * bytecode/SpeculatedType.cpp:
+ (JSC::speculationFromStructure):
+ * dfg/DFGAbstractValue.cpp:
+ (JSC::DFG::AbstractValue::filterByValue):
+ (JSC::DFG::AbstractValue::filter):
+ * dfg/DFGPredictionPropagationPhase.cpp:
+ * runtime/StringObject.cpp:
+ (JSC::StringObject::finishCreation):
+
2020-07-24 Alexey Shvayka <[email protected]>
JSON.parse should not modify non-configurable properties.
Modified: trunk/Source/_javascript_Core/bytecode/SpeculatedType.cpp (264856 => 264857)
--- trunk/Source/_javascript_Core/bytecode/SpeculatedType.cpp 2020-07-24 21:03:23 UTC (rev 264856)
+++ trunk/Source/_javascript_Core/bytecode/SpeculatedType.cpp 2020-07-24 21:29:40 UTC (rev 264857)
@@ -517,21 +517,32 @@
SpeculatedType speculationFromStructure(Structure* structure)
{
SpeculatedType filteredResult = SpecNone;
- if (structure->typeInfo().type() == StringType)
+ switch (structure->typeInfo().type()) {
+ case StringType:
filteredResult = SpecString;
- else if (structure->typeInfo().type() == SymbolType)
+ break;
+ case SymbolType:
filteredResult = SpecSymbol;
- else if (structure->typeInfo().type() == HeapBigIntType)
+ break;
+ case HeapBigIntType:
filteredResult = SpecHeapBigInt;
- else if (structure->typeInfo().type() == DerivedArrayType)
+ break;
+ case DerivedArrayType:
filteredResult = SpecDerivedArray;
- else if (structure->typeInfo().type() == ArrayType)
+ break;
+ case ArrayType:
filteredResult = SpecArray;
- else if (structure->typeInfo().type() == StringObjectType)
+ break;
+ case StringObjectType:
filteredResult = SpecStringObject;
- else
+ break;
+ // We do not want to accept String.prototype in StringObjectUse, so that we do not include it as SpecStringObject.
+ case DerivedStringObjectType:
+ filteredResult = SpecObjectOther;
+ break;
+ default:
return speculationFromClassInfoInheritance(structure->classInfo());
-
+ }
ASSERT(filteredResult);
ASSERT(isSubtypeSpeculation(filteredResult, speculationFromClassInfoInheritance(structure->classInfo())));
return filteredResult;
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractValue.cpp (264856 => 264857)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractValue.cpp 2020-07-24 21:03:23 UTC (rev 264856)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractValue.cpp 2020-07-24 21:29:40 UTC (rev 264857)
@@ -321,8 +321,15 @@
FiltrationResult AbstractValue::filterByValue(const FrozenValue& value)
{
FiltrationResult result = filter(speculationFromValue(value.value()));
- if (m_type)
+ if (m_type) {
m_value = value.value();
+ // It is possible that SpeculatedType from value is broader than original m_type.
+ // The filter operation can only keep m_type as is or make it narrower.
+ // As a result, the SpeculatedType from m_value can become broader than m_type. This breaks an invariant.
+ // When setting m_value after filtering, we should filter m_value with m_type.
+ filterValueByType();
+ }
+ checkConsistency();
return result;
}
@@ -353,6 +360,11 @@
if (!m_value) {
// We previously didn't prove a value but now we have done so.
m_value = other.m_value;
+ // It is possible that SpeculatedType from other.m_value is broader than original m_type.
+ // The filter operation can only keep m_type as is or make it narrower.
+ // As a result, the SpeculatedType from m_value can become broader than m_type. This breaks an invariant.
+ // When setting m_value after filtering, we should filter m_value with m_type.
+ filterValueByType();
return FiltrationOK;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (264856 => 264857)
--- trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2020-07-24 21:03:23 UTC (rev 264856)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2020-07-24 21:29:40 UTC (rev 264857)
@@ -588,9 +588,12 @@
if (isStrictMode)
changed |= mergePrediction(node->getHeapPrediction());
else if (prediction) {
+ SpeculatedType originalPrediction = prediction;
if (prediction & ~SpecObject) {
// Wrapper objects are created only in sloppy mode.
prediction &= SpecObject;
+ if (originalPrediction & SpecString)
+ prediction = mergeSpeculations(prediction, SpecStringObject);
prediction = mergeSpeculations(prediction, SpecObjectOther);
}
changed |= mergePrediction(prediction);
Modified: trunk/Source/_javascript_Core/runtime/StringObject.cpp (264856 => 264857)
--- trunk/Source/_javascript_Core/runtime/StringObject.cpp 2020-07-24 21:03:23 UTC (rev 264856)
+++ trunk/Source/_javascript_Core/runtime/StringObject.cpp 2020-07-24 21:29:40 UTC (rev 264857)
@@ -41,6 +41,7 @@
Base::finishCreation(vm);
ASSERT(inherits(vm, info()));
setInternalValue(vm, string);
+ ASSERT_WITH_MESSAGE(type() == StringObjectType || type() == DerivedStringObjectType, "Instance inheriting StringObject should have DerivedStringObjectType");
}
bool StringObject::getOwnPropertySlot(JSObject* cell, JSGlobalObject* globalObject, PropertyName propertyName, PropertySlot& slot)
