Bug 1016936 - Part 1: Throw if the value returned by iterator.next() is not an object. r=jorendorff
authorTooru Fujisawa <arai_a@mac.com>
Wed, 10 Aug 2016 22:26:07 +0900
changeset 308891 ec5f0eea0b6c1fc4b50369ceadbc18972b599953
parent 308890 a5584f816e28a745e476aecb69eac95c7fede022
child 308892 bdfe71b2abc66b2a875f49e10c5557c354748e77
push id30552
push userkwierso@gmail.com
push dateWed, 10 Aug 2016 23:15:29 +0000
treeherdermozilla-central@65520f4cf4cc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1016936
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1016936 - Part 1: Throw if the value returned by iterator.next() is not an object. r=jorendorff
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/BytecodeEmitter.h
js/src/jit-test/tests/generators/bug931414.js
js/src/jsopcode.cpp
js/src/tests/ecma_6/Generators/delegating-yield-1.js
js/src/tests/ecma_6/Generators/delegating-yield-3.js
js/src/tests/ecma_6/Generators/delegating-yield-5.js
js/src/tests/ecma_6/Generators/delegating-yield-7.js
js/src/vm/ForOfIterator.cpp
js/src/vm/Interpreter.cpp
js/src/vm/Interpreter.h
js/src/vm/Opcodes.h
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -452,16 +452,22 @@ BytecodeEmitter::emitDupAt(unsigned slot
     if (!emitN(JSOP_DUPAT, 3, &off))
         return false;
 
     jsbytecode* pc = code(off);
     SET_UINT24(pc, slotFromTop);
     return true;
 }
 
