From 7d753c1a8f22f85f6279a3c016034ce8f8e740f7 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Sat, 24 Mar 2018 12:09:30 +0100 Subject: Bug 1147371: Implement IteratorClose for for-of Issue #74 --- js/src/builtin/TypedArray.js | 4 +- js/src/frontend/BytecodeEmitter.cpp | 619 +++++++++++++++------ js/src/frontend/BytecodeEmitter.h | 6 + js/src/jit/JitFrames.cpp | 39 +- js/src/js.msg | 2 +- js/src/jsiter.cpp | 52 ++ js/src/jsiter.h | 3 + js/src/jsscript.h | 3 +- js/src/shell/js.cpp | 9 +- .../ecma_6/Statements/for-of-iterator-close.js | 102 ++++ js/src/tests/ecma_6/shell.js | 48 +- js/src/vm/Interpreter.cpp | 19 +- js/src/vm/Interpreter.h | 1 + 13 files changed, 704 insertions(+), 203 deletions(-) create mode 100644 js/src/tests/ecma_6/Statements/for-of-iterator-close.js (limited to 'js/src') diff --git a/js/src/builtin/TypedArray.js b/js/src/builtin/TypedArray.js index 4a3f38365..a2205dc92 100644 --- a/js/src/builtin/TypedArray.js +++ b/js/src/builtin/TypedArray.js @@ -1428,7 +1428,7 @@ function TypedArrayStaticFrom(source, mapfn = undefined, thisArg = undefined) { // 22.2.2.1.1 IterableToList, step 4.a. var next = callContentFunction(iterator.next, iterator); if (!IsObject(next)) - ThrowTypeError(JSMSG_NEXT_RETURNED_PRIMITIVE); + ThrowTypeError(JSMSG_ITER_METHOD_RETURNED_PRIMITIVE, "next"); // 22.2.2.1.1 IterableToList, step 4.b. if (next.done) @@ -1555,7 +1555,7 @@ function IterableToList(items, method) { // Step 4.a. var next = callContentFunction(iterator.next, iterator); if (!IsObject(next)) - ThrowTypeError(JSMSG_NEXT_RETURNED_PRIMITIVE); + ThrowTypeError(JSMSG_ITER_METHOD_RETURNED_PRIMITIVE, "next"); // Step 4.b. if (next.done) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index a4cfeb753..fb141e92d 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -58,6 +58,7 @@ using mozilla::Some; class BreakableControl; class LabelControl; class LoopControl; +class ForOfLoopControl; class TryFinallyControl; static bool @@ -150,6 +151,13 @@ BytecodeEmitter::NestableControl::is() const return StatementKindIsLoop(kind_); } +template <> +bool +BytecodeEmitter::NestableControl::is() const +{ + return kind_ == StatementKind::ForOfLoop; +} + template <> bool BytecodeEmitter::NestableControl::is() const @@ -270,6 +278,64 @@ class LoopControl : public BreakableControl } }; +class ForOfLoopControl : public LoopControl +{ + // The stack depth of the iterator. + int32_t iterDepth_; + + // for-of loops, when throwing from non-iterator code (i.e. from the body + // or from evaluating the LHS of the loop condition), need to call + // IteratorClose. If IteratorClose itself throws, we must not re-call + // IteratorClose. Since non-local jumps like break and return call + // IteratorClose, whenever a non-local jump is emitted, we must terminate + // the current JSTRY_ITERCLOSE note to skip the non-local jump code, then + // start a new one. + // + // Visually, + // + // for (x of y) { + // ... instantiate ForOfLoopControl + // ... + <-- iterCloseTryStart_ points to right before + // ... assignment to loop variable + // ... ^ + // ... | + // if (...) v + // + call finishIterCloseTryNote before |break| + // above range is noted with JSTRY_ITERCLOSE + // + // break; <-- break and IteratorClose are not inside + // JSTRY_ITERCLOSE note + // + // call startNewIterCloseTryNote after |break| + // + <-- next iterCloseTryStart_ points here + // ... | + // ... ~ + // } + ptrdiff_t iterCloseTryStart_; + + public: + ForOfLoopControl(BytecodeEmitter* bce, int32_t iterDepth) + : LoopControl(bce, StatementKind::ForOfLoop), + iterDepth_(iterDepth), + iterCloseTryStart_(-1) + { + MOZ_ASSERT(bce->stackDepth >= iterDepth); + } + + MOZ_MUST_USE bool finishIterCloseTryNote(BytecodeEmitter* bce) { + ptrdiff_t end = bce->offset(); + MOZ_ASSERT(end >= iterCloseTryStart_); + if (end != iterCloseTryStart_) + return bce->tryNoteList.append(JSTRY_ITERCLOSE, iterDepth_, iterCloseTryStart_, end); + return true; + } + + void startNewIterCloseTryNote(BytecodeEmitter* bce) { + MOZ_ASSERT(bce->offset() > iterCloseTryStart_); + iterCloseTryStart_ = bce->offset(); + } +}; + class TryFinallyControl : public BytecodeEmitter::NestableControl { bool emittingSubroutine_; @@ -1497,6 +1563,156 @@ BytecodeEmitter::TDZCheckCache::noteTDZCheck(BytecodeEmitter* bce, JSAtom* name, return true; } +class MOZ_STACK_CLASS IfThenElseEmitter +{ + BytecodeEmitter* bce_; + JumpList jumpAroundThen_; + JumpList jumpsAroundElse_; + unsigned noteIndex_; + int32_t thenDepth_; +#ifdef DEBUG + int32_t pushed_; + bool calculatedPushed_; +#endif + enum State { + Start, + If, + Cond, + IfElse, + Else, + End + }; + State state_; + + public: + explicit IfThenElseEmitter(BytecodeEmitter* bce) + : bce_(bce), + noteIndex_(-1), + thenDepth_(0), +#ifdef DEBUG + pushed_(0), + calculatedPushed_(false), +#endif + state_(Start) + {} + + ~IfThenElseEmitter() + {} + + private: + bool emitIf(State nextState) { + MOZ_ASSERT(state_ == Start || state_ == Else); + MOZ_ASSERT(nextState == If || nextState == IfElse || nextState == Cond); + + // Clear jumpAroundThen_ offset that points previous JSOP_IFEQ. + if (state_ == Else) + jumpAroundThen_ = JumpList(); + + // Emit an annotated branch-if-false around the then part. + SrcNoteType type = nextState == If ? SRC_IF : nextState == IfElse ? SRC_IF_ELSE : SRC_COND; + if (!bce_->newSrcNote(type, ¬eIndex_)) + return false; + if (!bce_->emitJump(JSOP_IFEQ, &jumpAroundThen_)) + return false; + + // To restore stack depth in else part, save depth of the then part. +#ifdef DEBUG + // If DEBUG, this is also necessary to calculate |pushed_|. + thenDepth_ = bce_->stackDepth; +#else + if (nextState == IfElse || nextState == Cond) + thenDepth_ = bce_->stackDepth; +#endif + state_ = nextState; + return true; + } + + public: + bool emitIf() { + return emitIf(If); + } + + bool emitCond() { + return emitIf(Cond); + } + + bool emitIfElse() { + return emitIf(IfElse); + } + + bool emitElse() { + MOZ_ASSERT(state_ == IfElse || state_ == Cond); + + calculateOrCheckPushed(); + + // Emit a jump from the end of our then part around the else part. The + // patchJumpsToTarget call at the bottom of this function will fix up + // the offset with jumpsAroundElse value. + if (!bce_->emitJump(JSOP_GOTO, &jumpsAroundElse_)) + return false; + + // Ensure the branch-if-false comes here, then emit the else. + if (!bce_->emitJumpTargetAndPatch(jumpAroundThen_)) + return false; + + // Annotate SRC_IF_ELSE or SRC_COND with the offset from branch to + // jump, for IonMonkey's benefit. We can't just "back up" from the pc + // of the else clause, because we don't know whether an extended + // jump was required to leap from the end of the then clause over + // the else clause. + if (!bce_->setSrcNoteOffset(noteIndex_, 0, + jumpsAroundElse_.offset - jumpAroundThen_.offset)) + { + return false; + } + + // Restore stack depth of the then part. + bce_->stackDepth = thenDepth_; + state_ = Else; + return true; + } + + bool emitEnd() { + MOZ_ASSERT(state_ == If || state_ == Else); + + calculateOrCheckPushed(); + + if (state_ == If) { + // No else part, fixup the branch-if-false to come here. + if (!bce_->emitJumpTargetAndPatch(jumpAroundThen_)) + return false; + } + + // Patch all the jumps around else parts. + if (!bce_->emitJumpTargetAndPatch(jumpsAroundElse_)) + return false; + + state_ = End; + return true; + } + + void calculateOrCheckPushed() { +#ifdef DEBUG + if (!calculatedPushed_) { + pushed_ = bce_->stackDepth - thenDepth_; + calculatedPushed_ = true; + } else { + MOZ_ASSERT(pushed_ == bce_->stackDepth - thenDepth_); + } +#endif + } + +#ifdef DEBUG + int32_t pushed() const { + return pushed_; + } + + int32_t popped() const { + return -pushed_; + } +#endif +}; + BytecodeEmitter::BytecodeEmitter(BytecodeEmitter* parent, Parser* parser, SharedContext* sc, HandleScript script, Handle lazyScript, @@ -2013,22 +2229,43 @@ BytecodeEmitter::flushPops(int* npops) namespace { -class NonLocalExitControl { +class NonLocalExitControl +{ + public: + enum Kind + { + // IteratorClose is handled especially inside the exception unwinder. + Throw, + + // A 'continue' statement does not call IteratorClose for the loop it + // is continuing, i.e. excluding the target loop. + Continue, + + // A 'break' or 'return' statement does call IteratorClose for the + // loop it is breaking out of or returning from, i.e. including the + // target loop. + Break, + Return + }; + + private: BytecodeEmitter* bce_; const uint32_t savedScopeNoteIndex_; const int savedDepth_; uint32_t openScopeNoteIndex_; + Kind kind_; NonLocalExitControl(const NonLocalExitControl&) = delete; MOZ_MUST_USE bool leaveScope(BytecodeEmitter::EmitterScope* scope); public: - explicit NonLocalExitControl(BytecodeEmitter* bce) + NonLocalExitControl(BytecodeEmitter* bce, Kind kind) : bce_(bce), savedScopeNoteIndex_(bce->scopeNoteList.length()), savedDepth_(bce->stackDepth), - openScopeNoteIndex_(bce->innermostEmitterScope->noteIndex()) + openScopeNoteIndex_(bce->innermostEmitterScope->noteIndex()), + kind_(kind) { } ~NonLocalExitControl() { @@ -2075,6 +2312,14 @@ NonLocalExitControl::prepareForNonLocalJump(BytecodeEmitter::NestableControl* ta EmitterScope* es = bce_->innermostEmitterScope; int npops = 0; + bool hasForOfLoopsWithIteratorClose = false; + + // IteratorClose is handled specially in the exception unwinder. For + // 'continue', 'break', and 'return' statements, emit IteratorClose + // bytecode inline. 'continue' statements do not call IteratorClose for + // the loop they are continuing. + bool emitIteratorClose = kind_ == Continue || kind_ == Break || kind_ == Return; + bool emitIteratorCloseAtTarget = emitIteratorClose && kind_ != Continue; auto flushPops = [&npops](BytecodeEmitter* bce) { if (npops && !bce->flushPops(&npops)) @@ -2114,15 +2359,29 @@ NonLocalExitControl::prepareForNonLocalJump(BytecodeEmitter::NestableControl* ta } case StatementKind::ForOfLoop: - npops += 2; + // The iterator and the current value are on the stack. + // + if (emitIteratorClose) { + hasForOfLoopsWithIteratorClose = true; + if (!control->as().finishIterCloseTryNote(bce_)) + return false; + if (!bce_->emit1(JSOP_POP)) // ... ITER + return false; + if (!bce_->emitIteratorClose()) // ... + return false; + } else { + if (!bce_->emit1(JSOP_POP)) // ... ITER + return false; + if (!bce_->emit1(JSOP_POP)) // ... + return false; + } break; case StatementKind::ForInLoop: - /* The iterator and the current value are on the stack. */ - npops += 1; - if (!flushPops(bce_)) + // The iterator and the current value are on the stack. + if (!bce_->emit1(JSOP_POP)) // ... ITER return false; - if (!bce_->emit1(JSOP_ENDITER)) + if (!bce_->emit1(JSOP_ENDITER)) // ... return false; break; @@ -2131,13 +2390,45 @@ NonLocalExitControl::prepareForNonLocalJump(BytecodeEmitter::NestableControl* ta } } + if (target && target->is() && emitIteratorCloseAtTarget) { + hasForOfLoopsWithIteratorClose = true; + if (!target->as().finishIterCloseTryNote(bce_)) + return false; + + // The iterator and the current value are on the stack. At the level + // of the target block, there's bytecode after the loop that will pop + // the iterator and the value, so duplicate the iterator and call + // IteratorClose. + if (!bce_->emitDupAt(1)) // ... ITER RESULT ITER + return false; + if (!bce_->emitIteratorClose()) // ... ITER RESULT + return false; + } + EmitterScope* targetEmitterScope = target ? target->emitterScope() : bce_->varEmitterScope; for (; es != targetEmitterScope; es = es->enclosingInFrame()) { if (!leaveScope(es)) return false; } - return flushPops(bce_); + if (!flushPops(bce_)) + return false; + + // See comment in ForOfLoopControl. + if (hasForOfLoopsWithIteratorClose) { + for (NestableControl* control = bce_->innermostNestableControl; + control != target; + control = control->enclosing()) + { + if (control->is()) + control->as().startNewIterCloseTryNote(bce_); + } + + if (target && target->is() && emitIteratorCloseAtTarget) + target->as().startNewIterCloseTryNote(bce_); + } + + return true; } } // anonymous namespace @@ -2145,7 +2436,9 @@ NonLocalExitControl::prepareForNonLocalJump(BytecodeEmitter::NestableControl* ta bool BytecodeEmitter::emitGoto(NestableControl* target, JumpList* jumplist, SrcNoteType noteType) { - NonLocalExitControl nle(this); + NonLocalExitControl nle(this, noteType == SRC_CONTINUE + ? NonLocalExitControl::Continue + : NonLocalExitControl::Break); if (!nle.prepareForNonLocalJump(target)) return false; @@ -4529,6 +4822,144 @@ BytecodeEmitter::emitIteratorNext(ParseNode* pn, bool allowSelfHosted) return true; } +bool +BytecodeEmitter::emitIteratorClose(Maybe yieldStarTryStart, bool allowSelfHosted) +{ + MOZ_ASSERT(allowSelfHosted || emitterMode != BytecodeEmitter::SelfHosting, + ".close() on iterators is prohibited in self-hosted code because it " + "can run user-modifiable iteration code"); + + // Generate inline logic corresponding to IteratorClose (ES 7.4.6). + // + // Callers need to ensure that the iterator object is at the top of the + // stack. + + if (!emit1(JSOP_DUP)) // ... ITER ITER + return false; + + // Step 3. + // + // Get the "return" method. + if (!emitAtomOp(cx->names().return_, JSOP_CALLPROP)) // ... ITER RET + return false; + + // Step 4. + // + // Do nothing if "return" is null or undefined. + IfThenElseEmitter ifReturnMethodIsDefined(this); + if (!emit1(JSOP_DUP)) // ... ITER RET RET + return false; + if (!emit1(JSOP_UNDEFINED)) // ... ITER RET RET UNDEFINED + return false; + if (!emit1(JSOP_NE)) // ... ITER RET ?NEQL + return false; + if (!ifReturnMethodIsDefined.emitIfElse()) + return false; + + // Steps 5, 8. + // + // Call "return" if it is not undefined or null, and check that it returns + // 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 (!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 (!ifReturnMethodIsDefined.emitElse()) + return false; + if (!emit1(JSOP_POP)) // ... ITER + return false; + if (!ifReturnMethodIsDefined.emitEnd()) + return false; + + return emit1(JSOP_POP); // ... +} + +template +bool +BytecodeEmitter::wrapWithIteratorCloseTryNote(int32_t iterDepth, InnerEmitter emitter) +{ + MOZ_ASSERT(this->stackDepth >= iterDepth); + + ptrdiff_t start = offset(); + if (!emitter(this)) + return false; + ptrdiff_t end = offset(); + if (start != end) + return tryNoteList.append(JSTRY_ITERCLOSE, iterDepth, start, end); + return true; +} + bool BytecodeEmitter::emitDefault(ParseNode* defaultExpr, ParseNode* pattern) { @@ -4617,156 +5048,6 @@ BytecodeEmitter::emitInitializerInBranch(ParseNode* initializer, ParseNode* patt return emitInitializer(initializer, pattern); } -class MOZ_STACK_CLASS IfThenElseEmitter -{ - BytecodeEmitter* bce_; - JumpList jumpAroundThen_; - JumpList jumpsAroundElse_; - unsigned noteIndex_; - int32_t thenDepth_; -#ifdef DEBUG - int32_t pushed_; - bool calculatedPushed_; -#endif - enum State { - Start, - If, - Cond, - IfElse, - Else, - End - }; - State state_; - - public: - explicit IfThenElseEmitter(BytecodeEmitter* bce) - : bce_(bce), - noteIndex_(-1), - thenDepth_(0), -#ifdef DEBUG - pushed_(0), - calculatedPushed_(false), -#endif - state_(Start) - {} - - ~IfThenElseEmitter() - {} - - private: - bool emitIf(State nextState) { - MOZ_ASSERT(state_ == Start || state_ == Else); - MOZ_ASSERT(nextState == If || nextState == IfElse || nextState == Cond); - - // Clear jumpAroundThen_ offset that points previous JSOP_IFEQ. - if (state_ == Else) - jumpAroundThen_ = JumpList(); - - // Emit an annotated branch-if-false around the then part. - SrcNoteType type = nextState == If ? SRC_IF : nextState == IfElse ? SRC_IF_ELSE : SRC_COND; - if (!bce_->newSrcNote(type, ¬eIndex_)) - return false; - if (!bce_->emitJump(JSOP_IFEQ, &jumpAroundThen_)) - return false; - - // To restore stack depth in else part, save depth of the then part. -#ifdef DEBUG - // If DEBUG, this is also necessary to calculate |pushed_|. - thenDepth_ = bce_->stackDepth; -#else - if (nextState == IfElse || nextState == Cond) - thenDepth_ = bce_->stackDepth; -#endif - state_ = nextState; - return true; - } - - public: - bool emitIf() { - return emitIf(If); - } - - bool emitCond() { - return emitIf(Cond); - } - - bool emitIfElse() { - return emitIf(IfElse); - } - - bool emitElse() { - MOZ_ASSERT(state_ == IfElse || state_ == Cond); - - calculateOrCheckPushed(); - - // Emit a jump from the end of our then part around the else part. The - // patchJumpsToTarget call at the bottom of this function will fix up - // the offset with jumpsAroundElse value. - if (!bce_->emitJump(JSOP_GOTO, &jumpsAroundElse_)) - return false; - - // Ensure the branch-if-false comes here, then emit the else. - if (!bce_->emitJumpTargetAndPatch(jumpAroundThen_)) - return false; - - // Annotate SRC_IF_ELSE or SRC_COND with the offset from branch to - // jump, for IonMonkey's benefit. We can't just "back up" from the pc - // of the else clause, because we don't know whether an extended - // jump was required to leap from the end of the then clause over - // the else clause. - if (!bce_->setSrcNoteOffset(noteIndex_, 0, - jumpsAroundElse_.offset - jumpAroundThen_.offset)) - { - return false; - } - - // Restore stack depth of the then part. - bce_->stackDepth = thenDepth_; - state_ = Else; - return true; - } - - bool emitEnd() { - MOZ_ASSERT(state_ == If || state_ == Else); - - calculateOrCheckPushed(); - - if (state_ == If) { - // No else part, fixup the branch-if-false to come here. - if (!bce_->emitJumpTargetAndPatch(jumpAroundThen_)) - return false; - } - - // Patch all the jumps around else parts. - if (!bce_->emitJumpTargetAndPatch(jumpsAroundElse_)) - return false; - - state_ = End; - return true; - } - - void calculateOrCheckPushed() { -#ifdef DEBUG - if (!calculatedPushed_) { - pushed_ = bce_->stackDepth - thenDepth_; - calculatedPushed_ = true; - } else { - MOZ_ASSERT(pushed_ == bce_->stackDepth - thenDepth_); - } -#endif - } - -#ifdef DEBUG - int32_t pushed() const { - return pushed_; - } - - int32_t popped() const { - return -pushed_; - } -#endif -}; - bool BytecodeEmitter::emitDestructuringOpsArray(ParseNode* pattern, DestructuringFlavor flav) { @@ -5710,7 +5991,7 @@ BytecodeEmitter::emitCatch(ParseNode* pn) return false; { - NonLocalExitControl nle(this); + NonLocalExitControl nle(this, NonLocalExitControl::Throw); // Move exception back to cx->exception to prepare for // the next catch. @@ -6303,12 +6584,14 @@ BytecodeEmitter::emitForOf(ParseNode* forOfLoop, EmitterScope* headLexicalEmitte if (!emitIterator()) // ITER return false; + int32_t iterDepth = stackDepth; + // For-of loops have both the iterator and the value on the stack. Push // undefined to balance the stack. if (!emit1(JSOP_UNDEFINED)) // ITER RESULT return false; - LoopControl loopInfo(this, StatementKind::ForOfLoop); + ForOfLoopControl loopInfo(this, iterDepth); // Annotate so IonMonkey can find the loop-closing jump. unsigned noteIndex; @@ -6354,18 +6637,23 @@ BytecodeEmitter::emitForOf(ParseNode* forOfLoop, EmitterScope* headLexicalEmitte #endif // Emit code to assign result.value to the iteration variable. + // + // Note that ES 13.7.5.13, step 5.c says getting result.value does not + // call IteratorClose, so start JSTRY_ITERCLOSE after the GETPROP. if (!emit1(JSOP_DUP)) // ITER RESULT RESULT return false; if (!emitAtomOp(cx->names().value, JSOP_GETPROP)) // ITER RESULT VALUE return false; + loopInfo.startNewIterCloseTryNote(this); + if (!emitInitializeForInOrOfTarget(forOfHead)) // ITER RESULT VALUE return false; if (!emit1(JSOP_POP)) // ITER RESULT return false; - MOZ_ASSERT(this->stackDepth == loopDepth, + MOZ_ASSERT(stackDepth == loopDepth, "the stack must be balanced around the initializing " "operation"); @@ -6374,6 +6662,9 @@ BytecodeEmitter::emitForOf(ParseNode* forOfLoop, EmitterScope* headLexicalEmitte if (!emitTree(forBody)) // ITER RESULT return false; + if (!loopInfo.finishIterCloseTryNote(this)) + return false; + // Set offset for continues. loopInfo.continueTarget = { offset() }; @@ -7609,7 +7900,7 @@ BytecodeEmitter::emitReturn(ParseNode* pn) return false; } - NonLocalExitControl nle(this); + NonLocalExitControl nle(this, NonLocalExitControl::Return); if (!nle.prepareForNonLocalJumpToOutermost()) return false; diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 066c06672..6b39069de 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -679,6 +679,12 @@ 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); + + template + MOZ_MUST_USE bool wrapWithIteratorCloseTryNote(int32_t iterDepth, InnerEmitter emitter); // Check if the value on top of the stack is "undefined". If so, replace // that value on the stack with the value defined by |defaultExpr|. diff --git a/js/src/jit/JitFrames.cpp b/js/src/jit/JitFrames.cpp index 646442b4c..d6c6fc4c9 100644 --- a/js/src/jit/JitFrames.cpp +++ b/js/src/jit/JitFrames.cpp @@ -328,11 +328,15 @@ NumArgAndLocalSlots(const InlineFrameIterator& frame) } static void -CloseLiveIteratorIon(JSContext* cx, const InlineFrameIterator& frame, uint32_t stackSlot) +CloseLiveIteratorIon(JSContext* cx, const InlineFrameIterator& frame, JSTryNote* tn) { + MOZ_ASSERT(tn->kind == JSTRY_FOR_IN || tn->kind == JSTRY_ITERCLOSE); + MOZ_ASSERT(tn->stackDepth > 0); + SnapshotIterator si = frame.snapshotIterator(); // Skip stack slots until we reach the iterator object. + uint32_t stackSlot = tn->stackDepth; uint32_t skipSlots = NumArgAndLocalSlots(frame) + stackSlot - 1; for (unsigned i = 0; i < skipSlots; i++) @@ -341,10 +345,14 @@ CloseLiveIteratorIon(JSContext* cx, const InlineFrameIterator& frame, uint32_t s Value v = si.read(); RootedObject obj(cx, &v.toObject()); - if (cx->isExceptionPending()) - UnwindIteratorForException(cx, obj); - else + if (cx->isExceptionPending()) { + if (tn->kind == JSTRY_FOR_IN) + UnwindIteratorForException(cx, obj); + else + IteratorCloseForException(cx, obj); + } else { UnwindIteratorForUncatchableException(cx, obj); + } } class IonFrameStackDepthOp @@ -417,12 +425,13 @@ HandleExceptionIon(JSContext* cx, const InlineFrameIterator& frame, ResumeFromEx JSTryNote* tn = *tni; switch (tn->kind) { - case JSTRY_FOR_IN: { - MOZ_ASSERT(JSOp(*(script->main() + tn->start + tn->length)) == JSOP_ENDITER); + case JSTRY_FOR_IN: + case JSTRY_ITERCLOSE: { + MOZ_ASSERT_IF(tn->kind == JSTRY_FOR_IN, + JSOp(*(script->main() + tn->start + tn->length)) == JSOP_ENDITER); MOZ_ASSERT(tn->stackDepth > 0); - uint32_t localSlot = tn->stackDepth; - CloseLiveIteratorIon(cx, frame, localSlot); + CloseLiveIteratorIon(cx, frame, tn); break; } @@ -598,17 +607,23 @@ ProcessTryNotesBaseline(JSContext* cx, const JitFrameIterator& frame, Environmen return true; } - case JSTRY_FOR_IN: { + case JSTRY_FOR_IN: + case JSTRY_ITERCLOSE: { uint8_t* framePointer; uint8_t* stackPointer; BaselineFrameAndStackPointersFromTryNote(tn, frame, &framePointer, &stackPointer); - Value iterValue(*(Value*) stackPointer); + Value iterValue(*(reinterpret_cast(stackPointer))); RootedObject iterObject(cx, &iterValue.toObject()); - if (!UnwindIteratorForException(cx, iterObject)) { + bool ok; + if (tn->kind == JSTRY_FOR_IN) + ok = UnwindIteratorForException(cx, iterObject); + else + ok = IteratorCloseForException(cx, iterObject); + if (!ok) { // See comment in the JSTRY_FOR_IN case in Interpreter.cpp's // ProcessTryNotes. SettleOnTryNote(cx, tn, frame, ei, rfe, pc); - MOZ_ASSERT(**pc == JSOP_ENDITER); + MOZ_ASSERT_IF(tn->kind == JSTRY_FOR_IN, **pc == JSOP_ENDITER); return false; } break; diff --git a/js/src/js.msg b/js/src/js.msg index 8766bfbf5..30c0d3035 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -96,7 +96,7 @@ MSG_DEF(JSMSG_NOT_ITERABLE, 1, JSEXN_TYPEERR, "{0} is not iterable") MSG_DEF(JSMSG_NOT_ITERATOR, 1, JSEXN_TYPEERR, "{0} is not iterator") MSG_DEF(JSMSG_ALREADY_HAS_PRAGMA, 2, JSEXN_WARN, "{0} is being assigned a {1}, but already has one") MSG_DEF(JSMSG_GET_ITER_RETURNED_PRIMITIVE, 0, JSEXN_TYPEERR, "[Symbol.iterator]() returned a non-object value") -MSG_DEF(JSMSG_NEXT_RETURNED_PRIMITIVE, 0, JSEXN_TYPEERR, "iterator.next() returned a non-object value") +MSG_DEF(JSMSG_ITER_METHOD_RETURNED_PRIMITIVE, 1, JSEXN_TYPEERR, "iterator.{0}() returned a non-object value") MSG_DEF(JSMSG_CANT_SET_PROTO, 0, JSEXN_TYPEERR, "can't set prototype of this object") MSG_DEF(JSMSG_CANT_SET_PROTO_OF, 1, JSEXN_TYPEERR, "can't set prototype of {0}") MSG_DEF(JSMSG_CANT_SET_PROTO_CYCLE, 0, JSEXN_TYPEERR, "can't set prototype: it would cause a prototype chain cycle") diff --git a/js/src/jsiter.cpp b/js/src/jsiter.cpp index 63c81e6af..c1ae5dc15 100644 --- a/js/src/jsiter.cpp +++ b/js/src/jsiter.cpp @@ -12,6 +12,7 @@ #include "mozilla/Maybe.h" #include "mozilla/MemoryReporting.h" #include "mozilla/PodOperations.h" +#include "mozilla/Unused.h" #include "jsarray.h" #include "jsatom.h" @@ -1229,6 +1230,7 @@ js::CloseIterator(JSContext* cx, HandleObject obj) } return LegacyGeneratorObject::close(cx, obj); } + return true; } @@ -1246,6 +1248,56 @@ js::UnwindIteratorForException(JSContext* cx, HandleObject obj) return true; } +bool +js::IteratorCloseForException(JSContext* cx, HandleObject obj) +{ + MOZ_ASSERT(cx->isExceptionPending()); + + bool isClosingGenerator = cx->isClosingGenerator(); + JS::AutoSaveExceptionState savedExc(cx); + + // Implements IteratorClose (ES 7.4.6) for exception unwinding. See + // also the bytecode generated by BytecodeEmitter::emitIteratorClose. + + // Step 3. + // + // Get the "return" method. + RootedValue returnMethod(cx); + if (!GetProperty(cx, obj, obj, cx->names().return_, &returnMethod)) + return false; + + // Step 4. + // + // Do nothing if "return" is null or undefined. Throw a TypeError if the + // method is not IsCallable. + if (returnMethod.isNullOrUndefined()) + return true; + if (!IsCallable(returnMethod)) + return ReportIsNotFunction(cx, returnMethod); + + // Step 5, 6, 8. + // + // Call "return" if it is not null or undefined. + RootedValue rval(cx); + bool ok = Call(cx, returnMethod, obj, &rval); + if (isClosingGenerator) { + // Closing an iterator is implemented as an exception, but in spec + // terms it is a Completion value with [[Type]] return. In this case + // we *do* care if the call threw and if it returned an object. + if (!ok) + return false; + if (!rval.isObject()) + return ThrowCheckIsObject(cx, CheckIsObjectKind::IteratorReturn); + } else { + // We don't care if the call threw or that it returned an Object, as + // Step 6 says if IteratorClose is being called during a throw, the + // original throw has primacy. + savedExc.restore(); + } + + return true; +} + void js::UnwindIteratorForUncatchableException(JSContext* cx, JSObject* obj) { diff --git a/js/src/jsiter.h b/js/src/jsiter.h index 35d7ef118..f11f09b55 100644 --- a/js/src/jsiter.h +++ b/js/src/jsiter.h @@ -189,6 +189,9 @@ CloseIterator(JSContext* cx, HandleObject iterObj); bool UnwindIteratorForException(JSContext* cx, HandleObject obj); +bool +IteratorCloseForException(JSContext* cx, HandleObject obj); + void UnwindIteratorForUncatchableException(JSContext* cx, JSObject* obj); diff --git a/js/src/jsscript.h b/js/src/jsscript.h index 690bc225d..b48d48686 100644 --- a/js/src/jsscript.h +++ b/js/src/jsscript.h @@ -84,7 +84,8 @@ enum JSTryNoteKind { JSTRY_FINALLY, JSTRY_FOR_IN, JSTRY_FOR_OF, - JSTRY_LOOP + JSTRY_LOOP, + JSTRY_ITERCLOSE }; /* diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 3f56981cd..294dd935d 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -2589,7 +2589,8 @@ JS_STATIC_ASSERT(JSTRY_CATCH == 0); JS_STATIC_ASSERT(JSTRY_FINALLY == 1); JS_STATIC_ASSERT(JSTRY_FOR_IN == 2); -static const char* const TryNoteNames[] = { "catch", "finally", "for-in", "for-of", "loop" }; +static const char* const TryNoteNames[] = { "catch", "finally", "for-in", "for-of", "loop", + "iterclose" }; static MOZ_MUST_USE bool TryNotes(JSContext* cx, HandleScript script, Sprinter* sp) @@ -2597,15 +2598,15 @@ TryNotes(JSContext* cx, HandleScript script, Sprinter* sp) if (!script->hasTrynotes()) return true; - if (sp->put("\nException table:\nkind stack start end\n") < 0) + if (sp->put("\nException table:\nkind stack start end\n") < 0) return false; JSTryNote* tn = script->trynotes()->vector; JSTryNote* tnlimit = tn + script->trynotes()->length; do { MOZ_ASSERT(tn->kind < ArrayLength(TryNoteNames)); - uint8_t startOff = script->pcToOffset(script->main()) + tn->start; - if (!sp->jsprintf(" %-7s %6u %8u %8u\n", + uint32_t startOff = script->pcToOffset(script->main()) + tn->start; + if (!sp->jsprintf(" %-9s %6u %8u %8u\n", TryNoteNames[tn->kind], tn->stackDepth, startOff, startOff + tn->length)) { diff --git a/js/src/tests/ecma_6/Statements/for-of-iterator-close.js b/js/src/tests/ecma_6/Statements/for-of-iterator-close.js new file mode 100644 index 000000000..b28895d8f --- /dev/null +++ b/js/src/tests/ecma_6/Statements/for-of-iterator-close.js @@ -0,0 +1,102 @@ +// Tests that IteratorReturn is called when a for-of loop has an abrupt +// completion value during non-iterator code. + +function test() { + var returnCalled = 0; + var returnCalledExpected = 0; + var iterable = {}; + iterable[Symbol.iterator] = makeIterator({ + ret: function() { + returnCalled++; + return {}; + } + }); + + // break calls iter.return + for (var x of iterable) + break; + assertEq(returnCalled, ++returnCalledExpected); + + // throw in body calls iter.return + assertThrowsValue(function() { + for (var x of iterable) + throw "in body"; + }, "in body"); + assertEq(returnCalled, ++returnCalledExpected); + + // throw in lhs ref calls iter.return + function throwlhs() { + throw "in lhs"; + } + assertThrowsValue(function() { + for ((throwlhs()) of iterable) + continue; + }, "in lhs"); + assertEq(returnCalled, ++returnCalledExpected); + + // throw in iter.return doesn't re-call iter.return + iterable[Symbol.iterator] = makeIterator({ + ret: function() { + returnCalled++; + throw "in iter.return"; + } + }); + assertThrowsValue(function() { + for (var x of iterable) + break; + }, "in iter.return"); + assertEq(returnCalled, ++returnCalledExpected); + + // throw in iter.next doesn't call IteratorClose + iterable[Symbol.iterator] = makeIterator({ + next: function() { + throw "in next"; + } + }); + assertThrowsValue(function() { + for (var x of iterable) + break; + }, "in next"); + assertEq(returnCalled, returnCalledExpected); + + // "return" must return an Object. + iterable[Symbol.iterator] = makeIterator({ + ret: function() { + returnCalled++; + return 42; + } + }); + assertThrowsInstanceOf(function() { + for (var x of iterable) + break; + }, TypeError); + assertEq(returnCalled, ++returnCalledExpected); + + // continue doesn't call iter.return for the loop it's continuing + var i = 0; + iterable[Symbol.iterator] = makeIterator({ + next: function() { + return { done: i++ > 5 }; + }, + ret: function() { + returnCalled++; + return {}; + } + }); + for (var x of iterable) + continue; + assertEq(returnCalled, returnCalledExpected); + + // continue does call iter.return for loops it skips + i = 0; + L: do { + for (var x of iterable) + continue L; + } while (false); + assertEq(returnCalled, ++returnCalledExpected); +} + +test(); + +if (typeof reportCompare === "function") + reportCompare(0, 0); diff --git a/js/src/tests/ecma_6/shell.js b/js/src/tests/ecma_6/shell.js index 06be6f2db..1d067a282 100644 --- a/js/src/tests/ecma_6/shell.js +++ b/js/src/tests/ecma_6/shell.js @@ -3,21 +3,39 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ (function(global) { - /** Yield every permutation of the elements in some array. */ - global.Permutations = function* Permutations(items) { - if (items.length == 0) { - yield []; - } else { - items = items.slice(0); - for (let i = 0; i < items.length; i++) { - let swap = items[0]; - items[0] = items[i]; - items[i] = swap; - for (let e of Permutations(items.slice(1, items.length))) - yield [items[0]].concat(e); - } - } - }; + /** Yield every permutation of the elements in some array. */ + global.Permutations = function* Permutations(items) { + if (items.length == 0) { + yield []; + } else { + items = items.slice(0); + for (let i = 0; i < items.length; i++) { + let swap = items[0]; + items[0] = items[i]; + items[i] = swap; + for (let e of Permutations(items.slice(1, items.length))) + yield [items[0]].concat(e); + } + } + }; + + /** Make an iterator with a return method. */ + global.makeIterator = function makeIterator(overrides) { + var iterator = { + next: function() { + if (overrides && overrides.next) + return overrides.next(); + return { done: false }; + }, + return: function() { + if (overrides && overrides.ret) + return overrides.ret(); + return { done: true }; + } + }; + + return function() { return iterator; }; + }; })(this); if (typeof assertThrowsInstanceOf === 'undefined') { diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index 8ae9c43b0..489d0a155 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -1171,13 +1171,19 @@ ProcessTryNotes(JSContext* cx, EnvironmentIter& ei, InterpreterRegs& regs) SettleOnTryNote(cx, tn, ei, regs); return FinallyContinuation; - case JSTRY_FOR_IN: { + case JSTRY_FOR_IN: + case JSTRY_ITERCLOSE: { /* This is similar to JSOP_ENDITER in the interpreter loop. */ DebugOnly pc = regs.fp()->script()->main() + tn->start + tn->length; - MOZ_ASSERT(JSOp(*pc) == JSOP_ENDITER); + MOZ_ASSERT_IF(tn->kind == JSTRY_FOR_IN, JSOp(*pc) == JSOP_ENDITER); Value* sp = regs.spForStackDepth(tn->stackDepth); RootedObject obj(cx, &sp[-1].toObject()); - if (!UnwindIteratorForException(cx, obj)) { + bool ok; + if (tn->kind == JSTRY_FOR_IN) + ok = UnwindIteratorForException(cx, obj); + else + ok = IteratorCloseForException(cx, obj); + if (!ok) { // We should only settle on the note only if // UnwindIteratorForException itself threw, as // onExceptionUnwind should be called anew with the new @@ -5040,7 +5046,12 @@ js::ThrowCheckIsObject(JSContext* cx, CheckIsObjectKind kind) { switch (kind) { case CheckIsObjectKind::IteratorNext: - JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_NEXT_RETURNED_PRIMITIVE); + JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, + JSMSG_ITER_METHOD_RETURNED_PRIMITIVE, "next"); + break; + case CheckIsObjectKind::IteratorReturn: + JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, + JSMSG_ITER_METHOD_RETURNED_PRIMITIVE, "return"); break; case CheckIsObjectKind::GetIterator: JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_GET_ITER_RETURNED_PRIMITIVE); diff --git a/js/src/vm/Interpreter.h b/js/src/vm/Interpreter.h index 1ffe1fdca..3a24240fc 100644 --- a/js/src/vm/Interpreter.h +++ b/js/src/vm/Interpreter.h @@ -562,6 +562,7 @@ ReportRuntimeRedeclaration(JSContext* cx, HandlePropertyName name, const char* r enum class CheckIsObjectKind : uint8_t { IteratorNext, + IteratorReturn, GetIterator }; -- cgit v1.2.3