JSArray::isArrayPrototypeIteratorProtocolFastAndNonObservable is wrong because it...
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Apr 2017 00:52:35 +0000 (00:52 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Apr 2017 00:52:35 +0000 (00:52 +0000)
commitff86143466866c19dd114a8fbc271c3692ada1af
tree68f22bb95c3b984fcef5e6aaf82c907074e21bba
parentd53c27b5159f44e2a303a903e3514a622c0a02ad
JSArray::isArrayPrototypeIteratorProtocolFastAndNonObservable is wrong because it does not do the necessary checks on the base object
https://bugs.webkit.org/show_bug.cgi?id=171150
<rdar://problem/31771880>

Reviewed by Sam Weinig.

JSTests:

* stress/spread-optimized-properly.js: Added.
(assert):
(test):
(shallowEq):
(makeArrayIterator):
(test.bar):
(test.callback):
(test.arr.__proto__.Symbol.iterator):
(test.arr.Symbol.iterator):
(test.get bar):
(test.hackedNext):
(test.test):
(test.):

Source/JavaScriptCore:

This patch fixes a huge oversight from the patch that introduced
op_spread/Spread. The original patch did not account for the
base object having Symbol.iterator or getters that could
change the iterator protocol. This patch fixes the oversight both
in the C code, as well as the DFG/FTL backends. We only perform
the memcpy version of spread if we've proven that it's guaranteed
to be side-effect free (no indexed getters), and if the iterator
protocol is guaranteed to be the original protocol. To do this, we
must prove that:
1. The protocol on Array.prototype hasn't changed (this is the same as the
introductory patch for op_spread).
2. The base object's __proto__ is Array.prototype
3. The base object does not have indexed getters
4. The base object does not have Symbol.iterator property.

* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::canDoFastSpread):
* dfg/DFGGraph.h:
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileSpread):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileSpread):
* runtime/JSArray.cpp:
(JSC::JSArray::isIteratorProtocolFastAndNonObservable):
* runtime/JSArray.h:
* runtime/JSArrayInlines.h:
(JSC::JSArray::isIteratorProtocolFastAndNonObservable): Deleted.
* runtime/JSGlobalObject.h:
* runtime/JSGlobalObjectInlines.h:
(JSC::JSGlobalObject::isArrayPrototypeIteratorProtocolFastAndNonObservable):
(JSC::JSGlobalObject::isArrayIteratorProtocolFastAndNonObservable): Deleted.

Source/WebCore:

This patch moves the sequence converters to use the now fixed
JSArray::isArrayPrototypeIteratorProtocolFastAndNonObservable test
inside JSC.

This patch also fixes a few bugs:
1. Converting to a sequence of numbers must prove that the JSArray
is filled only with Int32/Double. If there is a chance the array
contains objects, the conversion to a numeric IDLType can be observable
(via valueOf()), and can change the iterator protocol.
2. There are other conversions that can have side effects a-la valueOf().
This patch introduces a new static constant in the various Converter
classes that tell the sequence converter if the conversion operation
can have JS side effects. If it does have side effects, we fall back to
the generic conversion that uses the iterator protocol. If not, we can
do a faster version that iterates over each element of the array,
reading it directly, and converting it.

Tests: js/sequence-iterator-protocol-2.html
       js/sequence-iterator-protocol.html

* bindings/js/JSDOMConvertAny.h: Does not have side effects.
* bindings/js/JSDOMConvertBase.h: We pessimistically assume inside DefaultConverter that converions have side effects.
* bindings/js/JSDOMConvertBoolean.h: Does not have side effects.
* bindings/js/JSDOMConvertCallbacks.h: Does not have side effects.
* bindings/js/JSDOMConvertObject.h: Does not have side effects.
* bindings/js/JSDOMConvertSequences.h:
(WebCore::Detail::NumericSequenceConverter::convert):
(WebCore::Detail::SequenceConverter::convert):

LayoutTests:

* js/sequence-iterator-protocol-2-expected.txt: Added.
* js/sequence-iterator-protocol-2.html: Added.
* js/sequence-iterator-protocol-expected.txt: Added.
* js/sequence-iterator-protocol.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@215777 268f45cc-cd09-0410-ab3c-d52691b4dbfc
24 files changed:
JSTests/ChangeLog
JSTests/stress/spread-optimized-properly.js [new file with mode: 0644]
LayoutTests/ChangeLog
LayoutTests/js/sequence-iterator-protocol-2-expected.txt [new file with mode: 0644]
LayoutTests/js/sequence-iterator-protocol-2.html [new file with mode: 0644]
LayoutTests/js/sequence-iterator-protocol-expected.txt [new file with mode: 0644]
LayoutTests/js/sequence-iterator-protocol.html [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGGraph.cpp
Source/JavaScriptCore/dfg/DFGGraph.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/runtime/JSArray.cpp
Source/JavaScriptCore/runtime/JSArray.h
Source/JavaScriptCore/runtime/JSArrayInlines.h
Source/JavaScriptCore/runtime/JSGlobalObject.h
Source/JavaScriptCore/runtime/JSGlobalObjectInlines.h
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSDOMConvertAny.h
Source/WebCore/bindings/js/JSDOMConvertBase.h
Source/WebCore/bindings/js/JSDOMConvertBoolean.h
Source/WebCore/bindings/js/JSDOMConvertCallbacks.h
Source/WebCore/bindings/js/JSDOMConvertObject.h
Source/WebCore/bindings/js/JSDOMConvertSequences.h