Bug 1308578 - Consider .this bindings of derived class constructors to be always observable. (r=jandem)
authorShu-yu Guo <shu@rfrn.org>
Tue, 08 Nov 2016 15:46:05 -0800
changeset 351823 5dd60f3331c0a520c41b40d273c2959ba5dc4f73
parent 351822 7fb5ed60089575611b832109c91f501a0a7c3f14
child 351824 5174d6ae8e9c3c2393e8fd7f4fee05b28b99957c
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1308578
milestone52.0a1
Bug 1308578 - Consider .this bindings of derived class constructors to be always observable. (r=jandem)
js/src/jit-test/tests/debug/bug1308578.js
js/src/jit/CompileInfo.h
js/src/vm/Debugger.cpp
js/src/vm/EnvironmentObject.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/bug1308578.js
@@ -0,0 +1,10 @@
+// |jit-test| error: ReferenceError
+
+g = newGlobal();
+g.parent = this;
+g.eval("new Debugger(parent).onExceptionUnwind = function () {}");
+a = new class extends Array {
+    constructor() {
+        for (;; ([] = p)) {}
+    }
+}
--- a/js/src/jit/CompileInfo.h
+++ b/js/src/jit/CompileInfo.h
@@ -2,16 +2,18 @@
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef jit_CompileInfo_h
 #define jit_CompileInfo_h
 
+#include "mozilla/Maybe.h"
+
 #include "jsfun.h"
 
 #include "jit/JitAllocPolicy.h"
 #include "jit/JitFrames.h"
 #include "jit/Registers.h"
 #include "vm/EnvironmentObject.h"
 
 namespace js {
@@ -216,16 +218,33 @@ class CompileInfo
         }
 
         nimplicit_ = StartArgSlot(script)                   /* env chain and argument obj */
                    + (fun ? 1 : 0);                         /* this */
         nargs_ = fun ? fun->nargs() : 0;
         nlocals_ = script->nfixed();
         nstack_ = Max<unsigned>(script->nslots() - script->nfixed(), MinJITStackSize);
         nslots_ = nimplicit_ + nargs_ + nlocals_ + nstack_;
+
+        // For derived class constructors, find and cache the frame slot for
+        // the .this binding. This slot is assumed to be always
+        // observable. See isObservableFrameSlot.
+        if (script->isDerivedClassConstructor()) {
+            MOZ_ASSERT(script->functionHasThisBinding());
+            CompileRuntime* runtime = GetJitContext()->runtime;
+            for (BindingIter bi(script); bi; bi++) {
+                if (bi.name() != runtime->names().dotThis)
+                    continue;
+                BindingLocation loc = bi.location();
+                if (loc.kind() == BindingLocation::Kind::Frame) {
+                    thisSlotForDerivedClassConstructor_ = mozilla::Some(localSlot(loc.slot()));
+                    break;
+                }
+            }
+        }
     }
 
     explicit CompileInfo(unsigned nlocals)
       : script_(nullptr), fun_(nullptr), osrPc_(nullptr),
         analysisMode_(Analysis_None), scriptNeedsArgsObj_(false),
         mayReadFrameArgsDirectly_(false), inlineScriptTree_(nullptr)
     {
         nimplicit_ = 0;
@@ -432,16 +451,23 @@ class CompileInfo
     bool isObservableFrameSlot(uint32_t slot) const {
         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
+        // optimized out, as a Debugger might need to perform TDZ checks on it
+        // via, e.g., an exceptionUnwind handler. The TDZ check is required
+        // for correctness if the handler decides to continue execution.
+        if (thisSlotForDerivedClassConstructor_ && *thisSlotForDerivedClassConstructor_ == slot)
+            return true;
+
         if (funMaybeLazy()->needsSomeEnvironmentObject() && slot == environmentChainSlot())
             return true;
 
         // If the function may need an arguments object, then make sure to
         // preserve the env chain, because it may be needed to construct the
         // arguments object during bailout. If we've already created an
         // arguments object (or got one via OSR), preserve that as well.
         if (hasArguments() && (slot == environmentChainSlot() || slot == argsObjSlot()))
@@ -498,16 +524,17 @@ class CompileInfo
     }
 
   private:
     unsigned nimplicit_;
     unsigned nargs_;
     unsigned nlocals_;
     unsigned nstack_;
     unsigned nslots_;
+    mozilla::Maybe<unsigned> thisSlotForDerivedClassConstructor_;
     JSScript* script_;
     JSFunction* fun_;
     jsbytecode* osrPc_;
     AnalysisMode analysisMode_;
 
     // Whether a script needs an arguments object is unstable over compilation
     // since the arguments optimization could be marked as failed on the main
     // thread, so cache a value here and use it throughout for consistency.
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -1556,19 +1556,18 @@ ParseResumptionValue(JSContext* cx, Hand
 static bool
 CheckResumptionValue(JSContext* cx, AbstractFramePtr frame, const Maybe<HandleValue>& maybeThisv,
                      JSTrapStatus status, MutableHandleValue vp)
 {
     if (maybeThisv.isSome()) {
         const HandleValue& thisv = maybeThisv.ref();
         if (status == JSTRAP_RETURN && vp.isPrimitive()) {
             if (vp.isUndefined()) {
-                if (thisv.isMagic(JS_UNINITIALIZED_LEXICAL)) {
+                if (thisv.isMagic(JS_UNINITIALIZED_LEXICAL))
                     return ThrowUninitializedThis(cx, frame);
-                }
 
                 vp.set(thisv);
             } else {
                 ReportValueError(cx, JSMSG_BAD_DERIVED_RETURN, JSDVG_IGNORE_STACK, vp, nullptr);
                 return false;
             }
         }
     }
--- a/js/src/vm/EnvironmentObject.cpp
+++ b/js/src/vm/EnvironmentObject.cpp
@@ -3128,17 +3128,17 @@ js::GetThisValueForDebuggerMaybeOptimize
 
             BindingLocation loc = bi.location();
             if (loc.kind() == BindingLocation::Kind::Environment) {
                 RootedObject callObj(cx, &ei.environment().as<CallObject>());
                 return GetProperty(cx, callObj, callObj, bi.name()->asPropertyName(), res);
             }
 
             if (loc.kind() == BindingLocation::Kind::Frame && ei.withinInitialFrame())
-                res.set(frame.unaliasedLocal(bi.location().slot()));
+                res.set(frame.unaliasedLocal(loc.slot()));
             else
                 res.setMagic(JS_OPTIMIZED_OUT);
 
             return true;
         }
 
         MOZ_CRASH("'this' binding must be found");
     }