Fix endless OSR exits when creating a rope that contains an object that ToPrimitive's to a number.
https://bugs.webkit.org/show_bug.cgi?id=150583
Reviewed by Benjamin Poulain.
Source/JavaScriptCore:
Before we assumed that the result of ToPrimitive on any object was a string.
This had a couple of negative effects. First, the result ToPrimitive on an
object can be overridden to be any primitive type. In fact, as of ES6, ToPrimitive,
when part of a addition expression, will type hint a number value. Second, even after
repeatedly exiting with a bad type we would continue to think that the result
of ToPrimitive would be a string so we continue to convert StrCats into MakeRope.
The fix is to make Prediction Propagation match the behavior of Fixup and move
canOptimizeStringObjectAccess to DFGGraph.
* bytecode/SpeculatedType.h:
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::attemptToForceStringArrayModeByToStringConversion):
(JSC::DFG::FixupPhase::fixupToPrimitive):
(JSC::DFG::FixupPhase::fixupToStringOrCallStringConstructor):
(JSC::DFG::FixupPhase::attemptToMakeFastStringAdd):
(JSC::DFG::FixupPhase::isStringPrototypeMethodSane): Deleted.
(JSC::DFG::FixupPhase::canOptimizeStringObjectAccess): Deleted.
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::isStringPrototypeMethodSane):
(JSC::DFG::Graph::canOptimizeStringObjectAccess):
* dfg/DFGGraph.h:
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::resultOfToPrimitive):
(JSC::DFG::resultOfToPrimitive): Deleted.
* bytecode/SpeculatedType.h:
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::attemptToForceStringArrayModeByToStringConversion):
(JSC::DFG::FixupPhase::fixupToPrimitive):
(JSC::DFG::FixupPhase::fixupToStringOrCallStringConstructor):
(JSC::DFG::FixupPhase::attemptToMakeFastStringAdd):
(JSC::DFG::FixupPhase::isStringPrototypeMethodSane): Deleted.
(JSC::DFG::FixupPhase::canOptimizeStringObjectAccess): Deleted.
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::isStringPrototypeMethodSane):
(JSC::DFG::Graph::canOptimizeStringObjectAccess):
* dfg/DFGGraph.h:
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::resultOfToPrimitive):
(JSC::DFG::resultOfToPrimitive): Deleted.
* tests/stress/string-rope-with-custom-valueof.js: Added.
(catNumber):
(number.valueOf):
(catBool):
(bool.valueOf):
(catUndefined):
(undef.valueOf):
(catRandom):
(random.valueOf):
LayoutTests:
Created a regression test to look for OSRing in string concatenation when
valueOf returns a non-string primitive.
* js/regress/script-tests/string-rope-with-object.js: Added.
(body.f):
(body.String.prototype.valueOf):
(body.bar.valueOf):
(body):
* js/regress/string-rope-with-object-expected.txt: Added.
* js/regress/string-rope-with-object.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@192034 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index 491fbc1..c4f36f9 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -1447,7 +1447,7 @@
{
ASSERT(arrayMode == ArrayMode(Array::Generic));
- if (!canOptimizeStringObjectAccess(node->origin.semantic))
+ if (!m_graph.canOptimizeStringObjectAccess(node->origin.semantic))
return;
createToString<useKind>(node, node->child1());
@@ -1529,14 +1529,14 @@
}
if (node->child1()->shouldSpeculateStringObject()
- && canOptimizeStringObjectAccess(node->origin.semantic)) {
+ && m_graph.canOptimizeStringObjectAccess(node->origin.semantic)) {
fixEdge<StringObjectUse>(node->child1());
node->convertToToString();
return;
}
if (node->child1()->shouldSpeculateStringOrStringObject()
- && canOptimizeStringObjectAccess(node->origin.semantic)) {
+ && m_graph.canOptimizeStringObjectAccess(node->origin.semantic)) {
fixEdge<StringOrStringObjectUse>(node->child1());
node->convertToToString();
return;
@@ -1552,13 +1552,13 @@
}
if (node->child1()->shouldSpeculateStringObject()
- && canOptimizeStringObjectAccess(node->origin.semantic)) {
+ && m_graph.canOptimizeStringObjectAccess(node->origin.semantic)) {
fixEdge<StringObjectUse>(node->child1());
return;
}
if (node->child1()->shouldSpeculateStringOrStringObject()
- && canOptimizeStringObjectAccess(node->origin.semantic)) {
+ && m_graph.canOptimizeStringObjectAccess(node->origin.semantic)) {
fixEdge<StringOrStringObjectUse>(node->child1());
return;
}
@@ -1577,7 +1577,7 @@
[&] (Edge& edge) {
if (edge->shouldSpeculateString())
return;
- if (canOptimizeStringObjectAccess(node->origin.semantic)) {
+ if (m_graph.canOptimizeStringObjectAccess(node->origin.semantic)) {
if (edge->shouldSpeculateStringObject())
return;
if (edge->shouldSpeculateStringOrStringObject())
@@ -1595,7 +1595,7 @@
convertStringAddUse<StringUse>(node, edge);
return;
}
- ASSERT(canOptimizeStringObjectAccess(node->origin.semantic));
+ ASSERT(m_graph.canOptimizeStringObjectAccess(node->origin.semantic));
if (edge->shouldSpeculateStringObject()) {
convertStringAddUse<StringObjectUse>(node, edge);
return;
@@ -1610,65 +1610,7 @@
convertToMakeRope(node);
return true;
}
-
- bool isStringPrototypeMethodSane(
- JSObject* stringPrototype, Structure* stringPrototypeStructure, UniquedStringImpl* uid)
- {
- unsigned attributesUnused;
- PropertyOffset offset =
- stringPrototypeStructure->getConcurrently(uid, attributesUnused);
- if (!isValidOffset(offset))
- return false;
-
- JSValue value = m_graph.tryGetConstantProperty(
- stringPrototype, stringPrototypeStructure, offset);
- if (!value)
- return false;
-
- JSFunction* function = jsDynamicCast<JSFunction*>(value);
- if (!function)
- return false;
-
- if (function->executable()->intrinsicFor(CodeForCall) != StringPrototypeValueOfIntrinsic)
- return false;
-
- return true;
- }
-
- bool canOptimizeStringObjectAccess(const CodeOrigin& codeOrigin)
- {
- if (m_graph.hasExitSite(codeOrigin, NotStringObject))
- return false;
-
- Structure* stringObjectStructure = m_graph.globalObjectFor(codeOrigin)->stringObjectStructure();
- m_graph.registerStructure(stringObjectStructure);
- ASSERT(stringObjectStructure->storedPrototype().isObject());
- ASSERT(stringObjectStructure->storedPrototype().asCell()->classInfo() == StringPrototype::info());
- FrozenValue* stringPrototypeObjectValue =
- m_graph.freeze(stringObjectStructure->storedPrototype());
- StringPrototype* stringPrototypeObject =
- stringPrototypeObjectValue->dynamicCast<StringPrototype*>();
- Structure* stringPrototypeStructure = stringPrototypeObjectValue->structure();
- if (m_graph.registerStructure(stringPrototypeStructure) != StructureRegisteredAndWatched)
- return false;
-
- if (stringPrototypeStructure->isDictionary())
- return false;
-
- // We're being conservative here. We want DFG's ToString on StringObject to be
- // used in both numeric contexts (that would call valueOf()) and string contexts
- // (that would call toString()). We don't want the DFG to have to distinguish
- // between the two, just because that seems like it would get confusing. So we
- // just require both methods to be sane.
- if (!isStringPrototypeMethodSane(stringPrototypeObject, stringPrototypeStructure, vm().propertyNames->valueOf.impl()))
- return false;
- if (!isStringPrototypeMethodSane(stringPrototypeObject, stringPrototypeStructure, vm().propertyNames->toString.impl()))
- return false;
-
- return true;
- }
-
void fixupGetAndSetLocalsInBlock(BasicBlock* block)
{
if (!block)
diff --git a/Source/JavaScriptCore/dfg/DFGGraph.cpp b/Source/JavaScriptCore/dfg/DFGGraph.cpp
index 0c80b67..50c7e86 100644
--- a/Source/JavaScriptCore/dfg/DFGGraph.cpp
+++ b/Source/JavaScriptCore/dfg/DFGGraph.cpp
@@ -1479,6 +1479,59 @@
return MethodOfGettingAValueProfile();
}
+bool Graph::isStringPrototypeMethodSane(JSObject* stringPrototype, Structure* stringPrototypeStructure, UniquedStringImpl* uid)
+{
+ unsigned attributesUnused;
+ PropertyOffset offset = stringPrototypeStructure->getConcurrently(uid, attributesUnused);
+ if (!isValidOffset(offset))
+ return false;
+
+ JSValue value = tryGetConstantProperty(stringPrototype, stringPrototypeStructure, offset);
+ if (!value)
+ return false;
+
+ JSFunction* function = jsDynamicCast<JSFunction*>(value);
+ if (!function)
+ return false;
+
+ if (function->executable()->intrinsicFor(CodeForCall) != StringPrototypeValueOfIntrinsic)
+ return false;
+
+ return true;
+}
+
+bool Graph::canOptimizeStringObjectAccess(const CodeOrigin& codeOrigin)
+{
+ if (hasExitSite(codeOrigin, NotStringObject))
+ return false;
+
+ Structure* stringObjectStructure = globalObjectFor(codeOrigin)->stringObjectStructure();
+ registerStructure(stringObjectStructure);
+ ASSERT(stringObjectStructure->storedPrototype().isObject());
+ ASSERT(stringObjectStructure->storedPrototype().asCell()->classInfo() == StringPrototype::info());
+
+ FrozenValue* stringPrototypeObjectValue = freeze(stringObjectStructure->storedPrototype());
+ StringPrototype* stringPrototypeObject = stringPrototypeObjectValue->dynamicCast<StringPrototype*>();
+ Structure* stringPrototypeStructure = stringPrototypeObjectValue->structure();
+ if (registerStructure(stringPrototypeStructure) != StructureRegisteredAndWatched)
+ return false;
+
+ if (stringPrototypeStructure->isDictionary())
+ return false;
+
+ // We're being conservative here. We want DFG's ToString on StringObject to be
+ // used in both numeric contexts (that would call valueOf()) and string contexts
+ // (that would call toString()). We don't want the DFG to have to distinguish
+ // between the two, just because that seems like it would get confusing. So we
+ // just require both methods to be sane.
+ if (!isStringPrototypeMethodSane(stringPrototypeObject, stringPrototypeStructure, m_vm.propertyNames->valueOf.impl()))
+ return false;
+ if (!isStringPrototypeMethodSane(stringPrototypeObject, stringPrototypeStructure, m_vm.propertyNames->toString.impl()))
+ return false;
+
+ return true;
+}
+
} } // namespace JSC::DFG
#endif // ENABLE(DFG_JIT)
diff --git a/Source/JavaScriptCore/dfg/DFGGraph.h b/Source/JavaScriptCore/dfg/DFGGraph.h
index 7ca0ac7..1ff1b35 100644
--- a/Source/JavaScriptCore/dfg/DFGGraph.h
+++ b/Source/JavaScriptCore/dfg/DFGGraph.h
@@ -318,6 +318,8 @@
&& negate->canSpeculateInt52(pass);
}
+ bool canOptimizeStringObjectAccess(const CodeOrigin&);
+
bool roundShouldSpeculateInt32(Node* arithRound, PredictionPass pass)
{
ASSERT(arithRound->op() == ArithRound);
@@ -913,7 +915,9 @@
bool m_hasDebuggerEnabled;
bool m_hasExceptionHandlers { false };
private:
-
+
+ bool isStringPrototypeMethodSane(JSObject* stringPrototype, Structure* stringPrototypeStructure, UniquedStringImpl*);
+
void handleSuccessor(Vector<BasicBlock*, 16>& worklist, BasicBlock*, BasicBlock* successor);
AddSpeculationMode addImmediateShouldSpeculateInt32(Node* add, bool variableShouldSpeculateInt32, Node* operand, Node*immediate, RareCaseProfilingSource source)
diff --git a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
index d72da97..e65f6e4 100644
--- a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
@@ -34,17 +34,6 @@
namespace JSC { namespace DFG {
-SpeculatedType resultOfToPrimitive(SpeculatedType type)
-{
- if (type & SpecObject) {
- // Objects get turned into strings. So if the input has hints of objectness,
- // the output will have hinsts of stringiness.
- return mergeSpeculations(type & ~SpecObject, SpecString);
- }
-
- return type;
-}
-
class PredictionPropagationPhase : public Phase {
public:
PredictionPropagationPhase(Graph& graph)
@@ -908,6 +897,20 @@
for (unsigned i = 0; i < m_graph.m_argumentPositions.size(); ++i)
m_changed |= m_graph.m_argumentPositions[i].mergeArgumentPredictionAwareness();
}
+
+ SpeculatedType resultOfToPrimitive(SpeculatedType type)
+ {
+ if (type & SpecObject) {
+ // We try to be optimistic here about StringObjects since it's unlikely that
+ // someone overrides the valueOf or toString methods.
+ if (type & SpecStringObject && m_graph.canOptimizeStringObjectAccess(m_currentNode->origin.semantic))
+ return mergeSpeculations(type & ~SpecObject, SpecString);
+
+ return mergeSpeculations(type & ~SpecObject, SpecPrimitive);
+ }
+
+ return type;
+ }
Node* m_currentNode;
bool m_changed;
