From b311e8fa718e2a32805c25847781cc8a6a0541bd Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Sun, 25 Mar 2018 16:22:38 +0200 Subject: Bug 1342553, Bug 1343072, Bug 1344753 (details in the description) Bug 1342553 - Part 0.1: Use try-catch for IteratorClose in for-of Bug 1343072 - Update HasLiveStackValueAtDepth to follow the change in JSTRY_FOR_OF Bug 1344753 - Update for-of stack depth in ControlFlowGenerator::processWhileOrForInLoop Issue #74 --- js/src/frontend/BytecodeEmitter.cpp | 453 ++++++++++++++------- js/src/frontend/BytecodeEmitter.h | 6 +- .../auto-regress/for-of-iterator-close-debugger.js | 15 + js/src/jit/BaselineBailouts.cpp | 14 +- js/src/jit/JitFrames.cpp | 15 - js/src/jsapi.h | 9 + js/src/jsscript.h | 1 - js/src/shell/js.cpp | 2 - js/src/vm/Interpreter.cpp | 34 +- js/src/vm/Interpreter.h | 7 + js/src/vm/Opcodes.h | 11 +- 11 files changed, 378 insertions(+), 189 deletions(-) create mode 100644 js/src/jit-test/tests/auto-regress/for-of-iterator-close-debugger.js (limited to 'js/src') diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 4a60f1cf1..4d6ff6305 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -244,9 +244,9 @@ class LoopControl : public BreakableControl loopDepth_ = enclosingLoop ? enclosingLoop->loopDepth_ + 1 : 1; int loopSlots; - if (loopKind == StatementKind::Spread) + if (loopKind == StatementKind::Spread || loopKind == StatementKind::ForOfLoop) loopSlots = 3; - else if (loopKind == StatementKind::ForInLoop || loopKind == StatementKind::ForOfLoop) + else if (loopKind == StatementKind::ForInLoop) loopSlots = 2; else loopSlots = 0; @@ -278,64 +278,6 @@ 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_; @@ -2025,6 +1967,144 @@ class MOZ_STACK_CLASS IfThenElseEmitter #endif }; +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. This is done by enclosing non-iterator code with + // try-catch and call IteratorClose in `catch` block. + // 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 tell catch block not to perform + // IteratorClose. + // + // for (x of y) { + // // Operations for iterator (IteratorNext etc) are outside of + // // try-block. + // try { + // ... + // if (...) { + // // Before non-local jump, clear iterator on the stack to tell + // // catch block not to perform IteratorClose. + // tmpIterator = iterator; + // iterator = undefined; + // IteratorClose(tmpIterator, { break }); + // break; + // } + // ... + // } catch (e) { + // // Just throw again when iterator is cleared by non-local jump. + // if (iterator === undefined) + // throw e; + // IteratorClose(iterator, { throw, e }); + // } + // } + Maybe tryCatch_; + + bool allowSelfHosted_; + + public: + ForOfLoopControl(BytecodeEmitter* bce, int32_t iterDepth, bool allowSelfHosted) + : LoopControl(bce, StatementKind::ForOfLoop), + iterDepth_(iterDepth), + allowSelfHosted_(allowSelfHosted) + { + } + + bool emitBeginCodeNeedingIteratorClose(BytecodeEmitter* bce) { + tryCatch_.emplace(bce, TryEmitter::TryCatch, TryEmitter::DontUseRetVal); + + if (!tryCatch_->emitTry()) + return false; + return true; + } + + bool emitEndCodeNeedingIteratorClose(BytecodeEmitter* bce) { + if (!tryCatch_->emitCatch()) // ITER ... + return false; + + if (!bce->emit1(JSOP_EXCEPTION)) // ITER ... EXCEPTION + return false; + unsigned slotFromTop = bce->stackDepth - iterDepth_; + if (!bce->emitDupAt(slotFromTop)) // ITER ... EXCEPTION ITER + return false; + + // If ITER is undefined, it means the exception is thrown by + // IteratorClose for non-local jump, and we should't perform + // IteratorClose again here. + if (!bce->emit1(JSOP_UNDEFINED)) // ITER ... EXCEPTION ITER UNDEF + return false; + if (!bce->emit1(JSOP_STRICTNE)) // ITER ... EXCEPTION NE + return false; + + IfThenElseEmitter ifIteratorIsNotClosed(bce); + if (!ifIteratorIsNotClosed.emitIf()) // ITER ... EXCEPTION + return false; + + MOZ_ASSERT(slotFromTop == unsigned(bce->stackDepth - iterDepth_)); + if (!bce->emitDupAt(slotFromTop)) // ITER ... EXCEPTION ITER + return false; + if (!emitIteratorClose(bce, CompletionKind::Throw)) // ITER ... EXCEPTION + return false; + + if (!ifIteratorIsNotClosed.emitEnd()) // ITER ... EXCEPTION + return false; + + if (!bce->emit1(JSOP_THROW)) // ITER ... + return false; + + if (!tryCatch_->emitEnd()) + return false; + + tryCatch_.reset(); + return true; + } + + bool emitIteratorClose(BytecodeEmitter* bce, + CompletionKind completionKind = CompletionKind::Normal) { + return bce->emitIteratorClose(completionKind, allowSelfHosted_); + } + + bool emitPrepareForNonLocalJump(BytecodeEmitter* bce, bool isTarget) { + // Pop unnecessary values from the stack. Effectively this means + // leaving try-catch block. However, the performing IteratorClose can + // reach the depth for try-catch, and effectively re-enter the + // try-catch block. + if (!bce->emit1(JSOP_POP)) // ITER RESULT + return false; + if (!bce->emit1(JSOP_POP)) // ITER + return false; + + // Clear ITER slot on the stack to tell catch block to avoid performing + // IteratorClose again. + if (!bce->emit1(JSOP_UNDEFINED)) // ITER UNDEF + return false; + if (!bce->emit1(JSOP_SWAP)) // UNDEF ITER + return false; + + if (!emitIteratorClose(bce)) // UNDEF + return false; + + if (isTarget) { + // At the level of the target block, there's bytecode after the + // loop that will pop the iterator and the value, so push + // undefineds to balance the stack. + if (!bce->emit1(JSOP_UNDEFINED)) // UNDEF UNDEF + return false; + if (!bce->emit1(JSOP_UNDEFINED)) // UNDEF UNDEF UNDEF + return false; + } else { + if (!bce->emit1(JSOP_POP)) // + return false; + } + + return true; + } +}; + BytecodeEmitter::BytecodeEmitter(BytecodeEmitter* parent, Parser* parser, SharedContext* sc, HandleScript script, Handle lazyScript, @@ -2383,6 +2463,12 @@ BytecodeEmitter::emitCheckIsObj(CheckIsObjectKind kind) return emit2(JSOP_CHECKISOBJ, uint8_t(kind)); } +bool +BytecodeEmitter::emitCheckIsCallable(CheckIsCallableKind kind) +{ + return emit2(JSOP_CHECKISCALLABLE, uint8_t(kind)); +} + static inline unsigned LengthOfSetLine(unsigned line) { @@ -2624,10 +2710,8 @@ 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 + // 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; @@ -2664,31 +2748,22 @@ NonLocalExitControl::prepareForNonLocalJump(BytecodeEmitter::NestableControl* ta } else { if (!flushPops(bce_)) return false; - if (!bce_->emitJump(JSOP_GOSUB, &finallyControl.gosubs)) + if (!bce_->emitJump(JSOP_GOSUB, &finallyControl.gosubs)) // ... return false; } break; } case StatementKind::ForOfLoop: - if (!flushPops(bce_)) - return false; - - // 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 + if (!flushPops(bce_)) return false; - if (!bce_->emitIteratorClose()) // ... + + ForOfLoopControl& loopinfo = control->as(); + if (!loopinfo.emitPrepareForNonLocalJump(bce_, /* isTarget = */ false)) // ... return false; } else { - if (!bce_->emit1(JSOP_POP)) // ... ITER - return false; - if (!bce_->emit1(JSOP_POP)) // ... - return false; + npops += 3; } break; @@ -2711,18 +2786,9 @@ NonLocalExitControl::prepareForNonLocalJump(BytecodeEmitter::NestableControl* ta if (!flushPops(bce_)) return false; - 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 + if (target && emitIteratorCloseAtTarget && target->is()) { + ForOfLoopControl& loopinfo = target->as(); + if (!loopinfo.emitPrepareForNonLocalJump(bce_, /* isTarget = */ true)) // ... UNDEF UNDEF UNDEF return false; } @@ -2732,20 +2798,6 @@ NonLocalExitControl::prepareForNonLocalJump(BytecodeEmitter::NestableControl* ta 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; } @@ -5120,7 +5172,7 @@ BytecodeEmitter::emitSetOrInitializeDestructuring(ParseNode* target, Destructuri } bool -BytecodeEmitter::emitIteratorNext(ParseNode* pn, bool allowSelfHosted) +BytecodeEmitter::emitIteratorNext(ParseNode* pn, bool allowSelfHosted /* = false */) { MOZ_ASSERT(allowSelfHosted || emitterMode != BytecodeEmitter::SelfHosting, ".next() iteration is prohibited in self-hosted code because it " @@ -5141,7 +5193,8 @@ BytecodeEmitter::emitIteratorNext(ParseNode* pn, bool allowSelfHosted) } bool -BytecodeEmitter::emitIteratorClose(bool allowSelfHosted) +BytecodeEmitter::emitIteratorClose(CompletionKind completionKind /* = CompletionKind::Normal */, + bool allowSelfHosted /* = false */) { MOZ_ASSERT(allowSelfHosted || emitterMode != BytecodeEmitter::SelfHosting, ".close() on iterators is prohibited in self-hosted code because it " @@ -5174,17 +5227,86 @@ BytecodeEmitter::emitIteratorClose(bool allowSelfHosted) if (!ifReturnMethodIsDefined.emitIfElse()) return false; + if (completionKind == CompletionKind::Throw) { + // 7.4.6 IteratorClose ( iterator, completion ) + // ... + // 3. Let return be ? GetMethod(iterator, "return"). + // 4. If return is undefined, return Completion(completion). + // 5. Let innerResult be Call(return, iterator, « »). + // 6. If completion.[[Type]] is throw, return Completion(completion). + // 7. If innerResult.[[Type]] is throw, return + // Completion(innerResult). + // + // For CompletionKind::Normal case, JSOP_CALL for step 5 checks if RET + // is callable, and throws if not. Since step 6 doesn't match and + // error handling in step 3 and step 7 can be merged. + // + // For CompletionKind::Throw case, an error thrown by JSOP_CALL for + // step 5 is ignored by try-catch. So we should check if RET is + // callable here, outside of try-catch, and the throw immediately if + // not. + CheckIsCallableKind kind = CheckIsCallableKind::IteratorReturn; + if (!emitCheckIsCallable(kind)) // ... ITER RET + 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; - if (!emitCall(JSOP_CALL, 0)) // ... RESULT + + Maybe tryCatch; + + if (completionKind == CompletionKind::Throw) { + tryCatch.emplace(this, TryEmitter::TryCatch, TryEmitter::DontUseRetVal, + TryEmitter::DontUseControl); + + // Mutate stack to balance stack for try-catch. + if (!emit1(JSOP_UNDEFINED)) // ... RET ITER UNDEF + return false; + if (!tryCatch->emitTry()) // ... RET ITER UNDEF + return false; + if (!emitDupAt(2)) // ... RET ITER UNDEF RET + return false; + if (!emitDupAt(2)) // ... RET ITER UNDEF RET ITER + return false; + } + + if (!emitCall(JSOP_CALL, 0)) // ... ... RESULT return false; checkTypeSet(JSOP_CALL); - if (!emitCheckIsObj(CheckIsObjectKind::IteratorReturn)) // ... RESULT - return false; + + if (completionKind == CompletionKind::Throw) { + if (!emit1(JSOP_SWAP)) // ... RET ITER RESULT UNDEF + return false; + if (!emit1(JSOP_POP)) // ... RET ITER RESULT + return false; + + if (!tryCatch->emitCatch()) // ... RET ITER RESULT + return false; + + // Just ignore the exception thrown by call. + if (!emit1(JSOP_EXCEPTION)) // ... RET ITER RESULT EXC + return false; + if (!emit1(JSOP_POP)) // ... RET ITER RESULT + return false; + + if (!tryCatch->emitEnd()) // ... RET ITER RESULT + return false; + + // Restore stack. + if (!emit2(JSOP_UNPICK, 2)) // ... RESULT RET ITER + return false; + if (!emit1(JSOP_POP)) // ... RESULT RET + return false; + if (!emit1(JSOP_POP)) // ... RESULT + return false; + } else { + if (!emitCheckIsObj(CheckIsObjectKind::IteratorReturn)) // ... RESULT + return false; + } if (!ifReturnMethodIsDefined.emitElse()) return false; @@ -6749,8 +6871,19 @@ BytecodeEmitter::emitForOf(ParseNode* forOfLoop, EmitterScope* headLexicalEmitte MOZ_ASSERT(forOfHead->isKind(PNK_FOROF)); MOZ_ASSERT(forOfHead->isArity(PN_TERNARY)); - // Evaluate the expression being iterated. ParseNode* forHeadExpr = forOfHead->pn_kid3; + + // Certain builtins (e.g. Array.from) are implemented in self-hosting + // as for-of loops. + bool allowSelfHostedIter = false; + if (emitterMode == BytecodeEmitter::SelfHosting && + forHeadExpr->isKind(PNK_CALL) && + forHeadExpr->pn_head->name() == cx->names().allowContentIter) + { + allowSelfHostedIter = true; + } + + // Evaluate the expression being iterated. if (!emitTree(forHeadExpr)) // ITERABLE return false; if (!emitIterator()) // ITER @@ -6758,12 +6891,14 @@ BytecodeEmitter::emitForOf(ParseNode* forOfLoop, EmitterScope* headLexicalEmitte int32_t iterDepth = stackDepth; - // For-of loops have both the iterator and the value on the stack. Push - // undefined to balance the stack. + // For-of loops have both the iterator, the result, and the result.value + // on the stack. Push undefineds to balance the stack. if (!emit1(JSOP_UNDEFINED)) // ITER RESULT return false; + if (!emit1(JSOP_UNDEFINED)) // ITER RESULT UNDEF + return false; - ForOfLoopControl loopInfo(this, iterDepth); + ForOfLoopControl loopInfo(this, iterDepth, allowSelfHostedIter); // Annotate so IonMonkey can find the loop-closing jump. unsigned noteIndex; @@ -6771,11 +6906,11 @@ BytecodeEmitter::emitForOf(ParseNode* forOfLoop, EmitterScope* headLexicalEmitte return false; JumpList initialJump; - if (!emitJump(JSOP_GOTO, &initialJump)) // ITER RESULT + if (!emitJump(JSOP_GOTO, &initialJump)) // ITER RESULT UNDEF return false; JumpTarget top{ -1 }; - if (!emitLoopHead(nullptr, &top)) // ITER RESULT + if (!emitLoopHead(nullptr, &top)) // ITER RESULT UNDEF return false; // If the loop had an escaping lexical declaration, replace the current @@ -6792,7 +6927,7 @@ BytecodeEmitter::emitForOf(ParseNode* forOfLoop, EmitterScope* headLexicalEmitte MOZ_ASSERT(headLexicalEmitterScope->scope(this)->kind() == ScopeKind::Lexical); if (headLexicalEmitterScope->hasEnvironment()) { - if (!emit1(JSOP_RECREATELEXICALENV)) // ITER RESULT + if (!emit1(JSOP_RECREATELEXICALENV)) // ITER RESULT UNDEF return false; } @@ -6812,61 +6947,66 @@ BytecodeEmitter::emitForOf(ParseNode* forOfLoop, EmitterScope* headLexicalEmitte // // 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_POP)) // ITER RESULT + return false; 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 + if (!loopInfo.emitBeginCodeNeedingIteratorClose(this)) return false; - if (!emit1(JSOP_POP)) // ITER RESULT + if (!emitInitializeForInOrOfTarget(forOfHead)) // ITER RESULT VALUE return false; MOZ_ASSERT(stackDepth == loopDepth, "the stack must be balanced around the initializing " "operation"); + // Remove VALUE from the stack to release it. + if (!emit1(JSOP_POP)) // ITER RESULT + return false; + if (!emit1(JSOP_UNDEFINED)) // ITER RESULT UNDEF + return false; + // Perform the loop body. ParseNode* forBody = forOfLoop->pn_right; - if (!emitTree(forBody)) // ITER RESULT + if (!emitTree(forBody)) // ITER RESULT UNDEF return false; - if (!loopInfo.finishIterCloseTryNote(this)) + MOZ_ASSERT(stackDepth == loopDepth, + "the stack must be balanced around the for-of body"); + + if (!loopInfo.emitEndCodeNeedingIteratorClose(this)) return false; // Set offset for continues. loopInfo.continueTarget = { offset() }; - if (!emitLoopEntry(forHeadExpr, initialJump)) // ITER RESULT + if (!emitLoopEntry(forHeadExpr, initialJump)) // ITER RESULT UNDEF return false; - if (!emit1(JSOP_POP)) // ITER + if (!emit1(JSOP_SWAP)) // ITER UNDEF RESULT + return false; + if (!emit1(JSOP_POP)) // ITER UNDEF return false; - if (!emit1(JSOP_DUP)) // ITER ITER + if (!emitDupAt(1)) // ITER UNDEF ITER return false; - // Certain builtins (e.g. Array.from) are implemented in self-hosting - // as for-of loops. - bool allowSelfHostedIter = false; - if (emitterMode == BytecodeEmitter::SelfHosting && - forHeadExpr->isKind(PNK_CALL) && - forHeadExpr->pn_head->name() == cx->names().allowContentIter) - { - allowSelfHostedIter = true; - } + if (!emitIteratorNext(forOfHead, allowSelfHostedIter)) // ITER UNDEF RESULT + return false; - if (!emitIteratorNext(forOfHead, allowSelfHostedIter)) // ITER RESULT + if (!emit1(JSOP_SWAP)) // ITER RESULT UNDEF return false; - if (!emit1(JSOP_DUP)) // ITER RESULT RESULT + + if (!emitDupAt(1)) // ITER RESULT UNDEF RESULT return false; - if (!emitAtomOp(cx->names().done, JSOP_GETPROP)) // ITER RESULT DONE + if (!emitAtomOp(cx->names().done, JSOP_GETPROP)) // ITER RESULT UNDEF DONE return false; if (!emitBackwardJump(JSOP_IFEQ, top, &beq, &breakTarget)) - return false; // ITER RESULT + return false; // ITER RESULT UNDEF MOZ_ASSERT(this->stackDepth == loopDepth); } @@ -6881,7 +7021,7 @@ BytecodeEmitter::emitForOf(ParseNode* forOfLoop, EmitterScope* headLexicalEmitte if (!tryNoteList.append(JSTRY_FOR_OF, stackDepth, top.offset, breakTarget.offset)) return false; - return emitUint16Operand(JSOP_POPN, 2); // + return emitUint16Operand(JSOP_POPN, 3); // } bool @@ -7291,6 +7431,8 @@ BytecodeEmitter::emitComprehensionForOf(ParseNode* pn) // Push a dummy result so that we properly enter iteration midstream. if (!emit1(JSOP_UNDEFINED)) // ITER RESULT return false; + if (!emit1(JSOP_UNDEFINED)) // ITER RESULT VALUE + return false; // Enter the block before the loop body, after evaluating the obj. // Initialize let bindings with undefined when entering, as the name @@ -7328,42 +7470,59 @@ BytecodeEmitter::emitComprehensionForOf(ParseNode* pn) #endif // Emit code to assign result.value to the iteration variable. + if (!emit1(JSOP_POP)) // ITER RESULT + return false; if (!emit1(JSOP_DUP)) // ITER RESULT RESULT return false; if (!emitAtomOp(cx->names().value, JSOP_GETPROP)) // ITER RESULT VALUE return false; + + // Notice: Comprehension for-of doesn't perform IteratorClose, since it's + // not in the spec. + if (!emitAssignment(loopVariableName, JSOP_NOP, nullptr)) // ITER RESULT VALUE return false; + + // Remove VALUE from the stack to release it. if (!emit1(JSOP_POP)) // ITER RESULT return false; + if (!emit1(JSOP_UNDEFINED)) // ITER RESULT UNDEF + return false; // The stack should be balanced around the assignment opcode sequence. MOZ_ASSERT(this->stackDepth == loopDepth); // Emit code for the loop body. - if (!emitTree(forBody)) + if (!emitTree(forBody)) // ITER RESULT UNDEF return false; + // The stack should be balanced around the assignment opcode sequence. + MOZ_ASSERT(this->stackDepth == loopDepth); + // Set offset for continues. loopInfo.continueTarget = { offset() }; if (!emitLoopEntry(forHeadExpr, jmp)) return false; - if (!emit1(JSOP_POP)) // ITER + if (!emit1(JSOP_SWAP)) // ITER UNDEF RESULT return false; - if (!emit1(JSOP_DUP)) // ITER ITER + if (!emit1(JSOP_POP)) // ITER UNDEF return false; - if (!emitIteratorNext(forHead)) // ITER RESULT + if (!emitDupAt(1)) // ITER UNDEF ITER return false; - if (!emit1(JSOP_DUP)) // ITER RESULT RESULT + if (!emitIteratorNext(forHead)) // ITER UNDEF RESULT + return false; + if (!emit1(JSOP_SWAP)) // ITER RESULT UNDEF + return false; + if (!emitDupAt(1)) // ITER RESULT UNDEF RESULT return false; - if (!emitAtomOp(cx->names().done, JSOP_GETPROP)) // ITER RESULT DONE + if (!emitAtomOp(cx->names().done, JSOP_GETPROP)) // ITER RESULT UNDEF DONE return false; JumpList beq; JumpTarget breakTarget{ -1 }; - if (!emitBackwardJump(JSOP_IFEQ, top, &beq, &breakTarget)) // ITER RESULT + if (!emitBackwardJump(JSOP_IFEQ, top, &beq, &breakTarget)) // ITER RESULT UNDEF return false; MOZ_ASSERT(this->stackDepth == loopDepth); @@ -7385,7 +7544,7 @@ BytecodeEmitter::emitComprehensionForOf(ParseNode* pn) } // Pop the result and the iter. - return emitUint16Operand(JSOP_POPN, 2); // + return emitUint16Operand(JSOP_POPN, 3); // } bool diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 2ab6c6fef..7ff40b462 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -473,6 +473,9 @@ struct MOZ_STACK_CLASS BytecodeEmitter // Helper to emit JSOP_CHECKISOBJ. MOZ_MUST_USE bool emitCheckIsObj(CheckIsObjectKind kind); + // Helper to emit JSOP_CHECKISCALLABLE. + MOZ_MUST_USE bool emitCheckIsCallable(CheckIsCallableKind kind); + // Emit a bytecode followed by an uint16 immediate operand stored in // big-endian order. MOZ_MUST_USE bool emitUint16Operand(JSOp op, uint32_t operand); @@ -679,7 +682,8 @@ 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(bool allowSelfHosted = false); + MOZ_MUST_USE bool emitIteratorClose(CompletionKind completionKind = CompletionKind::Normal, + bool allowSelfHosted = false); template MOZ_MUST_USE bool wrapWithDestructuringIteratorCloseTryNote(int32_t iterDepth, diff --git a/js/src/jit-test/tests/auto-regress/for-of-iterator-close-debugger.js b/js/src/jit-test/tests/auto-regress/for-of-iterator-close-debugger.js new file mode 100644 index 000000000..a4d0bf654 --- /dev/null +++ b/js/src/jit-test/tests/auto-regress/for-of-iterator-close-debugger.js @@ -0,0 +1,15 @@ +// |jit-test| error:ReferenceError + +// for-of should close iterator even if the exception is once caught by the +// debugger. + +var g = newGlobal(); +g.parent = this; +g.eval("new Debugger(parent).onExceptionUnwind = function () { };"); +// jsfunfuzz-generated +for (var x of []) {}; +for (var l of [0]) { + for (var y = 0; y < 1; y++) { + g2; + } +} diff --git a/js/src/jit/BaselineBailouts.cpp b/js/src/jit/BaselineBailouts.cpp index 161e1aa4c..3ab722b3d 100644 --- a/js/src/jit/BaselineBailouts.cpp +++ b/js/src/jit/BaselineBailouts.cpp @@ -509,16 +509,10 @@ HasLiveStackValueAtDepth(JSScript* script, jsbytecode* pc, uint32_t stackDepth) break; case JSTRY_FOR_OF: - // For-of loops have both the iterator and the result object on - // stack. The iterator is below the result object. - if (stackDepth == tn->stackDepth - 1) - return true; - break; - - case JSTRY_ITERCLOSE: - // Code that need to call IteratorClose have the iterator on the - // stack. - if (stackDepth == tn->stackDepth) + // For-of loops have the iterator, the result object, and the value + // of the result object on stack. The iterator is below the result + // object and the value. + if (stackDepth == tn->stackDepth - 2) return true; break; diff --git a/js/src/jit/JitFrames.cpp b/js/src/jit/JitFrames.cpp index 7c3f0d120..a70356ad4 100644 --- a/js/src/jit/JitFrames.cpp +++ b/js/src/jit/JitFrames.cpp @@ -331,7 +331,6 @@ static void CloseLiveIteratorIon(JSContext* cx, const InlineFrameIterator& frame, JSTryNote* tn) { MOZ_ASSERT(tn->kind == JSTRY_FOR_IN || - tn->kind == JSTRY_ITERCLOSE || tn->kind == JSTRY_DESTRUCTURING_ITERCLOSE); bool isDestructuring = tn->kind == JSTRY_DESTRUCTURING_ITERCLOSE; @@ -442,7 +441,6 @@ HandleExceptionIon(JSContext* cx, const InlineFrameIterator& frame, ResumeFromEx switch (tn->kind) { case JSTRY_FOR_IN: - case JSTRY_ITERCLOSE: case JSTRY_DESTRUCTURING_ITERCLOSE: MOZ_ASSERT_IF(tn->kind == JSTRY_FOR_IN, JSOp(*(script->main() + tn->start + tn->length)) == JSOP_ENDITER); @@ -637,19 +635,6 @@ ProcessTryNotesBaseline(JSContext* cx, const JitFrameIterator& frame, Environmen break; } - case JSTRY_ITERCLOSE: { - uint8_t* framePointer; - uint8_t* stackPointer; - BaselineFrameAndStackPointersFromTryNote(tn, frame, &framePointer, &stackPointer); - Value iterValue(*reinterpret_cast(stackPointer)); - RootedObject iterObject(cx, &iterValue.toObject()); - if (!IteratorCloseForException(cx, iterObject)) { - SettleOnTryNote(cx, tn, frame, ei, rfe, pc); - return false; - } - break; - } - case JSTRY_DESTRUCTURING_ITERCLOSE: { uint8_t* framePointer; uint8_t* stackPointer; diff --git a/js/src/jsapi.h b/js/src/jsapi.h index cbef0f8fb..5523dfa1d 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -6622,5 +6622,14 @@ SetGetPerformanceGroupsCallback(JSContext*, GetGroupsCallback, void*); } /* namespace js */ +namespace js { + +enum class CompletionKind { + Normal, + Return, + Throw +}; + +} /* namespace js */ #endif /* jsapi_h */ diff --git a/js/src/jsscript.h b/js/src/jsscript.h index 8bba3ec39..9cb7c538f 100644 --- a/js/src/jsscript.h +++ b/js/src/jsscript.h @@ -85,7 +85,6 @@ enum JSTryNoteKind { JSTRY_FOR_IN, JSTRY_FOR_OF, JSTRY_LOOP, - JSTRY_ITERCLOSE, JSTRY_DESTRUCTURING_ITERCLOSE }; diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 6fe01de22..5d6508d59 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -2599,8 +2599,6 @@ TryNoteName(JSTryNoteKind kind) return "for-of"; case JSTRY_LOOP: return "loop"; - case JSTRY_ITERCLOSE: - return "iterclose"; case JSTRY_DESTRUCTURING_ITERCLOSE: return "dstr-iterclose"; } diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index 7f8ff8445..eb3000e07 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -1189,17 +1189,6 @@ ProcessTryNotes(JSContext* cx, EnvironmentIter& ei, InterpreterRegs& regs) break; } - case JSTRY_ITERCLOSE: { - // The iterator object is at the top of the stack. - Value* sp = regs.spForStackDepth(tn->stackDepth); - RootedObject iterObject(cx, &sp[-1].toObject()); - if (!IteratorCloseForException(cx, iterObject)) { - SettleOnTryNote(cx, tn, ei, regs); - return ErrorReturnContinuation; - } - break; - } - case JSTRY_DESTRUCTURING_ITERCLOSE: { // Whether the destructuring iterator is done is at the top of the // stack. The iterator object is second from the top. @@ -1892,7 +1881,6 @@ CASE(JSOP_UNUSED192) CASE(JSOP_UNUSED209) CASE(JSOP_UNUSED210) CASE(JSOP_UNUSED211) -CASE(JSOP_UNUSED219) CASE(JSOP_UNUSED220) CASE(JSOP_UNUSED221) CASE(JSOP_UNUSED222) @@ -2637,6 +2625,15 @@ CASE(JSOP_CHECKISOBJ) } END_CASE(JSOP_CHECKISOBJ) +CASE(JSOP_CHECKISCALLABLE) +{ + if (!IsCallable(REGS.sp[-1])) { + MOZ_ALWAYS_FALSE(ThrowCheckIsCallable(cx, CheckIsCallableKind(GET_UINT8(REGS.pc)))); + goto error; + } +} +END_CASE(JSOP_CHECKISCALLABLE) + CASE(JSOP_CHECKTHIS) { if (REGS.sp[-1].isMagic(JS_UNINITIALIZED_LEXICAL)) { @@ -5094,6 +5091,19 @@ js::ThrowCheckIsObject(JSContext* cx, CheckIsObjectKind kind) return false; } +bool +js::ThrowCheckIsCallable(JSContext* cx, CheckIsCallableKind kind) +{ + switch (kind) { + case CheckIsCallableKind::IteratorReturn: + JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_RETURN_NOT_CALLABLE); + break; + default: + MOZ_CRASH("Unknown kind"); + } + return false; +} + bool js::ThrowUninitializedThis(JSContext* cx, AbstractFramePtr frame) { diff --git a/js/src/vm/Interpreter.h b/js/src/vm/Interpreter.h index 38e23ec06..330dbef5f 100644 --- a/js/src/vm/Interpreter.h +++ b/js/src/vm/Interpreter.h @@ -570,6 +570,13 @@ enum class CheckIsObjectKind : uint8_t { bool ThrowCheckIsObject(JSContext* cx, CheckIsObjectKind kind); +enum class CheckIsCallableKind : uint8_t { + IteratorReturn +}; + +bool +ThrowCheckIsCallable(JSContext* cx, CheckIsCallableKind kind); + bool ThrowUninitializedThis(JSContext* cx, AbstractFramePtr frame); diff --git a/js/src/vm/Opcodes.h b/js/src/vm/Opcodes.h index 84f08c4d5..3848445ff 100644 --- a/js/src/vm/Opcodes.h +++ b/js/src/vm/Opcodes.h @@ -2201,7 +2201,16 @@ */ \ macro(JSOP_HOLE, 218, "hole", NULL, 1, 0, 1, JOF_BYTE) \ \ - macro(JSOP_UNUSED219, 219,"unused219", NULL, 1, 0, 0, JOF_BYTE) \ + /* + * Checks that the top value on the stack is callable, and throws a + * TypeError if not. The operand 'kind' is used only to generate an + * appropriate error message. + * Category: Statements + * Type: Function + * Operands: uint8_t kind + * Stack: result => result, callable + */ \ + macro(JSOP_CHECKISCALLABLE, 219, "checkiscallable", NULL, 2, 1, 1, JOF_UINT8) \ macro(JSOP_UNUSED220, 220,"unused220", NULL, 1, 0, 0, JOF_BYTE) \ macro(JSOP_UNUSED221, 221,"unused221", NULL, 1, 0, 0, JOF_BYTE) \ macro(JSOP_UNUSED222, 222,"unused222", NULL, 1, 0, 0, JOF_BYTE) \ -- cgit v1.2.3