diff --git a/graal-js/src/com.oracle.truffle.js.parser/src/com/oracle/truffle/js/parser/GraalJSTranslator.java b/graal-js/src/com.oracle.truffle.js.parser/src/com/oracle/truffle/js/parser/GraalJSTranslator.java index 61a1d660c27..f20f014dd00 100644 --- a/graal-js/src/com.oracle.truffle.js.parser/src/com/oracle/truffle/js/parser/GraalJSTranslator.java +++ b/graal-js/src/com.oracle.truffle.js.parser/src/com/oracle/truffle/js/parser/GraalJSTranslator.java @@ -2195,7 +2195,7 @@ private JavaScriptNode desugarForInOrOfBody(ForNode forNode, JavaScriptNode iter wrappedBody = factory.createIteratorCloseWrapper(context, wrappedBody, iteratorVar.createReadNode(), false); } - RepeatingNode repeatingNode = factory.createForOfRepeatingNode(iteratorNext, wrappedBody, + RepeatingNode repeatingNode = factory.createForOfRepeatingNode(iteratorVar.createReadNode(), iteratorNext, wrappedBody, (JSWriteFrameSlotNode) nextResultVar.createWriteNode(null)); LoopNode loopNode = factory.createLoopNode(repeatingNode); JavaScriptNode whileNode = forNode.isForOf() diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/ArrayIteratorPrototypeBuiltins.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/ArrayIteratorPrototypeBuiltins.java index 404e3cd4807..83e5033ae91 100644 --- a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/ArrayIteratorPrototypeBuiltins.java +++ b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/ArrayIteratorPrototypeBuiltins.java @@ -42,15 +42,20 @@ import com.oracle.truffle.api.dsl.Bind; import com.oracle.truffle.api.dsl.Cached; -import com.oracle.truffle.api.dsl.Cached.Shared; import com.oracle.truffle.api.dsl.Fallback; import com.oracle.truffle.api.dsl.GenerateCached; import com.oracle.truffle.api.dsl.GenerateInline; +import com.oracle.truffle.api.dsl.ImportStatic; import com.oracle.truffle.api.dsl.Specialization; +import com.oracle.truffle.api.interop.InteropLibrary; +import com.oracle.truffle.api.interop.UnsupportedMessageException; +import com.oracle.truffle.api.library.CachedLibrary; import com.oracle.truffle.api.nodes.Node; import com.oracle.truffle.api.profiles.InlinedBranchProfile; +import com.oracle.truffle.api.profiles.InlinedConditionProfile; import com.oracle.truffle.api.profiles.InlinedIntValueProfile; import com.oracle.truffle.js.builtins.ArrayIteratorPrototypeBuiltinsFactory.ArrayIteratorNextNodeGen; +import com.oracle.truffle.js.nodes.JavaScriptBaseNode; import com.oracle.truffle.js.nodes.access.CreateIterResultObjectNode; import com.oracle.truffle.js.nodes.access.ReadElementNode; import com.oracle.truffle.js.nodes.array.JSGetLengthNode; @@ -59,6 +64,7 @@ import com.oracle.truffle.js.nodes.function.JSBuiltin; import com.oracle.truffle.js.nodes.function.JSBuiltinNode; import com.oracle.truffle.js.runtime.Errors; +import com.oracle.truffle.js.runtime.JSConfig; import com.oracle.truffle.js.runtime.JSContext; import com.oracle.truffle.js.runtime.JSRuntime; import com.oracle.truffle.js.runtime.builtins.BuiltinEnum; @@ -108,27 +114,39 @@ protected Object createNode(JSContext context, JSBuiltin builtin, boolean constr public abstract static class ArrayIteratorNextNode extends JSBuiltinNode { + @Child private CreateIterResultObjectNode createIterResultObjectNode; + protected ArrayIteratorNextNode(JSContext context, JSBuiltin builtin) { super(context, builtin); + this.createIterResultObjectNode = CreateIterResultObjectNode.create(context); + } + + protected JSObject createIteratorResultObject(Object value, boolean done) { + return createIterResultObjectNode.execute(value, done); } @Specialization(guards = {"!isUndefined(array)"}) JSObject doArrayIterator(JSArrayIteratorObject iterator, @Bind("iterator.getIteratedObject()") Object array, - @Shared @Cached("create(getContext())") CreateIterResultObjectNode createIterResultObjectNode, @Cached("create(getContext())") ReadElementNode readElementNode, @Cached ArrayIteratorGetLengthNode getLengthNode, @Cached(inline = true) LongToIntOrDoubleNode toJSIndex, - @Cached InlinedIntValueProfile iterationKindProfile) { + @Cached InlinedIntValueProfile iterationKindProfile, + @Cached InlinedConditionProfile skipLengthCheckBranch) { long index = iterator.getNextIndex(); - int itemKind = iterationKindProfile.profile(this, iterator.getIterationKind()); - long length = getLengthNode.execute(this, array, getJSContext()); - - if (index >= length) { - iterator.setIteratedObject(Undefined.instance); - return createIterResultObjectNode.execute(Undefined.instance, true); + assert index >= 0; + if (skipLengthCheckBranch.profile(this, iterator.isSkipGetLength())) { + // Already checked in the caller, no need to get the length again. + iterator.setSkipGetLength(false); + } else { + long length = getLengthNode.execute(this, array, getContext()); + if (index >= length) { + iterator.setIteratedObject(Undefined.instance); + return createIteratorResultObject(Undefined.instance, true); + } } + int itemKind = iterationKindProfile.profile(this, iterator.getIterationKind()); iterator.setNextIndex(index + 1); Object indexNumber = null; if ((itemKind & JSRuntime.ITERATION_KIND_KEY) != 0) { @@ -146,13 +164,12 @@ JSObject doArrayIterator(JSArrayIteratorObject iterator, result = JSArray.createConstantObjectArray(getContext(), getRealm(), new Object[]{indexNumber, elementValue}); } } - return createIterResultObjectNode.execute(result, false); + return createIteratorResultObject(result, false); } @Specialization(guards = {"isUndefined(iterator.getIteratedObject())"}) - static JSObject doArrayIteratorDetached(@SuppressWarnings("unused") JSArrayIteratorObject iterator, - @Cached("create(getContext())") @Shared CreateIterResultObjectNode createIterResultObjectNode) { - return createIterResultObjectNode.execute(Undefined.instance, true); + final JSObject doArrayIteratorDetached(@SuppressWarnings("unused") JSArrayIteratorObject iterator) { + return createIteratorResultObject(Undefined.instance, true); } @SuppressWarnings("unused") @@ -161,33 +178,87 @@ static JSObject doIncompatibleReceiver(Object iterator) { throw Errors.createTypeError("not an Array Iterator"); } } -} -@GenerateInline -@GenerateCached(false) -abstract class ArrayIteratorGetLengthNode extends Node { + @GenerateInline + @GenerateCached(false) + public abstract static class ArrayIteratorGetLengthNode extends JavaScriptBaseNode { - abstract long execute(Node node, Object array, JSContext context); + public abstract long execute(Node node, Object array, JSContext context); - @Specialization - static long doArray(JSArrayObject array, @SuppressWarnings("unused") JSContext context) { - return JSArray.arrayGetLength(array); - } + @Specialization + static long doArray(JSArrayObject array, @SuppressWarnings("unused") JSContext context) { + return JSArray.arrayGetLength(array); + } - @Specialization - static long doTypedArray(Node node, JSTypedArrayObject typedArray, JSContext context, - @Cached TypedArrayLengthNode typedArrayLengthNode, - @Cached InlinedBranchProfile errorBranch) { - if (JSArrayBufferView.isOutOfBounds(typedArray, context)) { - errorBranch.enter(node); - throw Errors.createTypeError("Cannot perform Array Iterator.prototype.next on a detached ArrayBuffer"); + @Specialization + static long doTypedArray(Node node, JSTypedArrayObject typedArray, JSContext context, + @Cached TypedArrayLengthNode typedArrayLengthNode, + @Cached InlinedBranchProfile errorBranch) { + if (JSArrayBufferView.isOutOfBounds(typedArray, context)) { + errorBranch.enter(node); + throw Errors.createTypeError("Cannot perform Array Iterator.prototype.next on a detached ArrayBuffer"); + } + return typedArrayLengthNode.execute(node, typedArray, context); + } + + @Fallback + static long doArrayLike(Object array, @SuppressWarnings("unused") JSContext context, + @Cached(value = "create(context)", inline = false) JSGetLengthNode getLengthNode) { + return getLengthNode.executeLong(array); } - return typedArrayLengthNode.execute(node, typedArray, context); } - @Fallback - static long doArrayLike(Object array, @SuppressWarnings("unused") JSContext context, - @Cached(value = "create(context)", inline = false) JSGetLengthNode getLengthNode) { - return getLengthNode.executeLong(array); + /** + * Like {@link ArrayIteratorGetLengthNode}, but designed to avoid observable side effects. Only + * handles JS arrays, typed arrays, and foreign arrays (assumes getArraySize to well-behaved). + * Returns {@value JSRuntime#INVALID_INTEGER_INDEX} for other object types or if getting the + * length would throw an error. + */ + @ImportStatic(JSConfig.class) + @GenerateInline + @GenerateCached(false) + public abstract static class ArrayIteratorGetLengthSafeNode extends JavaScriptBaseNode { + + public abstract long execute(Node node, Object array); + + @Specialization + static long doArray(JSArrayObject array) { + return JSArray.arrayGetLength(array); + } + + @Specialization + static long doTypedArray(Node node, JSTypedArrayObject typedArray, + @Cached TypedArrayLengthNode typedArrayLengthNode, + @Cached InlinedBranchProfile errorBranch) { + JSContext context = JSContext.get(node); + if (JSArrayBufferView.isOutOfBounds(typedArray, context)) { + errorBranch.enter(node); + // Must call into the next method to ensure correct stack trace for the error. + return JSRuntime.INVALID_INTEGER_INDEX; + } + return typedArrayLengthNode.execute(node, typedArray, context); + } + + @Specialization(guards = "isUndefined(array)") + static long doUndefined(@SuppressWarnings("unused") Object array) { + // Iterator is already done and detached. + return 0L; + } + + @Specialization(guards = {"interop.hasArrayElements(array)", "isForeignObject(array)"}, limit = "InteropLibraryLimit") + static long doForeignArray(Object array, + @CachedLibrary("array") InteropLibrary interop) { + try { + return interop.getArraySize(array); + } catch (UnsupportedMessageException e) { + return JSRuntime.INVALID_INTEGER_INDEX; + } + } + + @Fallback + static long doArrayLike(@SuppressWarnings("unused") Object array) { + // Must call into the next method to ensure correct stack trace for length getters. + return JSRuntime.INVALID_INTEGER_INDEX; + } } } diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/NodeFactory.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/NodeFactory.java index 91e48c041ad..dfd8644e5bd 100644 --- a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/NodeFactory.java +++ b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/NodeFactory.java @@ -537,8 +537,8 @@ public JavaScriptNode createDesugaredForAwaitOf(LoopNode loopNode) { return WhileNode.createDesugaredForAwaitOf(loopNode); } - public RepeatingNode createForOfRepeatingNode(JavaScriptNode nextResultNode, JavaScriptNode body, JSWriteFrameSlotNode writeNextValueNode) { - return WhileNode.createForOfRepeatingNode(nextResultNode, body, writeNextValueNode); + public RepeatingNode createForOfRepeatingNode(JavaScriptNode iteratorNode, JavaScriptNode nextResultNode, JavaScriptNode body, JSWriteFrameSlotNode writeNextValueNode) { + return WhileNode.createForOfRepeatingNode(iteratorNode, nextResultNode, body, writeNextValueNode); } public RepeatingNode createForRepeatingNode(JavaScriptNode condition, JavaScriptNode body, JavaScriptNode modify, FrameDescriptor frameDescriptor, JavaScriptNode isFirstNode, diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/control/AbstractRepeatingNode.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/control/AbstractRepeatingNode.java index 85ddddeb909..3606f491bb1 100644 --- a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/control/AbstractRepeatingNode.java +++ b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/control/AbstractRepeatingNode.java @@ -83,10 +83,20 @@ private void checkThreadInterrupted() { } @Override - public Object execute(VirtualFrame frame) { + public final Object execute(VirtualFrame frame) { return executeRepeating(frame); } + @Override + public final boolean executeBoolean(VirtualFrame frame) { + return executeRepeating(frame); + } + + @Override + public final Object executeRepeatingWithValue(VirtualFrame frame) { + return RepeatingNode.super.executeRepeatingWithValue(frame); + } + protected boolean materializationNeeded() { // If we are using tagged nodes, this node is already materialized. return !JSNodeUtil.isTaggedNode(bodyNode); diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/control/WhileNode.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/control/WhileNode.java index 794d429f0d1..17a8606a201 100644 --- a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/control/WhileNode.java +++ b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/control/WhileNode.java @@ -43,12 +43,16 @@ import java.util.Set; import com.oracle.truffle.api.Truffle; +import com.oracle.truffle.api.dsl.Bind; +import com.oracle.truffle.api.dsl.Cached; +import com.oracle.truffle.api.dsl.Specialization; import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.instrumentation.InstrumentableNode; import com.oracle.truffle.api.instrumentation.Tag; import com.oracle.truffle.api.nodes.LoopNode; import com.oracle.truffle.api.nodes.NodeInfo; import com.oracle.truffle.api.nodes.RepeatingNode; +import com.oracle.truffle.js.builtins.ArrayIteratorPrototypeBuiltins.ArrayIteratorGetLengthSafeNode; import com.oracle.truffle.js.nodes.JavaScriptNode; import com.oracle.truffle.js.nodes.access.IteratorCompleteNode; import com.oracle.truffle.js.nodes.access.IteratorValueNode; @@ -57,6 +61,8 @@ import com.oracle.truffle.js.nodes.instrumentation.JSTags.ControlFlowBlockTag; import com.oracle.truffle.js.nodes.instrumentation.JSTags.ControlFlowBranchTag; import com.oracle.truffle.js.nodes.instrumentation.JSTags.ControlFlowRootTag; +import com.oracle.truffle.js.runtime.builtins.JSArrayIteratorObject; +import com.oracle.truffle.js.runtime.objects.IteratorRecord; import com.oracle.truffle.js.runtime.objects.Undefined; /** @@ -116,9 +122,9 @@ public static JavaScriptNode createDoWhile(LoopNode loopNode) { return new WhileNode(loopNode, ControlFlowRootTag.Type.DoWhileIteration); } - public static RepeatingNode createForOfRepeatingNode(JavaScriptNode nextResultNode, JavaScriptNode body, JSWriteFrameSlotNode writeNextValueNode) { + public static RepeatingNode createForOfRepeatingNode(JavaScriptNode iteratorNode, JavaScriptNode nextResultNode, JavaScriptNode body, JSWriteFrameSlotNode writeNextValueNode) { JavaScriptNode nonVoidBody = body instanceof DiscardResultNode ? ((DiscardResultNode) body).getOperand() : body; - return new ForOfRepeatingNode(nextResultNode, nonVoidBody, writeNextValueNode); + return ForOfRepeatingNode.create(iteratorNode, nextResultNode, nonVoidBody, writeNextValueNode); } @Override @@ -267,28 +273,70 @@ public AbstractRepeatingNode materializeInstrumentableNodes(Set= 0"}, limit = "1") + protected boolean doArrayIterator(VirtualFrame frame, + @Bind("getIteratorRecord(frame)") @SuppressWarnings("unused") IteratorRecord iteratorRecord, + @Cached @SuppressWarnings("unused") ArrayIteratorGetLengthSafeNode getLengthNode, + @Bind("getArrayIterator(iteratorRecord)") JSArrayIteratorObject arrayIterator, + @Bind("getLengthNode.execute($node, arrayIterator.getIteratedObject())") long length) { + long index = arrayIterator.getNextIndex(); + if (index >= length) { + arrayIterator.setIteratedObject(Undefined.instance); + return false; + } + + /* + * Call the "next" method as usual to advance the iterator and get the next result, but + * skip getting the length again and checking if (index >= length). Also skip getting + * the "done" property since it's side-effect-free for built-in iterator result objects. + */ + arrayIterator.setSkipGetLength(true); + Object nextResult = nextResultNode.execute(frame); + Object nextValue = iteratorValueNode.execute(nextResult); + writeNextValueNode.executeWrite(frame, nextValue); + try { + executeBody(frame); + } finally { + writeNextValueNode.executeWrite(frame, Undefined.instance); + } + return true; + } + + @Specialization(replaces = "doArrayIterator") + protected boolean doGeneric(VirtualFrame frame) { + Object nextResult = nextResultNode.execute(frame); boolean done = iteratorCompleteNode.execute(nextResult); Object nextValue = iteratorValueNode.execute(nextResult); if (done) { @@ -307,7 +355,7 @@ public boolean executeRepeating(VirtualFrame frame) { public Object resume(VirtualFrame frame, int stateSlot) { int state = getStateAsIntAndReset(frame, stateSlot); if (state == 0) { - Object nextResult = executeNextResult(frame); + Object nextResult = nextResultNode.execute(frame); boolean done = iteratorCompleteNode.execute(nextResult); Object nextValue = iteratorValueNode.execute(nextResult); if (done) { @@ -328,7 +376,9 @@ public Object resume(VirtualFrame frame, int stateSlot) { @Override protected JavaScriptNode copyUninitialized(Set> materializedTags) { - return new ForOfRepeatingNode(cloneUninitialized(nextResultNode, materializedTags), + return ForOfRepeatingNode.create( + cloneUninitialized(iteratorNode, materializedTags), + cloneUninitialized(nextResultNode, materializedTags), cloneUninitialized(bodyNode, materializedTags), cloneUninitialized(writeNextValueNode, materializedTags)); } @@ -338,9 +388,24 @@ public AbstractRepeatingNode materializeInstrumentableNodes(Set