haikuwebkit/LayoutTests/js/sequence-iterator-protocol-...

85 lines
2.4 KiB
HTML
Raw Permalink Normal View History

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. Canonical link: https://commits.webkit.org/188180@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@215777 268f45cc-cd09-0410-ab3c-d52691b4dbfc
2017-04-26 00:52:35 +00:00
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<script src="../resources/js-test-pre.js"></script>
</head>
<body>
<pre id='console'></pre>
<canvas></canvas>
<script>
"use strict";
const log = console.log.bind(console);
const canvas = document.querySelector("canvas");
const ctx = canvas.getContext("2d");
let callCount = 0;
function makeValue(x) {
return {
valueOf() { ++callCount; return x; }
};
}
let array = [makeValue(1), makeValue(2), makeValue(3)];
ctx.setLineDash(array);
let a = ctx.getLineDash();
log("Using an array but with objects with valueOf()");
log(a);
ctx.setLineDash([1,2,3]);
let b = ctx.getLineDash();
log(b);
if (a.toString() !== b.toString())
throw new Error("Bad result. They should be equal.");
log(`callCount: ${callCount}`);
if (callCount !== 3)
throw new Error("Bad result. callCount should be 3.");
let iter = [][Symbol.iterator]();
let iterProto = Object.getPrototypeOf(iter);
let oldNext = iterProto.next;
callCount = 0;
function hackedNext() {
++callCount;
let val = oldNext.call(this);
if ("value" in val) {
val.value++;
}
return val;
}
const obj = {
valueOf: function() {
iterProto.next = hackedNext;
return 2;
}
};
array = [1, obj, 3];
ctx.setLineDash(array);
b = ctx.getLineDash();
log(`${b}`);
iterProto.next = oldNext;
Speculatively change iteration protocall to use the same next function https://bugs.webkit.org/show_bug.cgi?id=175653 Reviewed by Saam Barati. JSTests: Change test to match the new iteration behavior. * stress/spread-optimized-properly.js: Source/JavaScriptCore: This patch speculatively makes a change to the iteration protocall to fetch the next property immediately after calling the Symbol.iterator function. This is, in theory, a breaking change, so we will see if this breaks things (most likely it won't as this is a relatively subtle point). See: https://github.com/tc39/ecma262/issues/976 * builtins/IteratorHelpers.js: (performIteration): * bytecompiler/BytecodeGenerator.cpp: (JSC::BytecodeGenerator::emitEnumeration): (JSC::BytecodeGenerator::emitIteratorNext): (JSC::BytecodeGenerator::emitIteratorNextWithValue): (JSC::BytecodeGenerator::emitDelegateYield): * bytecompiler/BytecodeGenerator.h: * bytecompiler/NodesCodegen.cpp: (JSC::ArrayPatternNode::bindValue const): * inspector/JSInjectedScriptHost.cpp: (Inspector::JSInjectedScriptHost::iteratorEntries): * runtime/IteratorOperations.cpp: (JSC::iteratorNext): (JSC::iteratorStep): (JSC::iteratorClose): (JSC::iteratorForIterable): * runtime/IteratorOperations.h: (JSC::forEachInIterable): * runtime/JSGenericTypedArrayViewConstructorInlines.h: (JSC::constructGenericTypedArrayViewFromIterator): (JSC::constructGenericTypedArrayViewWithArguments): LayoutTests: Change test to match the new iteration behavior. * js/sequence-iterator-protocol-2-expected.txt: * js/sequence-iterator-protocol-2.html: Canonical link: https://commits.webkit.org/193717@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@222421 268f45cc-cd09-0410-ab3c-d52691b4dbfc
2017-09-23 02:08:04 +00:00
ctx.setLineDash([1,2,3]);
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. Canonical link: https://commits.webkit.org/188180@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@215777 268f45cc-cd09-0410-ab3c-d52691b4dbfc
2017-04-26 00:52:35 +00:00
a = ctx.getLineDash();
log(a);
if (a.toString() !== b.toString())
throw new Error("Bad result. They should be equal.");
log(`callCount: ${callCount}`);
Speculatively change iteration protocall to use the same next function https://bugs.webkit.org/show_bug.cgi?id=175653 Reviewed by Saam Barati. JSTests: Change test to match the new iteration behavior. * stress/spread-optimized-properly.js: Source/JavaScriptCore: This patch speculatively makes a change to the iteration protocall to fetch the next property immediately after calling the Symbol.iterator function. This is, in theory, a breaking change, so we will see if this breaks things (most likely it won't as this is a relatively subtle point). See: https://github.com/tc39/ecma262/issues/976 * builtins/IteratorHelpers.js: (performIteration): * bytecompiler/BytecodeGenerator.cpp: (JSC::BytecodeGenerator::emitEnumeration): (JSC::BytecodeGenerator::emitIteratorNext): (JSC::BytecodeGenerator::emitIteratorNextWithValue): (JSC::BytecodeGenerator::emitDelegateYield): * bytecompiler/BytecodeGenerator.h: * bytecompiler/NodesCodegen.cpp: (JSC::ArrayPatternNode::bindValue const): * inspector/JSInjectedScriptHost.cpp: (Inspector::JSInjectedScriptHost::iteratorEntries): * runtime/IteratorOperations.cpp: (JSC::iteratorNext): (JSC::iteratorStep): (JSC::iteratorClose): (JSC::iteratorForIterable): * runtime/IteratorOperations.h: (JSC::forEachInIterable): * runtime/JSGenericTypedArrayViewConstructorInlines.h: (JSC::constructGenericTypedArrayViewFromIterator): (JSC::constructGenericTypedArrayViewWithArguments): LayoutTests: Change test to match the new iteration behavior. * js/sequence-iterator-protocol-2-expected.txt: * js/sequence-iterator-protocol-2.html: Canonical link: https://commits.webkit.org/193717@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@222421 268f45cc-cd09-0410-ab3c-d52691b4dbfc
2017-09-23 02:08:04 +00:00
if (callCount !== 0)
throw new Error("Bad result. callCount should be 0.");
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. Canonical link: https://commits.webkit.org/188180@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@215777 268f45cc-cd09-0410-ab3c-d52691b4dbfc
2017-04-26 00:52:35 +00:00
</script>
<script src="../resources/js-test-post.js"></script>
</body>
</html>