mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 1480390: Move ForOfIterClose logic inside TryNoteIter r=tcampbell 

This patch was intended to be a pure refactoring of existing code with
no side-effects, moving the logic for handling for-of/for-of-iterclose
trynotes inside TryNoteIter to avoid duplicating logic in all users of
TryNoteIter. However, it turns out that there was a subtle preexisting
bug in TryNoteIter that is fixed by the refactoring. Specifically, the
logic to skip from a for-of-iterclose to its enclosing for-of must run
before the logic to skip trynotes based on stack depth. Otherwise, the
stack depth code may filter out the enclosing for-of (see the attached
test case for an example) and we will skip too many try-notes.

Differential Revision: https://phabricator.services.mozilla.com/D14783

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Iain Ireland 2019-01-11 18:05:36 +00:00
Родитель 854a0f4352
Коммит fce1e90420
4 изменённых файлов: 97 добавлений и 134 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

42
js/src/jit-test/tests/for-of/throw-during-break.js Normal file
Просмотреть файл

@ -0,0 +1,42 @@
var whoCaught = "nobody"
function* wrapNoThrow() {
let iter = {
[Symbol.iterator]() {
return this;
},
next() {
return { value: 10, done: false };
},
return() { throw "nonsense"; }
};
for (const i of iter)
yield i;
}
function foo() {
try {
l2:
for (j of wrapNoThrow()) {
try {
for (i of [1,2,3]) {
try {
break l2;
} catch(e) {
whoCaught = "inner"
}
}
} catch (e) {
whoCaught = "inner2"
}
}
} catch (e) {
whoCaught = "correct"
}
}
try {
foo();
} catch (e) { whoCaught = "outer" }
assertEq(whoCaught, "correct");

72
js/src/jit/JitFrames.cpp
Просмотреть файл

