Bug 1475417 - Part 1: Minor code tweaks, in preparation. There should be no observable change in behavior yet. r=jandem
authorJason Orendorff <jorendorff@mozilla.com>
Thu, 12 Jul 2018 19:43:55 -0500
changeset 481191 da2c87b3210c160afc98ee238f3f61a2a26b3a55
parent 481190 a981419137d9ba16d0b1ce7423e39e4a4ec113de
child 481192 87509a363c9ee2a38998a2e4dacc16e577a877ec
push id232
push userfmarier@mozilla.com
push dateWed, 05 Sep 2018 20:45:54 +0000
reviewersjandem
bugs1475417
milestone63.0a1
Bug 1475417 - Part 1: Minor code tweaks, in preparation. There should be no observable change in behavior yet. r=jandem
js/src/jit/BaselineCompiler.cpp
js/src/jit/VMFunctions.cpp
js/src/vm/GeneratorObject.cpp
js/src/vm/GeneratorObject.h
js/src/vm/Interpreter.cpp
js/src/vm/Stack.h
--- a/js/src/jit/BaselineCompiler.cpp
+++ b/js/src/jit/BaselineCompiler.cpp
@@ -4688,17 +4688,17 @@ BaselineCompiler::emit_JSOP_FINALYIELDRV
 
 typedef bool (*InterpretResumeFn)(JSContext*, HandleObject, HandleValue, HandlePropertyName,
                                   MutableHandleValue);
 static const VMFunction InterpretResumeInfo =
     FunctionInfo<InterpretResumeFn>(jit::InterpretResume, "InterpretResume");
 
 typedef bool (*GeneratorThrowFn)(JSContext*, BaselineFrame*, Handle<GeneratorObject*>,
                                  HandleValue, uint32_t);
