StringObjectUse should not be a structure check for the original string object structure
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 17 Jan 2019 17:50:27 +0000 (17:50 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 17 Jan 2019 17:50:27 +0000 (17:50 +0000)
commit0485d32103e5efa69de1249930d85603d1f9ac7b
treecca9ccb636585e8f6c0882cd32f87fe5048215c4
parente28b7bfe87ea07b6064f2e8799ced213dcf439fa
StringObjectUse should not be a structure check for the original string object structure
https://bugs.webkit.org/show_bug.cgi?id=193483
<rdar://problem/47280522>

Reviewed by Yusuke Suzuki.

JSTests:

* stress/cant-eliminate-string-object-structure-check-when-string-object-is-proven.js: Added.
(foo):
(a.valueOf.0):

Source/JavaScriptCore:

Prior to this patch, the use kind for StringObjectUse implied that we
do a StructureCheck on the input operand for the *original* StringObject
structure. This is generally not how we use UseKinds, so it's no surprise
that this is buggy. A UseKind should map to a set of SpeculatedTypes, not an
actual set of structures. This patch changes the meaning of StringObjectUse
to mean an object where jsDynamicCast<StringObject*> would succeed.

This patch also fixes a bug that was caused by the old and weird usage of the
UseKind to mean StructureCheck. Consider a program like this:
```
S1 = Original StringObject structure
S2 = Original StringObject structure with the field "f" added

a: GetLocal()
b: CheckStructure(@a, {S2})
c: ToString(StringObject:@a)
```

According to AI, in the above program, we would exit at @c, since
StringObject:@a implies a structure check of {S1}, and the intersection
of {S1} and {S2} is {}. So, we'd convert the program to be:
```
a: GetLocal()
b: CheckStructure(@a, {S2})
c: Check(StringObject:@a)
d: Unreachable
```

However, AI would set the proof status of the StringObject:@a edge
to be proven, since the SpeculatedType for @a is SpecStringObject.
This was incorrect of AI to do because the SpeculatedType itself
didn't capture the full power of StringObjectUse. However, having
a UseKind mean CheckStructure is weird precisely because what AI was
doing is a natural fit to how we typically we think about UseKinds.

So the above program would then incorrectly be converted to this, and
we'd crash when reaching the Unreachable node:
```
a: GetLocal()
b: CheckStructure(@a, {S2})
d: Unreachable
```

This patch makes it so that StringObjectUse just means that the object that
filters through a StringObjectUse check must !!jsDynamicCast<StringObject*>.
This is now in line with all other UseKinds. It also lets us simplify a bunch
of other code that had weird checks for the StringObjectUse UseKind.

This patch also makes it so that anywhere where we used to rely on
StringObjectUse implying a structure check we actually emit an explicit
CheckStructure node.

* JavaScriptCore.xcodeproj/project.pbxproj:
* bytecode/ExitKind.cpp:
(JSC::exitKindToString):
* bytecode/ExitKind.h:
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGCSEPhase.cpp:
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGEdgeUsesStructure.h: Removed.
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::attemptToForceStringArrayModeByToStringConversion):
(JSC::DFG::FixupPhase::addCheckStructureForOriginalStringObjectUse):
(JSC::DFG::FixupPhase::fixupToPrimitive):
(JSC::DFG::FixupPhase::fixupToStringOrCallStringConstructor):
(JSC::DFG::FixupPhase::attemptToMakeFastStringAdd):
(JSC::DFG::FixupPhase::isStringObjectUse): Deleted.
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::canOptimizeStringObjectAccess):
* dfg/DFGMayExit.cpp:
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructorOrStringValueOf):
(JSC::DFG::SpeculativeJIT::speculateStringObject):
(JSC::DFG::SpeculativeJIT::speculateStringOrStringObject):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::speculateStringObjectForStructure): Deleted.
* dfg/DFGUseKind.h:
(JSC::DFG::alreadyChecked):
(JSC::DFG::usesStructure): Deleted.
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileToStringOrCallStringConstructorOrStringValueOf):
(JSC::FTL::DFG::LowerDFGToB3::speculateStringObject):
(JSC::FTL::DFG::LowerDFGToB3::speculateStringOrStringObject):
(JSC::FTL::DFG::LowerDFGToB3::speculateStringObjectForCell):
(JSC::FTL::DFG::LowerDFGToB3::speculateStringObjectForStructureID): Deleted.
* runtime/JSType.cpp:
(WTF::printInternal):
* runtime/JSType.h:
* runtime/StringObject.h:
(JSC::StringObject::createStructure):
* runtime/StringPrototype.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240114 268f45cc-cd09-0410-ab3c-d52691b4dbfc
21 files changed:
JSTests/ChangeLog
JSTests/stress/cant-eliminate-string-object-structure-check-when-string-object-is-proven.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
Source/JavaScriptCore/bytecode/ExitKind.cpp
Source/JavaScriptCore/bytecode/ExitKind.h
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/dfg/DFGCSEPhase.cpp
Source/JavaScriptCore/dfg/DFGClobberize.h
Source/JavaScriptCore/dfg/DFGEdgeUsesStructure.h [deleted file]
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGGraph.cpp
Source/JavaScriptCore/dfg/DFGMayExit.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
Source/JavaScriptCore/dfg/DFGUseKind.h
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/runtime/JSType.cpp
Source/JavaScriptCore/runtime/JSType.h
Source/JavaScriptCore/runtime/StringObject.h
Source/JavaScriptCore/runtime/StringPrototype.h