@ -200,43 +200,19 @@ static void HandleExceptionIon(JSContext* cx, const InlineFrameIterator& frame,
return;
}
bool inForOfIterClose = false;
for (TryNoteIterIon tni(cx, frame); !tni.done(); ++tni) {
const JSTryNote* tn = *tni;
switch (tn->kind) {
case JSTRY_FOR_IN:
case JSTRY_DESTRUCTURING:
// See corresponding comment in ProcessTryNotes.
if (inForOfIterClose) {
break;
}
MOZ_ASSERT_IF(tn->kind == JSTRY_FOR_IN,
JSOp(*(script->offsetToPC(tn->start + tn->length))) ==
JSOP_ENDITER);
CloseLiveIteratorIon(cx, frame, tn);
break;
case JSTRY_FOR_OF_ITERCLOSE:
inForOfIterClose = true;
break;
case JSTRY_FOR_OF:
inForOfIterClose = false;
break;
case JSTRY_LOOP:
break;
case JSTRY_CATCH:
if (cx->isExceptionPending()) {
// See corresponding comment in ProcessTryNotes.
if (inForOfIterClose) {
break;
}
// Ion can compile try-catch, but bailing out to catch
// exceptions is slow. Reset the warm-up counter so that if we
// catch many exceptions we won't Ion-compile the script.
@ -265,6 +241,11 @@ static void HandleExceptionIon(JSContext* cx, const InlineFrameIterator& frame,
}
break;
case JSTRY_FOR_OF:
case JSTRY_LOOP:
break;
// JSTRY_FOR_OF_ITERCLOSE is handled internally by the try note iterator.
default:
MOZ_CRASH("Unexpected try note");
}
@ -349,17 +330,11 @@ class TryNoteIterBaseline : public TryNoteIter<BaselineFrameStackDepthOp> {
// pc parameter is updated to where the envs have been unwound to.
static void CloseLiveIteratorsBaselineForUncatchableException(
JSContext* cx, const JSJitFrameIter& frame, jsbytecode* pc) {
bool inForOfIterClose = false;
for (TryNoteIterBaseline tni(cx, frame.baselineFrame(), pc); !tni.done();
++tni) {
const JSTryNote* tn = *tni;
switch (tn->kind) {
case JSTRY_FOR_IN: {
// See corresponding comment in ProcessTryNotes.
if (inForOfIterClose) {
break;
}
uint8_t* framePointer;
uint8_t* stackPointer;
BaselineFrameAndStackPointersFromTryNote(tn, frame, &framePointer,
@ -370,14 +345,6 @@ static void CloseLiveIteratorsBaselineForUncatchableException(
break;
}
case JSTRY_FOR_OF_ITERCLOSE:
inForOfIterClose = true;
break;
case JSTRY_FOR_OF:
inForOfIterClose = false;
break;
default:
break;
}
@ -388,7 +355,6 @@ static bool ProcessTryNotesBaseline(JSContext* cx, const JSJitFrameIter& frame,
EnvironmentIter& ei,
ResumeFromException* rfe, jsbytecode** pc) {
RootedScript script(cx, frame.baselineFrame()->script());
bool inForOfIterClose = false;
for (TryNoteIterBaseline tni(cx, frame.baselineFrame(), *pc); !tni.done();
++tni) {
@ -403,11 +369,6 @@ static bool ProcessTryNotesBaseline(JSContext* cx, const JSJitFrameIter& frame,
break;
}
// See corresponding comment in ProcessTryNotes.
if (inForOfIterClose) {
break;
}
SettleOnTryNote(cx, tn, frame, ei, rfe, pc);
// Ion can compile try-catch, but bailing out to catch
@ -425,11 +386,6 @@ static bool ProcessTryNotesBaseline(JSContext* cx, const JSJitFrameIter& frame,
}
case JSTRY_FINALLY: {
// See corresponding comment in ProcessTryNotes.
if (inForOfIterClose) {
break;
}
PCMappingSlotInfo slotInfo;
SettleOnTryNote(cx, tn, frame, ei, rfe, pc);
rfe->kind = ResumeFromException::RESUME_FINALLY;
@ -446,11 +402,6 @@ static bool ProcessTryNotesBaseline(JSContext* cx, const JSJitFrameIter& frame,
}
case JSTRY_FOR_IN: {
// See corresponding comment in ProcessTryNotes.
if (inForOfIterClose) {
break;
}
uint8_t* framePointer;
uint8_t* stackPointer;
BaselineFrameAndStackPointersFromTryNote(tn, frame, &framePointer,
@ -462,11 +413,6 @@ static bool ProcessTryNotesBaseline(JSContext* cx, const JSJitFrameIter& frame,
}
case JSTRY_DESTRUCTURING: {
// See corresponding comment in ProcessTryNotes.
if (inForOfIterClose) {
break;
}
uint8_t* framePointer;
uint8_t* stackPointer;
BaselineFrameAndStackPointersFromTryNote(tn, frame, &framePointer,
@ -484,17 +430,11 @@ static bool ProcessTryNotesBaseline(JSContext* cx, const JSJitFrameIter& frame,
break;
}
case JSTRY_FOR_OF_ITERCLOSE:
inForOfIterClose = true;
break;
case JSTRY_FOR_OF:
inForOfIterClose = false;
break;
case JSTRY_LOOP:
break;
// JSTRY_FOR_OF_ITERCLOSE is handled internally by the try note iterator.
default:
MOZ_CRASH("Invalid try note");
}

65
js/src/vm/Interpreter.cpp
Просмотреть файл

@ -1120,29 +1120,14 @@ static void UnwindIteratorsForUncatchableException(
JSContext* cx, const InterpreterRegs& regs) {
// c.f. the regular (catchable) TryNoteIterInterpreter loop in
// ProcessTryNotes.
bool inForOfIterClose = false;
for (TryNoteIterInterpreter tni(cx, regs); !tni.done(); ++tni) {
const JSTryNote* tn = *tni;
switch (tn->kind) {
case JSTRY_FOR_IN: {
// See corresponding comment in ProcessTryNotes.
if (inForOfIterClose) {
break;
}
Value* sp = regs.spForStackDepth(tn->stackDepth);
UnwindIteratorForUncatchableException(&sp[-1].toObject());
break;
}
case JSTRY_FOR_OF_ITERCLOSE:
inForOfIterClose = true;
break;
case JSTRY_FOR_OF:
inForOfIterClose = false;
break;
default:
break;
}
@ -1159,7 +1144,6 @@ enum HandleErrorContinuation {
static HandleErrorContinuation ProcessTryNotes(JSContext* cx,
EnvironmentIter& ei,
InterpreterRegs& regs) {
bool inForOfIterClose = false;
for (TryNoteIterInterpreter tni(cx, regs); !tni.done(); ++tni) {
const JSTryNote* tn = *tni;
@ -1170,50 +1154,14 @@ static HandleErrorContinuation ProcessTryNotes(JSContext* cx,
break;
}
// If IteratorClose due to abnormal completion threw inside a
// for-of loop, it is not catchable by try statements inside of
// the for-of loop.
//
// This is handled by this weirdness in the exception handler
// instead of in bytecode because it is hard to do so in bytecode:
//
// 1. IteratorClose emitted due to abnormal completion (break,
// throw, return) are emitted inline, at the source location of
// the break, throw, or return statement. For example:
//
// for (x of iter) {
// try { return; } catch (e) { }
// }
//
// From the try-note nesting's perspective, the IteratorClose
// resulting from |return| is covered by the inner try, when it
// should not be.
//
// 2. Try-catch notes cannot be disjoint. That is, we can't have
// multiple notes with disjoint pc ranges jumping to the same
// catch block.
if (inForOfIterClose) {
break;
}
SettleOnTryNote(cx, tn, ei, regs);
return CatchContinuation;
case JSTRY_FINALLY:
// See note above.
if (inForOfIterClose) {
break;
}
SettleOnTryNote(cx, tn, ei, regs);
return FinallyContinuation;
case JSTRY_FOR_IN: {
// Don't let (extra) values pushed on the stack while closing a
// for-of iterator confuse us into thinking we still have to close
// an inner for-in iterator.
if (inForOfIterClose) {
break;
}
/* This is similar to JSOP_ENDITER in the interpreter loop. */
DebugOnly<jsbytecode*> pc =
regs.fp()->script()->offsetToPC(tn->start + tn->length);
@ -1225,11 +1173,6 @@ static HandleErrorContinuation ProcessTryNotes(JSContext* cx,
}
case JSTRY_DESTRUCTURING: {
// See note above.
if (inForOfIterClose) {
break;
}
// Whether the destructuring iterator is done is at the top of the
// stack. The iterator object is second from the top.
MOZ_ASSERT(tn->stackDepth > 1);
@ -1246,17 +1189,11 @@ static HandleErrorContinuation ProcessTryNotes(JSContext* cx,
break;
}
case JSTRY_FOR_OF_ITERCLOSE:
inForOfIterClose = true;
break;
case JSTRY_FOR_OF:
inForOfIterClose = false;
break;
case JSTRY_LOOP:
break;
// JSTRY_FOR_OF_ITERCLOSE is handled internally by the try note iterator.
default:
MOZ_CRASH("Invalid try note");
}

52
js/src/vm/Interpreter.h
Просмотреть файл

@ -320,11 +320,46 @@ class MOZ_STACK_CLASS TryNoteIter {
void settle() {
for (; tn_ != tnEnd_; ++tn_) {
/* If pc is out of range, try the next one. */
if (pcOffset_ - tn_->start >= tn_->length) {
if (!pcInRange()) {
continue;
}
/* Try notes cannot be disjoint. That is, we can't have
* multiple notes with disjoint pc ranges jumping to the same
* catch block. This interacts awkwardly with for-of loops, in
* which calls to IteratorClose emitted due to abnormal
* completion (break, throw, return) are emitted inline, at the
* source location of the break, throw, or return
* statement. For example:
*
* for (x of iter) {
* try { return; } catch (e) { }
* }
*
* From the try-note nesting's perspective, the IteratorClose
* resulting from |return| is covered by the inner try, when it
* should not be. If IteratorClose throws, we don't want to
* catch it here.
*
* To make this work, we use JSTRY_FOR_OF_ITERCLOSE try-notes,
* which cover the range of the abnormal completion. When
* looking up trynotes, a for-of iterclose note indicates that
* the enclosing for-of has just been terminated. As a result,
* trynotes within that for-of are no longer active. When we
* see a for-of-iterclose, we skip ahead in the trynotes list
* until we see the matching for-of.
*/
if (tn_->kind == JSTRY_FOR_OF_ITERCLOSE) {
do {
++tn_;
MOZ_ASSERT(tn_ != tnEnd_);
MOZ_ASSERT_IF(pcInRange(), tn_->kind != JSTRY_FOR_OF_ITERCLOSE);
} while (!(pcInRange() && tn_->kind == JSTRY_FOR_OF));
// Advance to trynote following the enclosing for-of.
++tn_;
}
/*
* We have a note that covers the exception pc but we must check
* whether the interpreter has already executed the corresponding
@ -344,8 +379,8 @@ class MOZ_STACK_CLASS TryNoteIter {
* depth exceeding the current one and this condition is what we use to
* filter them out.
*/
if (tn_->stackDepth <= getStackDepth_()) {
break;
if (tn_ == tnEnd_ || tn_->stackDepth <= getStackDepth_()) {
return;
}
}
}
@ -373,6 +408,15 @@ class MOZ_STACK_CLASS TryNoteIter {
settle();
}
bool pcInRange() const {
// This checks both ends of the range at once
// because unsigned integers wrap on underflow.
uint32_t offset = pcOffset_;
uint32_t start = tn_->start;
uint32_t length = tn_->length;
return offset - start < length;
}
bool done() const { return tn_ == tnEnd_; }
const JSTryNote* operator*() const { return tn_; }
};