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
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 */