-static const VMFunction GeneratorThrowInfo =
+static const VMFunction GeneratorThrowOrReturnInfo =
     FunctionInfo<GeneratorThrowFn>(jit::GeneratorThrowOrReturn, "GeneratorThrowOrReturn", TailCall);
 
 bool
 BaselineCompiler::emit_JSOP_RESUME()
 {
     GeneratorObject::ResumeKind resumeKind = GeneratorObject::getResumeKind(pc);
 
     frame.syncStack(0);
@@ -4873,17 +4873,17 @@ BaselineCompiler::emit_JSOP_RESUME()
         masm.loadBaselineFramePtr(BaselineFrameReg, scratch2);
 
         prepareVMCall();
         pushArg(Imm32(resumeKind));
         pushArg(retVal);
         pushArg(genObj);
         pushArg(scratch2);
 
-        TrampolinePtr code = cx->runtime()->jitRuntime()->getVMWrapper(GeneratorThrowInfo);
+        TrampolinePtr code = cx->runtime()->jitRuntime()->getVMWrapper(GeneratorThrowOrReturnInfo);
 
         // Create the frame descriptor.
         masm.subStackPtrFrom(scratch1);
         masm.makeFrameDescriptor(scratch1, JitFrame_BaselineJS, ExitFrameLayout::Size());
 
         // Push the frame descriptor and a dummy return address (it doesn't
         // matter what we push here, frame iterators will use the frame pc
         // set in jit::GeneratorThrowOrReturn).
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -972,16 +972,20 @@ GeneratorThrowOrReturn(JSContext* cx, Ba
 {
     // Set the frame's pc to the current resume pc, so that frame iterators
     // work. This function always returns false, so we're guaranteed to enter
     // the exception handler where we will clear the pc.
     JSScript* script = frame->script();
     uint32_t offset = script->yieldAndAwaitOffsets()[genObj->yieldAndAwaitIndex()];
     frame->setOverridePc(script->offsetToPC(offset));
 
+    // In the interpreter, GeneratorObject::resume marks the generator as running,
+    // so we do the same.
+    genObj->setRunning();
+
     MOZ_ALWAYS_TRUE(DebugAfterYield(cx, frame));
     MOZ_ALWAYS_FALSE(js::GeneratorThrowOrReturn(cx, frame, genObj, arg, resumeKind));
     return false;
 }
 
 bool
 CheckGlobalOrEvalDeclarationConflicts(JSContext* cx, BaselineFrame* frame)
 {
--- a/js/src/vm/GeneratorObject.cpp
+++ b/js/src/vm/GeneratorObject.cpp
@@ -129,35 +129,33 @@ js::SetGeneratorClosed(JSContext* cx, Ab
 }
 
 bool
 js::GeneratorThrowOrReturn(JSContext* cx, AbstractFramePtr frame, Handle<GeneratorObject*> genObj,
                            HandleValue arg, uint32_t resumeKind)
 {
     if (resumeKind == GeneratorObject::THROW) {
         cx->setPendingException(arg);
-        genObj->setRunning();
     } else {
         MOZ_ASSERT(resumeKind == GeneratorObject::RETURN);
 
         MOZ_ASSERT(arg.isObject());
         frame.setReturnValue(arg);
 
         RootedValue closing(cx, MagicValue(JS_GENERATOR_CLOSING));
         cx->setPendingException(closing);
         genObj->setClosing();
     }
     return false;
 }
 
 bool
 GeneratorObject::resume(JSContext* cx, InterpreterActivation& activation,
-                        HandleObject obj, HandleValue arg, GeneratorObject::ResumeKind resumeKind)
+                        Handle<GeneratorObject*> genObj, HandleValue arg)
 {
-    Rooted<GeneratorObject*> genObj(cx, &obj->as<GeneratorObject>());
     MOZ_ASSERT(genObj->isSuspended());
 
     RootedFunction callee(cx, &genObj->callee());
     RootedObject envChain(cx, &genObj->environmentChain());
     if (!activation.resumeGeneratorFrame(callee, envChain))
         return false;
     activation.regs().fp()->setResumedGenerator();
 
@@ -179,28 +177,18 @@ GeneratorObject::resume(JSContext* cx, I
 
     // Always push on a value, even if we are raising an exception. In the
     // exception case, the stack needs to have something on it so that exception
     // handling doesn't skip the catch blocks. See TryNoteIter::settle.
     activation.regs().sp++;
     MOZ_ASSERT(activation.regs().spForStackDepth(activation.regs().stackDepth()));
     activation.regs().sp[-1] = arg;
 
-    switch (resumeKind) {
-      case NEXT:
-        genObj->setRunning();
-        return true;
-
-      case THROW:
-      case RETURN:
-        return GeneratorThrowOrReturn(cx, activation.regs().fp(), genObj, arg, resumeKind);
-
-      default:
-        MOZ_CRASH("bad resumeKind");
-    }
+    genObj->setRunning();
+    return true;
 }
 
 const Class GeneratorObject::class_ = {
     "Generator",
     JSCLASS_HAS_RESERVED_SLOTS(GeneratorObject::RESERVED_SLOTS)
 };
 
 static const JSFunctionSpec generator_methods[] = {
--- a/js/src/vm/GeneratorObject.h
+++ b/js/src/vm/GeneratorObject.h
@@ -55,17 +55,17 @@ class GeneratorObject : public NativeObj
             return THROW;
         MOZ_ASSERT(atom == cx->names().return_);
         return RETURN;
     }
 
     static JSObject* create(JSContext* cx, AbstractFramePtr frame);
 
     static bool resume(JSContext* cx, InterpreterActivation& activation,
-                       HandleObject obj, HandleValue arg, ResumeKind resumeKind);
+                       Handle<GeneratorObject*> genObj, HandleValue arg);
 
     static bool initialSuspend(JSContext* cx, HandleObject obj, AbstractFramePtr frame, jsbytecode* pc) {
         return suspend(cx, obj, frame, pc, nullptr, 0);
     }
 
     static bool normalSuspend(JSContext* cx, HandleObject obj, AbstractFramePtr frame, jsbytecode* pc,
                               Value* vp, unsigned nvalues) {
         return suspend(cx, obj, frame, pc, vp, nvalues);
@@ -142,17 +142,17 @@ class GeneratorObject : public NativeObj
                       "test below should return false for YIELD_AND_AWAIT_INDEX_RUNNING");
         return getFixedSlot(YIELD_AND_AWAIT_INDEX_SLOT).toInt32() < YIELD_AND_AWAIT_INDEX_CLOSING;
     }
     void setRunning() {
         MOZ_ASSERT(isSuspended());
         setFixedSlot(YIELD_AND_AWAIT_INDEX_SLOT, Int32Value(YIELD_AND_AWAIT_INDEX_RUNNING));
     }
     void setClosing() {
-        MOZ_ASSERT(isSuspended());
+        MOZ_ASSERT(isRunning());
         setFixedSlot(YIELD_AND_AWAIT_INDEX_SLOT, Int32Value(YIELD_AND_AWAIT_INDEX_CLOSING));
     }
     void setYieldAndAwaitIndex(uint32_t yieldAndAwaitIndex) {
         MOZ_ASSERT_IF(yieldAndAwaitIndex == 0,
                       getFixedSlot(YIELD_AND_AWAIT_INDEX_SLOT).isUndefined());
         MOZ_ASSERT_IF(yieldAndAwaitIndex != 0, isRunning() || isClosing());
         MOZ_ASSERT(yieldAndAwaitIndex < uint32_t(YIELD_AND_AWAIT_INDEX_CLOSING));
         setFixedSlot(YIELD_AND_AWAIT_INDEX_SLOT, Int32Value(yieldAndAwaitIndex));
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -4262,36 +4262,46 @@ CASE(JSOP_AWAIT)
     POP_RETURN_VALUE();
 
     goto successful_return_continuation;
 }
 
 CASE(JSOP_RESUME)
 {
     {
-        ReservedRooted<JSObject*> gen(&rootObject0, &REGS.sp[-2].toObject());
+        Rooted<GeneratorObject*> gen(cx, &REGS.sp[-2].toObject().as<GeneratorObject>());
         ReservedRooted<Value> val(&rootValue0, REGS.sp[-1]);
         // popInlineFrame expects there to be an additional value on the stack
         // to pop off, so leave "gen" on the stack.
 
         GeneratorObject::ResumeKind resumeKind = GeneratorObject::getResumeKind(REGS.pc);
-        bool ok = GeneratorObject::resume(cx, activation, gen, val, resumeKind);
+        if (!GeneratorObject::resume(cx, activation, gen, val))
+            goto error;
 
         JSScript* generatorScript = REGS.fp()->script();
         if (cx->realm() != generatorScript->realm())
             cx->enterRealmOf(generatorScript);
         SET_SCRIPT(generatorScript);
 
         TraceLoggerThread* logger = TraceLoggerForCurrentThread(cx);
         TraceLoggerEvent scriptEvent(TraceLogger_Scripts, script);
         TraceLogStartEvent(logger, scriptEvent);
         TraceLogStartEvent(logger, TraceLogger_Interpreter);
 
-        if (!ok)
+        switch (resumeKind) {
+          case GeneratorObject::NEXT:
+            break;
+          case GeneratorObject::THROW:
+          case GeneratorObject::RETURN:
+            MOZ_ALWAYS_FALSE(GeneratorThrowOrReturn(cx, activation.regs().fp(), gen, val,
+                                                    resumeKind));
             goto error;
+          default:
+            MOZ_CRASH("bad resumeKind");
+        }
     }
     ADVANCE_AND_DISPATCH(0);
 }
 
 CASE(JSOP_DEBUGAFTERYIELD)
 {
     // No-op in the interpreter, as GeneratorObject::resume takes care of
     // fixing up InterpreterFrames.
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -2154,17 +2154,17 @@ class FrameIter
     size_t      numFrameSlots() const;
     Value       frameSlotValue(size_t index) const;
 
     // Ensures that we have rematerialized the top frame and its associated
     // inline frames. Can only be called when isIon().
     bool ensureHasRematerializedFrame(JSContext* cx);
 
     // True when isInterp() or isBaseline(). True when isIon() if it
-    // has a rematerialized frame. False otherwise false otherwise.
+    // has a rematerialized frame. False otherwise.
     bool hasUsableAbstractFramePtr() const;
 
     // -----------------------------------------------------------
     // The following functions can only be called when isInterp(),
     // isBaseline(), isWasm() or isIon(). Further, abstractFramePtr() can
     // only be called when hasUsableAbstractFramePtr().
     // -----------------------------------------------------------