summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorjanekptacijarabaci <janekptacijarabaci@seznam.cz>2018-03-25 18:21:15 +0200
committerjanekptacijarabaci <janekptacijarabaci@seznam.cz>2018-03-25 18:21:15 +0200
commit05441d12b6bbc9dde268914fcfd374db61b83462 (patch)
tree87443cdcba99085f3a878e9e3a796ef8c3215bbc
parent727c27a30d10a811d5a3fe04e2407cd7b3993b5e (diff)
downloadUXP-05441d12b6bbc9dde268914fcfd374db61b83462.tar
UXP-05441d12b6bbc9dde268914fcfd374db61b83462.tar.gz
UXP-05441d12b6bbc9dde268914fcfd374db61b83462.tar.lz
UXP-05441d12b6bbc9dde268914fcfd374db61b83462.tar.xz
UXP-05441d12b6bbc9dde268914fcfd374db61b83462.zip
Bug 1346862 - Fix IteratorClose due to non-local jumps being catchable by try statements inside for-of
Issue #74
-rw-r--r--js/src/frontend/BytecodeEmitter.cpp20
-rw-r--r--js/src/frontend/BytecodeEmitter.h2
-rw-r--r--js/src/jit/JitFrames.cpp31
-rw-r--r--js/src/jsscript.h1
-rw-r--r--js/src/shell/js.cpp6
-rw-r--r--js/src/tests/ecma_6/Statements/for-of-iterator-close-throw.js35
-rw-r--r--js/src/vm/Interpreter.cpp36
7 files changed, 113 insertions, 18 deletions
diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp
index 4d6ff6305..b2e48d7ea 100644
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -2065,7 +2065,11 @@ class ForOfLoopControl : public LoopControl
bool emitIteratorClose(BytecodeEmitter* bce,
CompletionKind completionKind = CompletionKind::Normal) {
- return bce->emitIteratorClose(completionKind, allowSelfHosted_);
+ ptrdiff_t start = bce->offset();
+ if (!bce->emitIteratorClose(completionKind, allowSelfHosted_))
+ return false;
+ ptrdiff_t end = bce->offset();
+ return bce->tryNoteList.append(JSTRY_FOR_OF_ITERCLOSE, 0, start, end);
}
bool emitPrepareForNonLocalJump(BytecodeEmitter* bce, bool isTarget) {
@@ -2614,17 +2618,6 @@ BytecodeEmitter::emitUint32Operand(JSOp op, uint32_t operand)
return true;
}
-bool
-BytecodeEmitter::flushPops(int* npops)
-{
- MOZ_ASSERT(*npops != 0);
- if (!emitUint16Operand(JSOP_POPN, *npops))
- return false;
-
- *npops = 0;
- return true;
-}
-
namespace {
class NonLocalExitControl
@@ -2718,8 +2711,9 @@ NonLocalExitControl::prepareForNonLocalJump(BytecodeEmitter::NestableControl* ta
bool emitIteratorCloseAtTarget = emitIteratorClose && kind_ != Continue;
auto flushPops = [&npops](BytecodeEmitter* bce) {
- if (npops && !bce->flushPops(&npops))
+ if (npops && !bce->emitUint16Operand(JSOP_POPN, npops))
return false;
+ npops = 0;
return true;
};
diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h
index 7ff40b462..7ac9e540b 100644
--- a/js/src/frontend/BytecodeEmitter.h
+++ b/js/src/frontend/BytecodeEmitter.h
@@ -452,8 +452,6 @@ struct MOZ_STACK_CLASS BytecodeEmitter
JSOp strictifySetNameOp(JSOp op);
- MOZ_MUST_USE bool flushPops(int* npops);
-
MOZ_MUST_USE bool emitCheck(ptrdiff_t delta, ptrdiff_t* offset);
// Emit one bytecode.
diff --git a/js/src/jit/JitFrames.cpp b/js/src/jit/JitFrames.cpp
index a70356ad4..966d952d3 100644
--- a/js/src/jit/JitFrames.cpp
+++ b/js/src/jit/JitFrames.cpp
@@ -436,6 +436,8 @@ HandleExceptionIon(JSContext* cx, const InlineFrameIterator& frame, ResumeFromEx
if (!script->hasTrynotes())
return;
+ bool inForOfIterClose = false;
+
for (TryNoteIterIon tni(cx, frame); !tni.done(); ++tni) {
JSTryNote* tn = *tni;
@@ -447,12 +449,23 @@ HandleExceptionIon(JSContext* cx, const InlineFrameIterator& frame, ResumeFromEx
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.
@@ -583,6 +596,7 @@ ProcessTryNotesBaseline(JSContext* cx, const JitFrameIterator& frame, Environmen
ResumeFromException* rfe, jsbytecode** pc)
{
RootedScript script(cx, frame.baselineFrame()->script());
+ bool inForOfIterClose = false;
for (TryNoteIterBaseline tni(cx, frame.baselineFrame(), *pc); !tni.done(); ++tni) {
JSTryNote* tn = *tni;
@@ -593,7 +607,11 @@ ProcessTryNotesBaseline(JSContext* cx, const JitFrameIterator& frame, Environmen
// If we're closing a legacy generator, we have to skip catch
// blocks.
if (cx->isClosingGenerator())
- continue;
+ break;
+
+ // See corresponding comment in ProcessTryNotes.
+ if (inForOfIterClose)
+ break;
SettleOnTryNote(cx, tn, frame, ei, rfe, pc);
@@ -609,6 +627,10 @@ ProcessTryNotesBaseline(JSContext* cx, const JitFrameIterator& frame, Environmen
}
case JSTRY_FINALLY: {
+ // See corresponding comment in ProcessTryNotes.
+ if (inForOfIterClose)
+ break;
+
SettleOnTryNote(cx, tn, frame, ei, rfe, pc);
rfe->kind = ResumeFromException::RESUME_FINALLY;
rfe->target = script->baselineScript()->nativeCodeForPC(script, *pc);
@@ -652,7 +674,14 @@ ProcessTryNotesBaseline(JSContext* cx, const JitFrameIterator& frame, Environmen
break;
}
+ case JSTRY_FOR_OF_ITERCLOSE:
+ inForOfIterClose = true;
+ break;
+
case JSTRY_FOR_OF:
+ inForOfIterClose = false;
+ break;
+
case JSTRY_LOOP:
break;
diff --git a/js/src/jsscript.h b/js/src/jsscript.h
index 9cb7c538f..87da79901 100644
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -85,6 +85,7 @@ enum JSTryNoteKind {
JSTRY_FOR_IN,
JSTRY_FOR_OF,
JSTRY_LOOP,
+ JSTRY_FOR_OF_ITERCLOSE,
JSTRY_DESTRUCTURING_ITERCLOSE
};
diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp
index 5d6508d59..b53914942 100644
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -2599,6 +2599,8 @@ TryNoteName(JSTryNoteKind kind)
return "for-of";
case JSTRY_LOOP:
return "loop";
+ case JSTRY_FOR_OF_ITERCLOSE:
+ return "for-of-iterclose";
case JSTRY_DESTRUCTURING_ITERCLOSE:
return "dstr-iterclose";
}
@@ -2612,14 +2614,14 @@ 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 {
uint32_t startOff = script->pcToOffset(script->main()) + tn->start;
- if (!sp->jsprintf(" %-14s %6u %8u %8u\n",
+ if (!sp->jsprintf(" %-16s %6u %8u %8u\n",
TryNoteName(static_cast<JSTryNoteKind>(tn->kind)),
tn->stackDepth, startOff, startOff + tn->length))
{
diff --git a/js/src/tests/ecma_6/Statements/for-of-iterator-close-throw.js b/js/src/tests/ecma_6/Statements/for-of-iterator-close-throw.js
new file mode 100644
index 000000000..1974e416b
--- /dev/null
+++ b/js/src/tests/ecma_6/Statements/for-of-iterator-close-throw.js
@@ -0,0 +1,35 @@
+function test() {
+ var returnCalled = 0;
+ var returnCalledExpected = 0;
+ var catchEntered = 0;
+ var finallyEntered = 0;
+ var finallyEnteredExpected = 0;
+ var iterable = {};
+ iterable[Symbol.iterator] = makeIterator({
+ ret: function() {
+ returnCalled++;
+ throw 42;
+ }
+ });
+
+ // inner try cannot catch IteratorClose throwing
+ assertThrowsValue(function() {
+ for (var x of iterable) {
+ try {
+ return;
+ } catch (e) {
+ catchEntered++;
+ } finally {
+ finallyEntered++;
+ }
+ }
+ }, 42);
+ assertEq(returnCalled, ++returnCalledExpected);
+ assertEq(catchEntered, 0);
+ assertEq(finallyEntered, ++finallyEnteredExpected);
+}
+
+test();
+
+if (typeof reportCompare === "function")
+ reportCompare(0, 0);
diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp
index eb3000e07..d20e5284d 100644
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -1156,6 +1156,7 @@ enum HandleErrorContinuation
static HandleErrorContinuation
ProcessTryNotes(JSContext* cx, EnvironmentIter& ei, InterpreterRegs& regs)
{
+ bool inForOfIterClose = false;
for (TryNoteIterInterpreter tni(cx, regs); !tni.done(); ++tni) {
JSTryNote* tn = *tni;
@@ -1164,10 +1165,38 @@ ProcessTryNotes(JSContext* cx, EnvironmentIter& ei, InterpreterRegs& regs)
/* Catch cannot intercept the closing of a generator. */
if (cx->isClosingGenerator())
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;
@@ -1206,7 +1235,14 @@ ProcessTryNotes(JSContext* cx, EnvironmentIter& ei, InterpreterRegs& regs)
break;
}
+ case JSTRY_FOR_OF_ITERCLOSE:
+ inForOfIterClose = true;
+ break;
+
case JSTRY_FOR_OF:
+ inForOfIterClose = false;
+ break;
+
case JSTRY_LOOP:
break;