Bug 1342483 - Preserve envChain in Ion if script uses lexical environments draft
authorTed Campbell <tcampbell@mozilla.com>
Fri, 03 Mar 2017 10:10:13 -0500
changeset 493200 46167a2732ac
parent 493198 21a259aa0f24
child 547790 3158e77f6013
push id47682
push userbmo:tcampbell@mozilla.com
push dateFri, 03 Mar 2017 15:40:01 +0000
bugs1342483
milestone54.0a1
Bug 1342483 - Preserve envChain in Ion if script uses lexical environments Under rare cases, Ion was optimizing out |envChain| while lexical environments were in use, leading to a crash during bailout. This extends the criteria for preserving the |envChain| slot to include lexical blocks. MozReview-Commit-ID: 4sd42F4TIq8
js/src/jit-test/tests/ion/bug1342483-1.js
js/src/jit-test/tests/ion/bug1342483-2.js
js/src/jit/BaselineBailouts.cpp
js/src/jit/CompileInfo.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug1342483-1.js
@@ -0,0 +1,6 @@
+// |jit-test| error: ReferenceError
+for (var i = 0; i < 10; ++i) {}
+for (var i = 0; i < 3; i++) {
+    throw eval(raisesException);
+    function ff() {}
+}
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug1342483-2.js
@@ -0,0 +1,17 @@
+// |jit-test| error: () => g
+function f() {
+    // Block Scope
+    {
+        // Lexical capture creates environment
+        function g() {}
+        var h = [() => g];
+
+        // OSR Re-Entry Point
+        for (;;) { break; }
+
+        // Type Invalidation + Throw
+        throw h[0];
+    }
+}
+
+f();
--- a/js/src/jit/BaselineBailouts.cpp
+++ b/js/src/jit/BaselineBailouts.cpp
@@ -731,16 +731,31 @@ InitFromBailout(JSContext* cx, HandleScr
             if (fun && fun->needsFunctionEnvironmentObjects()) {
                 MOZ_ASSERT(fun->nonLazyScript()->initialEnvironmentShape());
                 MOZ_ASSERT(!fun->needsExtraBodyVarEnvironment());
                 flags |= BaselineFrame::HAS_INITIAL_ENV;
             }
         } else {
             MOZ_ASSERT(v.isUndefined() || v.isMagic(JS_OPTIMIZED_OUT));
 
+#ifdef DEBUG
+            // The |envChain| slot must not be optimized out if the currently
+            // active scope requires any EnvironmentObjects beyond what is
+            // available at body scope. This checks that scope chain does not
+            // require any such EnvironmentObjects.
+            // See also: |CompileInfo::isObservableFrameSlot|
+            jsbytecode* pc = script->offsetToPC(iter.pcOffset());
+            Scope* scopeIter = script->innermostScope(pc);
+            while (scopeIter != script->bodyScope()) {
+                MOZ_ASSERT(scopeIter);
+                MOZ_ASSERT(!scopeIter->hasEnvironment());
+                scopeIter = scopeIter->enclosing();
+            }
+#endif
+
             // Get env chain from function or script.
             if (fun) {
                 // If pcOffset == 0, we may have to push a new call object, so
                 // we leave envChain nullptr and enter baseline code before
                 // the prologue.
                 if (!IsPrologueBailout(iter, excInfo))
                     envChain = fun->environment();
             } else if (script->module()) {
--- a/js/src/jit/CompileInfo.h
+++ b/js/src/jit/CompileInfo.h
@@ -235,22 +235,27 @@ class CompileInfo
                     continue;
                 BindingLocation loc = bi.location();
                 if (loc.kind() == BindingLocation::Kind::Frame) {
                     thisSlotForDerivedClassConstructor_ = mozilla::Some(localSlot(loc.slot()));
                     break;
                 }
             }
         }
+
+        // If the script uses an environment in body, the environment chain
+        // will need to be observable.
+        needsBodyEnvironmentObject_ = script->needsBodyEnvironment();
     }
 
     explicit CompileInfo(unsigned nlocals)
       : script_(nullptr), fun_(nullptr), osrPc_(nullptr),
         analysisMode_(Analysis_None), scriptNeedsArgsObj_(false),
-        mayReadFrameArgsDirectly_(false), inlineScriptTree_(nullptr)
+        mayReadFrameArgsDirectly_(false), inlineScriptTree_(nullptr),
+        needsBodyEnvironmentObject_(false)
     {
         nimplicit_ = 0;
         nargs_ = 0;
         nlocals_ = nlocals;
         nstack_ = 1;  /* For FunctionCompiler::pushPhiInput/popPhiOutput */
         nslots_ = nlocals_ + nstack_;
     }
 
@@ -429,31 +434,40 @@ class CompileInfo
     AnalysisMode analysisMode() const {
         return analysisMode_;
     }
 
     bool isAnalysis() const {
         return analysisMode_ != Analysis_None;
     }
 
+    bool needsBodyEnvironmentObject() const {
+        return needsBodyEnvironmentObject_;
+    }
+
     // Returns true if a slot can be observed out-side the current frame while
     // the frame is active on the stack.  This implies that these definitions
     // would have to be executed and that they cannot be removed even if they
     // are unused.
     bool isObservableSlot(uint32_t slot) const {
         if (isObservableFrameSlot(slot))
             return true;
 
         if (isObservableArgumentSlot(slot))
             return true;
 
         return false;
     }
 
     bool isObservableFrameSlot(uint32_t slot) const {
+        // The |envChain| value must be preserved if environments are added
+        // after the prologue.
+        if (needsBodyEnvironmentObject() && slot == environmentChainSlot())
+            return true;
+
         if (!funMaybeLazy())
             return false;
 
         // The |this| value must always be observable.
         if (slot == thisSlot())
             return true;
 
         // The |this| frame slot in derived class constructors should never be
@@ -490,19 +504,21 @@ class CompileInfo
 
         return false;
     }
 
     // Returns true if a slot can be recovered before or during a bailout.  A
     // definition which can be observed and recovered, implies that this
     // definition can be optimized away as long as we can compute its values.
     bool isRecoverableOperand(uint32_t slot) const {
-        // If this script is not a function, then none of the slots are
-        // observable.  If it this |slot| is not observable, thus we can always
-        // recover it.
+        // The |envChain| value cannot be recovered if environments can be
+        // added in body (after the prologue).
+        if (needsBodyEnvironmentObject() && slot == environmentChainSlot())
+            return false;
+
         if (!funMaybeLazy())
             return true;
 
         // The |this| and the |envChain| values can be recovered.
         if (slot == thisSlot() || slot == environmentChainSlot())
             return true;
 
         if (isObservableFrameSlot(slot))
@@ -542,14 +558,18 @@ class CompileInfo
 
     // Record the state of previous bailouts in order to prevent compiling the
     // same function identically the next time.
     bool hadOverflowBailout_;
 
     bool mayReadFrameArgsDirectly_;
 
     InlineScriptTree* inlineScriptTree_;
+
+    // Whether a script needs environments within its body. This informs us
+    // that the environment chain is not easy to reconstruct.
+    bool needsBodyEnvironmentObject_;
 };
 
 } // namespace jit
 } // namespace js
 
 #endif /* jit_CompileInfo_h */