From 4ee42e38e0a490eb4880b4a260e3cbe07dd486d1 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Sun, 25 Mar 2018 14:20:11 +0200 Subject: Bug 1338796 - Do not call iterator.return if iterator.throw is present in yield* Issue #74 --- js/src/frontend/BytecodeEmitter.cpp | 280 ++++++++++++--------- js/src/frontend/BytecodeEmitter.h | 4 +- .../ecma_6/Generators/yield-star-iterator-close.js | 38 ++- js/src/tests/ecma_6/shell.js | 7 +- 4 files changed, 197 insertions(+), 132 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index b1b0055e2..4a60f1cf1 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -5141,7 +5141,7 @@ BytecodeEmitter::emitIteratorNext(ParseNode* pn, bool allowSelfHosted) } bool -BytecodeEmitter::emitIteratorClose(Maybe yieldStarTryStart, bool allowSelfHosted) +BytecodeEmitter::emitIteratorClose(bool allowSelfHosted) { MOZ_ASSERT(allowSelfHosted || emitterMode != BytecodeEmitter::SelfHosting, ".close() on iterators is prohibited in self-hosted code because it " @@ -5180,86 +5180,11 @@ BytecodeEmitter::emitIteratorClose(Maybe yieldStarTryStart, bool all // an Object. if (!emit1(JSOP_SWAP)) // ... RET ITER return false; - - // ES 14.4.13, yield * AssignmentExpression, step 5.c - // - // When emitting iterator.return() for yield* forced return, we need to - // pass the argument passed to Generator.prototype.return to the return - // method. - if (yieldStarTryStart) { - IfThenElseEmitter ifGeneratorClosing(this); - if (!emitDupAt(2)) // ... FTYPE FVALUE RET ITER FVALUE - return false; - if (!emit1(JSOP_ISGENCLOSING)) // ... FTYPE FVALUE RET ITER FVALUE CLOSING - return false; - if (!emit1(JSOP_SWAP)) // ... FTYPE FVALUE RET ITER CLOSING FVALUE - return false; - if (!emit1(JSOP_POP)) // ... FTYPE FVALUE RET ITER CLOSING - return false; - if (!ifGeneratorClosing.emitIfElse()) // ... FTYPE FVALUE RET ITER - return false; - - if (!emit1(JSOP_GETRVAL)) // ... FTYPE FVALUE RET ITER RVAL - return false; - if (!emitAtomOp(cx->names().value, JSOP_GETPROP)) // ... FTYPE FVALUE RET ITER VALUE - return false; - if (!emitCall(JSOP_CALL, 1)) // ... FTYPE FVALUE RESULT - return false; - checkTypeSet(JSOP_CALL); - if (!emitCheckIsObj(CheckIsObjectKind::IteratorReturn)) // ... FTYPE FVALUE RESULT - return false; - - IfThenElseEmitter ifReturnDone(this); - if (!emit1(JSOP_DUP)) // ITER OLDRESULT FTYPE FVALUE RESULT RESULT - return false; - if (!emitAtomOp(cx->names().done, JSOP_GETPROP)) // ITER OLDRESULT FTYPE FVALUE RESULT DONE - return false; - if (!ifReturnDone.emitIfElse()) // ITER OLDRESULT FTYPE FVALUE RESULT - return false; - if (!emitAtomOp(cx->names().value, JSOP_GETPROP)) // ITER OLDRESULT FTYPE FVALUE VALUE - return false; - if (!emitPrepareIteratorResult()) // ITER OLDRESULT FTYPE FVALUE VALUE RESULT - return false; - if (!emit1(JSOP_SWAP)) // ITER OLDRESULT FTYPE FVALUE RESULT VALUE - return false; - if (!emitFinishIteratorResult(true)) // ITER OLDRESULT FTYPE FVALUE RESULT - return false; - if (!emit1(JSOP_DUP)) // ITER OLDRESULT FTYPE FVALUE RESULT RESULT - return false; - if (!emit1(JSOP_SETRVAL)) // ITER OLDRESULT FTYPE FVALUE RESULT - return false; - if (!ifReturnDone.emitElse()) // ITER OLDRESULT FTYPE FVALUE RESULT - return false; - int32_t savedDepth = this->stackDepth; - if (!emit2(JSOP_UNPICK, 3)) // ITER RESULT OLDRESULT FTYPE FVALUE - return false; - if (!emitUint16Operand(JSOP_POPN, 3)) // ITER RESULT - return false; - JumpList beq; - JumpTarget breakTarget{ -1 }; - if (!emitBackwardJump(JSOP_GOTO, *yieldStarTryStart, &beq, &breakTarget)) // ITER RESULT - return false; - this->stackDepth = savedDepth; - if (!ifReturnDone.emitEnd()) - return false; - - if (!ifGeneratorClosing.emitElse()) // ... FTYPE FVALUE RET ITER - return false; - if (!emitCall(JSOP_CALL, 0)) // ... FTYPE FVALUE RESULT - return false; - checkTypeSet(JSOP_CALL); - if (!emitCheckIsObj(CheckIsObjectKind::IteratorReturn)) // ... FTYPE FVALUE RESULT - return false; - - if (!ifGeneratorClosing.emitEnd()) - return false; - } else { - if (!emitCall(JSOP_CALL, 0)) // ... RESULT - return false; - checkTypeSet(JSOP_CALL); - if (!emitCheckIsObj(CheckIsObjectKind::IteratorReturn)) // ... RESULT - return false; - } + if (!emitCall(JSOP_CALL, 0)) // ... RESULT + return false; + checkTypeSet(JSOP_CALL); + if (!emitCheckIsObj(CheckIsObjectKind::IteratorReturn)) // ... RESULT + return false; if (!ifReturnMethodIsDefined.emitElse()) return false; @@ -8227,93 +8152,202 @@ BytecodeEmitter::emitYieldStar(ParseNode* iter, ParseNode* gen) MOZ_ASSERT(sc->isFunctionBox()); MOZ_ASSERT(sc->asFunctionBox()->isStarGenerator()); - if (!emitTree(iter)) // ITERABLE + if (!emitTree(iter)) // ITERABLE return false; - if (!emitIterator()) // ITER + if (!emitIterator()) // ITER return false; // Initial send value is undefined. - if (!emit1(JSOP_UNDEFINED)) // ITER RECEIVED + if (!emit1(JSOP_UNDEFINED)) // ITER RECEIVED return false; - int depth = stackDepth; - MOZ_ASSERT(depth >= 2); + int32_t savedDepthTemp; + int32_t startDepth = stackDepth; + MOZ_ASSERT(startDepth >= 2); TryEmitter tryCatch(this, TryEmitter::TryCatchFinally, TryEmitter::DontUseRetVal, TryEmitter::DontUseControl); - if (!tryCatch.emitJumpOverCatchAndFinally()) // ITER RESULT + if (!tryCatch.emitJumpOverCatchAndFinally()) // ITER RESULT return false; JumpTarget tryStart{ offset() }; - if (!tryCatch.emitTry()) // ITER RESULT + if (!tryCatch.emitTry()) // ITER RESULT return false; - MOZ_ASSERT(this->stackDepth == depth); + MOZ_ASSERT(this->stackDepth == startDepth); // Load the generator object. - if (!emitTree(gen)) // ITER RESULT GENOBJ + if (!emitTree(gen)) // ITER RESULT GENOBJ return false; // Yield RESULT as-is, without re-boxing. - if (!emitYieldOp(JSOP_YIELD)) // ITER RECEIVED + if (!emitYieldOp(JSOP_YIELD)) // ITER RECEIVED return false; - if (!tryCatch.emitCatch()) // ITER RESULT + if (!tryCatch.emitCatch()) // ITER RESULT return false; - stackDepth = uint32_t(depth); // ITER RESULT - if (!emit1(JSOP_EXCEPTION)) // ITER RESULT EXCEPTION + stackDepth = startDepth; // ITER RESULT + if (!emit1(JSOP_EXCEPTION)) // ITER RESULT EXCEPTION return false; - if (!emitDupAt(2)) // ITER RESULT EXCEPTION ITER + if (!emitDupAt(2)) // ITER RESULT EXCEPTION ITER return false; - if (!emit1(JSOP_DUP)) // ITER RESULT EXCEPTION ITER ITER + if (!emit1(JSOP_DUP)) // ITER RESULT EXCEPTION ITER ITER return false; - if (!emitAtomOp(cx->names().throw_, JSOP_CALLPROP)) // ITER RESULT EXCEPTION ITER THROW + if (!emitAtomOp(cx->names().throw_, JSOP_CALLPROP)) // ITER RESULT EXCEPTION ITER THROW return false; - if (!emit1(JSOP_DUP)) // ITER RESULT EXCEPTION ITER THROW THROW + if (!emit1(JSOP_DUP)) // ITER RESULT EXCEPTION ITER THROW THROW return false; - if (!emit1(JSOP_UNDEFINED)) // ITER RESULT EXCEPTION ITER THROW THROW UNDEFINED + if (!emit1(JSOP_UNDEFINED)) // ITER RESULT EXCEPTION ITER THROW THROW UNDEFINED return false; - if (!emit1(JSOP_EQ)) // ITER RESULT EXCEPTION ITER THROW ?EQL + if (!emit1(JSOP_EQ)) // ITER RESULT EXCEPTION ITER THROW ?EQL return false; IfThenElseEmitter ifThrowMethodIsNotDefined(this); - if (!ifThrowMethodIsNotDefined.emitIf()) // ITER RESULT EXCEPTION ITER THROW + if (!ifThrowMethodIsNotDefined.emitIf()) // ITER RESULT EXCEPTION ITER THROW + return false; + savedDepthTemp = stackDepth; + if (!emit1(JSOP_POP)) // ITER RESULT EXCEPTION ITER + return false; + // ES 14.4.13, YieldExpression : yield * AssignmentExpression, step 5.b.iii.2 + // + // If the iterator does not have a "throw" method, it calls IteratorClose + // and then throws a TypeError. + if (!emitIteratorClose()) // ITER RESULT EXCEPTION return false; if (!emitUint16Operand(JSOP_THROWMSG, JSMSG_ITERATOR_NO_THROW)) // throw return false; - if (!ifThrowMethodIsNotDefined.emitEnd()) // ITER OLDRESULT EXCEPTION ITER THROW + stackDepth = savedDepthTemp; + if (!ifThrowMethodIsNotDefined.emitEnd()) // ITER OLDRESULT EXCEPTION ITER THROW return false; // ES 14.4.13, YieldExpression : yield * AssignmentExpression, step 5.b.iii.4. - // RESULT = ITER.throw(EXCEPTION) // ITER OLDRESULT EXCEPTION ITER THROW - if (!emit1(JSOP_SWAP)) // ITER OLDRESULT EXCEPTION THROW ITER + // RESULT = ITER.throw(EXCEPTION) // ITER OLDRESULT EXCEPTION ITER THROW + if (!emit1(JSOP_SWAP)) // ITER OLDRESULT EXCEPTION THROW ITER return false; - if (!emit2(JSOP_PICK, 2)) // ITER OLDRESULT THROW ITER EXCEPTION + if (!emit2(JSOP_PICK, 2)) // ITER OLDRESULT THROW ITER EXCEPTION return false; - if (!emitCall(JSOP_CALL, 1, iter)) // ITER OLDRESULT RESULT + if (!emitCall(JSOP_CALL, 1, iter)) // ITER OLDRESULT RESULT return false; checkTypeSet(JSOP_CALL); - if (!emitCheckIsObj(CheckIsObjectKind::IteratorThrow)) // ITER OLDRESULT RESULT + if (!emitCheckIsObj(CheckIsObjectKind::IteratorThrow)) // ITER OLDRESULT RESULT return false; - if (!emit1(JSOP_SWAP)) // ITER RESULT OLDRESULT + if (!emit1(JSOP_SWAP)) // ITER RESULT OLDRESULT return false; - if (!emit1(JSOP_POP)) // ITER RESULT + if (!emit1(JSOP_POP)) // ITER RESULT return false; - MOZ_ASSERT(this->stackDepth == depth); + MOZ_ASSERT(this->stackDepth == startDepth); JumpList checkResult; - // Note that there is no GOSUB to the finally block here. If the iterator has a - // "throw" method, it does not perform IteratorClose per // ES 14.4.13, YieldExpression : yield * AssignmentExpression, step 5.b.ii. - if (!emitJump(JSOP_GOTO, &checkResult)) // goto checkResult + // + // Note that there is no GOSUB to the finally block here. If the iterator has a + // "throw" method, it does not perform IteratorClose. + if (!emitJump(JSOP_GOTO, &checkResult)) // goto checkResult return false; - // IteratorClose logic. if (!tryCatch.emitFinally()) return false; - if (!emitDupAt(3)) // ITER RESULT FTYPE FVALUE ITER + // ES 14.4.13, yield * AssignmentExpression, step 5.c + // + // Call iterator.return() for receiving a "forced return" completion from + // the generator. + + IfThenElseEmitter ifGeneratorClosing(this); + if (!emit1(JSOP_ISGENCLOSING)) // ITER RESULT FTYPE FVALUE CLOSING + return false; + if (!ifGeneratorClosing.emitIf()) // ITER RESULT FTYPE FVALUE + return false; + + // Step ii. + // + // Get the "return" method. + if (!emitDupAt(3)) // ITER RESULT FTYPE FVALUE ITER + return false; + if (!emit1(JSOP_DUP)) // ITER RESULT FTYPE FVALUE ITER ITER + return false; + if (!emitAtomOp(cx->names().return_, JSOP_CALLPROP)) // ITER RESULT FTYPE FVALUE ITER RET + return false; + + // Step iii. + // + // Do nothing if "return" is undefined. + IfThenElseEmitter ifReturnMethodIsDefined(this); + if (!emit1(JSOP_DUP)) // ITER RESULT FTYPE FVALUE ITER RET RET + return false; + if (!emit1(JSOP_UNDEFINED)) // ITER RESULT FTYPE FVALUE ITER RET RET UNDEFINED + return false; + if (!emit1(JSOP_NE)) // ITER RESULT FTYPE FVALUE ITER RET ?NEQL + return false; + + // Step iv. + // + // Call "return" with the argument passed to Generator.prototype.return, + // which is currently in rval.value. + if (!ifReturnMethodIsDefined.emitIfElse()) // ITER OLDRESULT FTYPE FVALUE ITER RET + return false; + if (!emit1(JSOP_SWAP)) // ITER OLDRESULT FTYPE FVALUE RET ITER + return false; + if (!emit1(JSOP_GETRVAL)) // ITER OLDRESULT FTYPE FVALUE RET ITER RVAL + return false; + if (!emitAtomOp(cx->names().value, JSOP_GETPROP)) // ITER OLDRESULT FTYPE FVALUE RET ITER VALUE + return false; + if (!emitCall(JSOP_CALL, 1)) // ITER OLDRESULT FTYPE FVALUE RESULT + return false; + checkTypeSet(JSOP_CALL); + + // Step v. + if (!emitCheckIsObj(CheckIsObjectKind::IteratorReturn)) // ITER OLDRESULT FTYPE FVALUE RESULT + return false; + + // Steps vi-viii. + // + // Check if the returned object from iterator.return() is done. If not, + // continuing yielding. + IfThenElseEmitter ifReturnDone(this); + if (!emit1(JSOP_DUP)) // ITER OLDRESULT FTYPE FVALUE RESULT RESULT + return false; + if (!emitAtomOp(cx->names().done, JSOP_GETPROP)) // ITER OLDRESULT FTYPE FVALUE RESULT DONE + return false; + if (!ifReturnDone.emitIfElse()) // ITER OLDRESULT FTYPE FVALUE RESULT + return false; + if (!emitAtomOp(cx->names().value, JSOP_GETPROP)) // ITER OLDRESULT FTYPE FVALUE VALUE + return false; + if (!emitPrepareIteratorResult()) // ITER OLDRESULT FTYPE FVALUE VALUE RESULT + return false; + if (!emit1(JSOP_SWAP)) // ITER OLDRESULT FTYPE FVALUE RESULT VALUE + return false; + if (!emitFinishIteratorResult(true)) // ITER OLDRESULT FTYPE FVALUE RESULT + return false; + if (!emit1(JSOP_SETRVAL)) // ITER OLDRESULT FTYPE FVALUE + return false; + savedDepthTemp = this->stackDepth; + if (!ifReturnDone.emitElse()) // ITER OLDRESULT FTYPE FVALUE RESULT + return false; + if (!emit2(JSOP_UNPICK, 3)) // ITER RESULT OLDRESULT FTYPE FVALUE + return false; + if (!emitUint16Operand(JSOP_POPN, 3)) // ITER RESULT + return false; + { + // goto tryStart; + JumpList beq; + JumpTarget breakTarget{ -1 }; + if (!emitBackwardJump(JSOP_GOTO, tryStart, &beq, &breakTarget)) // ITER RESULT + return false; + } + this->stackDepth = savedDepthTemp; + if (!ifReturnDone.emitEnd()) + return false; + + if (!ifReturnMethodIsDefined.emitElse()) // ITER RESULT FTYPE FVALUE ITER RET + return false; + if (!emit1(JSOP_POP)) // ITER RESULT FTYPE FVALUE ITER return false; - if (!emitIteratorClose(Some(tryStart))) // ITER RESULT FTYPE FVALUE + if (!emit1(JSOP_POP)) // ITER RESULT FTYPE FVALUE + return false; + if (!ifReturnMethodIsDefined.emitEnd()) + return false; + + if (!ifGeneratorClosing.emitEnd()) return false; if (!tryCatch.emitEnd()) @@ -8338,7 +8372,7 @@ BytecodeEmitter::emitYieldStar(ParseNode* iter, ParseNode* gen) if (!emitCheckIsObj(CheckIsObjectKind::IteratorNext)) // ITER RESULT return false; checkTypeSet(JSOP_CALL); - MOZ_ASSERT(this->stackDepth == depth); + MOZ_ASSERT(this->stackDepth == startDepth); if (!emitJumpTargetAndPatch(checkResult)) // checkResult: return false; @@ -8349,10 +8383,12 @@ BytecodeEmitter::emitYieldStar(ParseNode* iter, ParseNode* gen) if (!emitAtomOp(cx->names().done, JSOP_GETPROP)) // ITER RESULT DONE return false; // if (!DONE) goto tryStart; - JumpList beq; - JumpTarget breakTarget{ -1 }; - if (!emitBackwardJump(JSOP_IFEQ, tryStart, &beq, &breakTarget)) // ITER RESULT - return false; + { + JumpList beq; + JumpTarget breakTarget{ -1 }; + if (!emitBackwardJump(JSOP_IFEQ, tryStart, &beq, &breakTarget)) // ITER RESULT + return false; + } // result.value if (!emit1(JSOP_SWAP)) // RESULT ITER @@ -8362,7 +8398,7 @@ BytecodeEmitter::emitYieldStar(ParseNode* iter, ParseNode* gen) if (!emitAtomOp(cx->names().value, JSOP_GETPROP)) // VALUE return false; - MOZ_ASSERT(this->stackDepth == depth - 1); + MOZ_ASSERT(this->stackDepth == startDepth - 1); return true; } diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 78eb3510c..2ab6c6fef 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -679,9 +679,7 @@ struct MOZ_STACK_CLASS BytecodeEmitter // Pops iterator from the top of the stack. Pushes the result of |.next()| // onto the stack. MOZ_MUST_USE bool emitIteratorNext(ParseNode* pn, bool allowSelfHosted = false); - MOZ_MUST_USE bool emitIteratorClose( - mozilla::Maybe yieldStarTryStart = mozilla::Nothing(), - bool allowSelfHosted = false); + MOZ_MUST_USE bool emitIteratorClose(bool allowSelfHosted = false); template MOZ_MUST_USE bool wrapWithDestructuringIteratorCloseTryNote(int32_t iterDepth, diff --git a/js/src/tests/ecma_6/Generators/yield-star-iterator-close.js b/js/src/tests/ecma_6/Generators/yield-star-iterator-close.js index ec62dd86d..91ad31cb6 100644 --- a/js/src/tests/ecma_6/Generators/yield-star-iterator-close.js +++ b/js/src/tests/ecma_6/Generators/yield-star-iterator-close.js @@ -6,6 +6,8 @@ function test() { var returnCalledExpected = 0; var nextCalled = 0; var nextCalledExpected = 0; + var throwCalled = 0; + var throwCalledExpected = 0; var iterable = {}; iterable[Symbol.iterator] = makeIterator({ next: function() { @@ -25,9 +27,9 @@ function test() { // G.p.throw on an iterator without "throw" calls IteratorClose. var g1 = y(); g1.next(); - assertThrowsValue(function() { + assertThrowsInstanceOf(function() { g1.throw("foo"); - }, "foo"); + }, TypeError); assertEq(returnCalled, ++returnCalledExpected); assertEq(nextCalled, ++nextCalledExpected); g1.next(); @@ -98,9 +100,18 @@ function test() { // IteratorClose expects iter.return to return an Object. var g6 = y(); g6.next(); - assertThrowsInstanceOf(function() { + var exc; + try { g6.throw("foo"); - }, TypeError); + } catch (e) { + exc = e; + } finally { + assertEq(exc instanceof TypeError, true); + // The message test is here because instanceof TypeError doesn't + // distinguish the non-Object return TypeError and the + // throw-method-is-not-defined iterator protocol error. + assertEq(exc.toString().indexOf("non-object") > 0, true); + } assertEq(returnCalled, ++returnCalledExpected); // G.p.return passes its argument to "return". @@ -115,6 +126,25 @@ function test() { g7.next(); g7.return("in test"); assertEq(returnCalled, ++returnCalledExpected); + + // If a throw method is present, do not call "return". + iterable[Symbol.iterator] = makeIterator({ + throw: function(e) { + throwCalled++; + throw e; + }, + ret: function(x) { + returnCalled++; + return { done: true }; + } + }); + var g8 = y(); + g8.next(); + assertThrowsValue(function() { + g8.throw("foo"); + }, "foo"); + assertEq(throwCalled, ++throwCalledExpected); + assertEq(returnCalled, returnCalledExpected); } test(); diff --git a/js/src/tests/ecma_6/shell.js b/js/src/tests/ecma_6/shell.js index 4da9221d6..756da9f36 100644 --- a/js/src/tests/ecma_6/shell.js +++ b/js/src/tests/ecma_6/shell.js @@ -21,10 +21,11 @@ /** Make an iterator with a return method. */ global.makeIterator = function makeIterator(overrides) { + var throwMethod; + if (overrides && overrides.throw) + throwMethod = overrides.throw; var iterator = { - throw: function(e) { - throw e; - }, + throw: throwMethod, next: function(x) { if (overrides && overrides.next) return overrides.next(x); -- cgit v1.2.3