+bool
+BytecodeEmitter::emitCheckIsObj(CheckIsObjectKind kind)
+{
+    return emit2(JSOP_CHECKISOBJ, uint8_t(kind));
+}
+
 static const char*
 StatementName(StmtInfoBCE* stmt)
 {
     if (!stmt)
         return js_script_str;
 
     /* XXX too many "... statement" L10N gaffes -- fix via js.msg! */
     static const char* const statementName[] = {
@@ -4142,16 +4148,18 @@ BytecodeEmitter::emitIteratorNext(ParseN
     if (!emit1(JSOP_DUP))                                 // ... ITER ITER
         return false;
     if (!emitAtomOp(cx->names().next, JSOP_CALLPROP))     // ... ITER NEXT
         return false;
     if (!emit1(JSOP_SWAP))                                // ... NEXT ITER
         return false;
     if (!emitCall(JSOP_CALL, 0, pn))                      // ... RESULT
         return false;
+    if (!emitCheckIsObj(CheckIsObjectKind::IteratorNext)) // ... RESULT
+        return false;
     checkTypeSet(JSOP_CALL);
     return true;
 }
 
 bool
 BytecodeEmitter::emitDefault(ParseNode* defaultExpr)
 {
     if (!emit1(JSOP_DUP))                                 // VALUE VALUE
@@ -7166,16 +7174,18 @@ BytecodeEmitter::emitYieldStar(ParseNode
     if (!emitAtomOp(cx->names().next, JSOP_CALLPROP))            // RECEIVED ITER ITER NEXT
         return false;
     if (!emit1(JSOP_SWAP))                                       // RECEIVED ITER NEXT ITER
         return false;
     if (!emit2(JSOP_PICK, 3))                                    // ITER NEXT ITER RECEIVED
         return false;
     if (!emitCall(JSOP_CALL, 1, iter))                           // ITER RESULT
         return false;
+    if (!emitCheckIsObj(CheckIsObjectKind::IteratorNext))        // ITER RESULT
+        return false;
     checkTypeSet(JSOP_CALL);
     MOZ_ASSERT(this->stackDepth == depth);
 
     if (!emitJumpTargetAndPatch(checkResult))                    // checkResult:
         return false;
 
     // if (!result.done) goto tryStart;                          // ITER RESULT
     if (!emit1(JSOP_DUP))                                        // ITER RESULT RESULT
--- a/js/src/frontend/BytecodeEmitter.h
+++ b/js/src/frontend/BytecodeEmitter.h
@@ -14,16 +14,17 @@
 #include "jscntxt.h"
 #include "jsopcode.h"
 #include "jsscript.h"
 
 #include "frontend/ParseMaps.h"
 #include "frontend/Parser.h"
 #include "frontend/SharedContext.h"
 #include "frontend/SourceNotes.h"
+#include "vm/Interpreter.h"
 
 namespace js {
 
 class ScopeObject;
 
 namespace frontend {
 
 class FullParseHandler;
@@ -499,16 +500,19 @@ struct BytecodeEmitter
 
     // Emit three bytecodes, an opcode with two bytes of immediate operands.
     MOZ_MUST_USE bool emit3(JSOp op, jsbytecode op1, jsbytecode op2);
 
     // Helper to emit JSOP_DUPAT. The argument is the value's depth on the
     // JS stack, as measured from the top.
     MOZ_MUST_USE bool emitDupAt(unsigned slotFromTop);
 
+    // Helper to emit JSOP_CHECKISOBJ.
+    MOZ_MUST_USE bool emitCheckIsObj(CheckIsObjectKind 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);
 
     // Emit a bytecode followed by an uint32 immediate operand.
     MOZ_MUST_USE bool emitUint32Operand(JSOp op, uint32_t operand);
 
     // Emit (1 + extra) bytecodes, for N bytes of op and its immediate operand.
--- a/js/src/jit-test/tests/generators/bug931414.js
+++ b/js/src/jit-test/tests/generators/bug931414.js
@@ -1,9 +1,9 @@
-// |jit-test| error: is undefined
+// |jit-test| error: TypeError
 
 load(libdir + "iteration.js");
 
 function iterable() {
   var iterable = {};
   iterable[Symbol.iterator] = () => ({next: () => void 0});
   return iterable;
 }
--- a/js/src/jsopcode.cpp
+++ b/js/src/jsopcode.cpp
@@ -1223,16 +1223,18 @@ ExpressionDecompiler::decompilePC(jsbyte
       case JSOP_NEWARRAY_COPYONWRITE: {
         JSObject* obj = script->getObject(GET_UINT32_INDEX(pc));
         RootedValue objv(cx, ObjectValue(*obj));
         JSString* str = ValueToSource(cx, objv);
         if (!str)
             return false;
         return write(str);
       }
+      case JSOP_CHECKISOBJ:
+        return decompilePCForStackOperand(pc, -1);
       case JSOP_VOID:
         return write("void ") && decompilePCForStackOperand(pc, -1);
       default:
         break;
     }
     return write("(intermediate value)");
 }
 
--- a/js/src/tests/ecma_6/Generators/delegating-yield-1.js
+++ b/js/src/tests/ecma_6/Generators/delegating-yield-1.js
@@ -27,16 +27,16 @@ function collect_results(iterable) {
     do {
         result = iter.next();
         ret.push(result);
     } while (!result.done);
     return ret;
 }
 
 // We have to put a full result for the end, because the return will re-box.
-var expected = [{value: 1}, 13, "foo", {value: 34, done: true}];
+var expected = [{value: 1}, {value: 34, done: true}];
 
 // Sanity check.
 assertDeepEq(expected, collect_results(results(expected)));
 assertDeepEq(expected, collect_results(yield_results(expected)));
 
 if (typeof reportCompare == "function")
     reportCompare(true, true);
--- a/js/src/tests/ecma_6/Generators/delegating-yield-3.js
+++ b/js/src/tests/ecma_6/Generators/delegating-yield-3.js
@@ -5,36 +5,36 @@ function* delegate(iter) { return yield*
 var GeneratorObjectPrototype = Object.getPrototypeOf(g).prototype;
 var GeneratorObjectPrototype_next = GeneratorObjectPrototype.next;
 
 // Monkeypatch next on an iterator.
 var inner = g(20);
 var outer = delegate(inner);
 assertIteratorNext(outer, 0);
 assertIteratorNext(outer, 1);
-inner.next = function() { return 0; };
+inner.next = function() { return { patched: true }; };
 // 42 yielded directly without re-boxing.
-assertEq(0, outer.next());
+assertEq(true, outer.next().patched);
 // Outer generator not terminated.
-assertEq(0, outer.next());
+assertEq(true, outer.next().patched);
 // Restore.
 inner.next = GeneratorObjectPrototype_next;
 assertIteratorNext(outer, 2);
 // Repatch.
 inner.next = function() { return { value: 42, done: true }; };
 assertIteratorDone(outer, 42);
 
 // Monkeypunch next on the prototype.
 var inner = g(20);
 var outer = delegate(inner);
 assertIteratorNext(outer, 0);
 assertIteratorNext(outer, 1);
-GeneratorObjectPrototype.next = function() { return 0; };
+GeneratorObjectPrototype.next = function() { return { patched: true }; };
 // 42 yielded directly without re-boxing.
-assertEq(0, GeneratorObjectPrototype_next.call(outer));
+assertEq(true, GeneratorObjectPrototype_next.call(outer).patched);
 // Outer generator not terminated.
-assertEq(0, GeneratorObjectPrototype_next.call(outer));
+assertEq(true, GeneratorObjectPrototype_next.call(outer).patched);
 // Restore.
 GeneratorObjectPrototype.next = GeneratorObjectPrototype_next;
 assertIteratorNext(outer, 2);
 
 if (typeof reportCompare == "function")
     reportCompare(true, true);
--- a/js/src/tests/ecma_6/Generators/delegating-yield-5.js
+++ b/js/src/tests/ecma_6/Generators/delegating-yield-5.js
@@ -23,15 +23,15 @@ function collect_results(iterable) {
     do {
         result = iter.next();
         ret.push(result);
     } while (!result.done);
     return ret;
 }
 
 // We have to put a full result for the end, because the return will re-box.
-var expected = [{value: 1}, 13, "foo", {value: 34, done: true}];
+var expected = [{value: 1}, {value: 34, done: true}];
 
 assertDeepEq(expected, collect_results(results(expected)));
 assertDeepEq(expected, collect_results(yield_results(expected, 20)));
 
 if (typeof reportCompare == "function")
     reportCompare(true, true);
--- a/js/src/tests/ecma_6/Generators/delegating-yield-7.js
+++ b/js/src/tests/ecma_6/Generators/delegating-yield-7.js
@@ -23,16 +23,16 @@ function collect_results(iter) {
     do {
         result = iter.next();
         ret.push(result);
     } while (!result.done);
     return ret;
 }
 
 // We have to put a full result for the end, because the return will re-box.
-var expected = [{value: 1}, 13, "foo", {value: 34, done: true}];
+var expected = [{value: 1}, {value: 34, done: true}];
 
 // Sanity check.
 assertDeepEq(expected, collect_results(results(expected)));
 assertDeepEq(expected, collect_results(yield_results(expected)));
 
 if (typeof reportCompare == "function")
     reportCompare(true, true);
--- a/js/src/vm/ForOfIterator.cpp
+++ b/js/src/vm/ForOfIterator.cpp
@@ -130,20 +130,20 @@ ForOfIterator::next(MutableHandleValue v
 
     RootedValue v(cx_);
     if (!GetProperty(cx_, iterator, iterator, cx_->names().next, &v))
         return false;
 
     if (!js::Call(cx_, v, iterator, &v))
         return false;
 
-    RootedObject resultObj(cx_, ToObject(cx_, v));
-    if (!resultObj)
-        return false;
+    if (!v.isObject())
+        return ThrowCheckIsObject(cx_, CheckIsObjectKind::IteratorNext);
 
+    RootedObject resultObj(cx_, &v.toObject());
     if (!GetProperty(cx_, resultObj, resultObj, cx_->names().done, &v))
         return false;
 
     *done = ToBoolean(v);
     if (*done) {
         vp.setUndefined();
         return true;
     }
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -1820,17 +1820,16 @@ CASE(EnableInterruptsPseudoOpcode)
     /* Commence executing the actual opcode. */
     SANITY_CHECKS();
     DISPATCH_TO(op);
 }
 
 /* Various 1-byte no-ops. */
 CASE(JSOP_NOP)
 CASE(JSOP_NOP_DESTRUCTURING)
-CASE(JSOP_UNUSED14)
 CASE(JSOP_UNUSED149)
 CASE(JSOP_UNUSED179)
 CASE(JSOP_UNUSED180)
 CASE(JSOP_UNUSED181)
 CASE(JSOP_UNUSED182)
 CASE(JSOP_UNUSED183)
 CASE(JSOP_UNUSED187)
 CASE(JSOP_UNUSED192)
@@ -2551,16 +2550,25 @@ CASE(JSOP_GLOBALTHIS)
             goto error;
     } else {
         ClonedBlockObject* lexicalScope = &cx->global()->lexicalScope();
         PUSH_COPY(lexicalScope->thisValue());
     }
 }
 END_CASE(JSOP_GLOBALTHIS)
 
+CASE(JSOP_CHECKISOBJ)
+{
+    if (!REGS.sp[-1].isObject()) {
+        MOZ_ALWAYS_FALSE(ThrowCheckIsObject(cx, CheckIsObjectKind(GET_UINT8(REGS.pc))));
+        goto error;
+    }
+}
+END_CASE(JSOP_CHECKISOBJ)
+
 CASE(JSOP_CHECKTHIS)
 {
     if (REGS.sp[-1].isMagic(JS_UNINITIALIZED_LEXICAL)) {
         MOZ_ALWAYS_FALSE(ThrowUninitializedThis(cx, REGS.fp()));
         goto error;
     }
 }
 END_CASE(JSOP_CHECKTHIS)
@@ -4964,16 +4972,29 @@ js::ReportRuntimeRedeclaration(JSContext
         else
             kindStr = frontend::Definition::kindString(declKind);
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_REDECLARED_VAR,
                              kindStr, printable.ptr());
     }
 }
 
 bool
+js::ThrowCheckIsObject(JSContext* cx, CheckIsObjectKind kind)
+{
+    switch (kind) {
+      case CheckIsObjectKind::IteratorNext:
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_NEXT_RETURNED_PRIMITIVE);
+        break;
+      default:
+        MOZ_CRASH("Unknown kind");
+    }
+    return false;
+}
+
+bool
 js::ThrowUninitializedThis(JSContext* cx, AbstractFramePtr frame)
 {
     RootedFunction fun(cx);
     if (frame.isFunctionFrame()) {
         fun = frame.callee();
     } else {
         MOZ_ASSERT(frame.isEvalFrame());
         MOZ_ASSERT(frame.script()->isDirectEvalInFunction());
--- a/js/src/vm/Interpreter.h
+++ b/js/src/vm/Interpreter.h
@@ -554,16 +554,23 @@ ReportRuntimeLexicalError(JSContext* cx,
 
 // The parser only reports redeclarations that occurs within a single
 // script. Due to the extensibility of the global lexical scope, we also check
 // for redeclarations during runtime in JSOP_DEF{VAR,LET,CONST}.
 void
 ReportRuntimeRedeclaration(JSContext* cx, HandlePropertyName name,
                            frontend::Definition::Kind declKind);
 
+enum class CheckIsObjectKind : uint8_t {
+    IteratorNext
+};
+
+bool
+ThrowCheckIsObject(JSContext* cx, CheckIsObjectKind kind);
+
 bool
 ThrowUninitializedThis(JSContext* cx, AbstractFramePtr frame);
 
 bool
 DefaultClassConstructor(JSContext* cx, unsigned argc, Value* vp);
 
 bool
 Debug_CheckSelfHosted(JSContext* cx, HandleValue v);
--- a/js/src/vm/Opcodes.h
+++ b/js/src/vm/Opcodes.h
@@ -215,17 +215,26 @@ 1234567890123456789012345678901234567890
      * Duplicates the top two values on the stack.
      *   Category: Operators
      *   Type: Stack Operations
      *   Operands:
      *   Stack: v1, v2 => v1, v2, v1, v2
      */ \
     macro(JSOP_DUP2,      13, "dup2",       NULL,         1,  2,  4, JOF_BYTE) \
     \
-    macro(JSOP_UNUSED14,  14, "unused14",   NULL,         1,  0,  0, JOF_BYTE) \
+    /*
+     * Checks that the top value on the stack is an object, and throws a
+     * TypeError if not. The operand 'kind' is used only to generate an
+     * appropriate error message.
+     *   Category: Statements
+     *   Type: Generator
+     *   Operands: uint8_t kind
+     *   Stack: result => result
+     */ \
+    macro(JSOP_CHECKISOBJ,14, "checkisobj", NULL,         2,  1,  1, JOF_UINT8) \
     \
     /*
      * Pops the top two values 'lval' and 'rval' from the stack, then pushes
      * the result of the operation applied to the two operands, converting
      * both to 32-bit signed integers if necessary.
      *   Category: Operators
      *   Type: Bitwise Logical Operators
      *   Operands: