Bug 1600808 - Verify frame state in individual frame functions. r=jimb
authorLogan Smyth <loganfsmyth@gmail.com>
Fri, 06 Dec 2019 02:28:48 +0000
changeset 567944 464a15bfedb87e6dda633d65bea9ea5d4761b804
parent 567943 482892b2dd97385d2ab07520b5d4884d8441aa26
child 567945 a22c83382c801f86bfb724224db590bfb8961059
push id12493
push userffxbld-merge
push dateMon, 06 Jan 2020 15:38:57 +0000
treeherdermozilla-beta@63ae456b848d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs1600808
milestone73.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 1600808 - Verify frame state in individual frame functions. r=jimb Differential Revision: https://phabricator.services.mozilla.com/D55915
js/src/debugger/Frame.cpp
js/src/debugger/Frame.h
--- a/js/src/debugger/Frame.cpp
+++ b/js/src/debugger/Frame.cpp
@@ -1195,18 +1195,17 @@ void DebuggerFrame::trace(JSTracer* trc)
   }
 
   if (hasGenerator()) {
     generatorInfo()->trace(trc, *this);
   }
 }
 
 /* static */
-DebuggerFrame* DebuggerFrame::check(JSContext* cx, HandleValue thisv,
-                                    MinState minState) {
+DebuggerFrame* DebuggerFrame::check(JSContext* cx, HandleValue thisv) {
   JSObject* thisobj = RequireObject(cx, thisv);
   if (!thisobj) {
     return nullptr;
   }
   if (thisobj->getClass() != &class_) {
     JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
                               JSMSG_INCOMPATIBLE_PROTO, "Debugger.Frame",
                               "method", thisobj->getClass()->name);
@@ -1222,36 +1221,16 @@ DebuggerFrame* DebuggerFrame::check(JSCo
   if (!frame->getPrivate() &&
       frame->getReservedSlot(OWNER_SLOT).isUndefined()) {
     JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
                               JSMSG_INCOMPATIBLE_PROTO, "Debugger.Frame",
                               "method", "prototype object");
     return nullptr;
   }
 
-  switch (minState) {
-    case MinState::OnStack:
-      if (!frame->isOnStack()) {
-        JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                                  JSMSG_DEBUG_NOT_ON_STACK, "Debugger.Frame");
-        return nullptr;
-      }
-      break;
-    case MinState::OnStackOrSuspended:
-      if (!frame->isOnStack() && !frame->hasGenerator()) {
-        JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                                  JSMSG_DEBUG_NOT_ON_STACK_OR_SUSPENDED,
-                                  "Debugger.Frame");
-        return nullptr;
-      }
-      break;
-    case MinState::OnStackOrSuspendedOrTerminated:
-      break;
-  }
-
   return frame;
 }
 
 struct MOZ_STACK_CLASS DebuggerFrame::CallData {
   JSContext* cx;
   const CallArgs& args;
 
   HandleDebuggerFrame frame;
@@ -1280,52 +1259,70 @@ struct MOZ_STACK_CLASS DebuggerFrame::Ca
   bool onPopSetter();
   bool evalMethod();
   bool evalWithBindingsMethod();
 
   using Method = bool (CallData::*)();
 
   template <Method MyMethod>
   static bool ToNative(JSContext* cx, unsigned argc, Value* vp);
+
+  bool ensureOnStack() const;
+  bool ensureOnStackOrSuspended() const;
 };
 
 template <DebuggerFrame::CallData::Method MyMethod>
 /* static */
 bool DebuggerFrame::CallData::ToNative(JSContext* cx, unsigned argc,
                                        Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
 
-  MinState minState = MinState::OnStack;
-  if (MyMethod == &CallData::asyncPromiseGetter ||
-      MyMethod == &CallData::calleeGetter || MyMethod == &CallData::getScript ||
-      MyMethod == &CallData::offsetGetter) {
-    minState = MinState::OnStackOrSuspended;
-  } else if (
-      // These methods do not require any frame metadata.
-      MyMethod == &CallData::liveGetter ||
-      MyMethod == &CallData::onStackGetter ||
-      MyMethod == &CallData::terminatedGetter ||
-      MyMethod == &CallData::onStepGetter ||
-      MyMethod == &CallData::onStepSetter ||
-      MyMethod == &CallData::onPopGetter ||
-      MyMethod == &CallData::onPopSetter) {
-    minState = MinState::OnStackOrSuspendedOrTerminated;
-  }
-
-  RootedDebuggerFrame frame(cx,
-                            DebuggerFrame::check(cx, args.thisv(), minState));
+  RootedDebuggerFrame frame(cx, DebuggerFrame::check(cx, args.thisv()));
   if (!frame) {
     return false;
   }
 
   CallData data(cx, args, frame);
   return (data.*MyMethod)();
 }
 
+static bool EnsureOnStack(JSContext* cx, HandleDebuggerFrame frame) {
+  MOZ_ASSERT(frame);
+  if (!frame->isOnStack()) {
+    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                              JSMSG_DEBUG_NOT_ON_STACK, "Debugger.Frame");
+    return false;
+  }
+
+  return true;
+}
+static bool EnsureOnStackOrSuspended(JSContext* cx, HandleDebuggerFrame frame) {
+  MOZ_ASSERT(frame);
+  if (!frame->isOnStack() && !frame->hasGenerator()) {
+    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                              JSMSG_DEBUG_NOT_ON_STACK_OR_SUSPENDED,
+                              "Debugger.Frame");
+    return false;
+  }
+
+  return true;
+}
+
+bool DebuggerFrame::CallData::ensureOnStack() const {
+  return EnsureOnStack(cx, frame);
+}
+bool DebuggerFrame::CallData::ensureOnStackOrSuspended() const {
+  return EnsureOnStackOrSuspended(cx, frame);
+}
+
 bool DebuggerFrame::CallData::typeGetter() {
+  if (!ensureOnStack()) {
+    return false;
+  }
+
   DebuggerFrameType type = DebuggerFrame::getType(frame);
 
   JSString* str;
   switch (type) {
     case DebuggerFrameType::Eval:
       str = cx->names().eval;
       break;
     case DebuggerFrameType::Global:
@@ -1344,16 +1341,20 @@ bool DebuggerFrame::CallData::typeGetter
       MOZ_CRASH("bad DebuggerFrameType value");
   }
 
   args.rval().setString(str);
   return true;
 }
 
 bool DebuggerFrame::CallData::implementationGetter() {
+  if (!ensureOnStack()) {
+    return false;
+  }
+
   DebuggerFrameImplementation implementation =
       DebuggerFrame::getImplementation(frame);
 
   const char* s;
   switch (implementation) {
     case DebuggerFrameImplementation::Baseline:
       s = "baseline";
       break;
@@ -1375,51 +1376,71 @@ bool DebuggerFrame::CallData::implementa
     return false;
   }
 
   args.rval().setString(str);
   return true;
 }
 
 bool DebuggerFrame::CallData::environmentGetter() {
+  if (!ensureOnStack()) {
+    return false;
+  }
+
   RootedDebuggerEnvironment result(cx);
   if (!DebuggerFrame::getEnvironment(cx, frame, &result)) {
     return false;
   }
 
   args.rval().setObject(*result);
   return true;
 }
 
 bool DebuggerFrame::CallData::calleeGetter() {
+  if (!ensureOnStackOrSuspended()) {
+    return false;
+  }
+
   RootedDebuggerObject result(cx);
   if (!DebuggerFrame::getCallee(cx, frame, &result)) {
     return false;
   }
 
   args.rval().setObjectOrNull(result);
   return true;
 }
 
 bool DebuggerFrame::CallData::generatorGetter() {
+  if (!ensureOnStack()) {
+    return false;
+  }
+
   args.rval().setBoolean(DebuggerFrame::getIsGenerator(frame));
   return true;
 }
 
 bool DebuggerFrame::CallData::constructingGetter() {
+  if (!ensureOnStack()) {
+    return false;
+  }
+
   bool result;
   if (!DebuggerFrame::getIsConstructing(cx, frame, result)) {
     return false;
   }
 
   args.rval().setBoolean(result);
   return true;
 }
 
 bool DebuggerFrame::CallData::asyncPromiseGetter() {
+  if (!ensureOnStackOrSuspended()) {
+    return false;
+  }
+
   RootedScript script(cx);
   if (frame->isOnStack()) {
     FrameIter iter(*frame->frameIterData());
     AbstractFramePtr framePtr = iter.abstractFramePtr();
 
     if (!framePtr.isWasmDebugFrame()) {
       script = framePtr.script();
     }
@@ -1439,20 +1460,28 @@ bool DebuggerFrame::CallData::asyncPromi
     return false;
   }
 
   args.rval().setObjectOrNull(result);
   return true;
 }
 
 bool DebuggerFrame::CallData::thisGetter() {
+  if (!ensureOnStack()) {
+    return false;
+  }
+
   return DebuggerFrame::getThis(cx, frame, args.rval());
 }
 
 bool DebuggerFrame::CallData::olderGetter() {
+  if (!ensureOnStack()) {
+    return false;
+  }
+
   RootedDebuggerFrame result(cx);
   if (!DebuggerFrame::getOlder(cx, frame, &result)) {
     return false;
   }
 
   args.rval().setObjectOrNull(result);
   return true;
 }
@@ -1472,19 +1501,18 @@ static bool DebuggerArguments_getArg(JSC
     JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
                               JSMSG_INCOMPATIBLE_PROTO, "Arguments",
                               "getArgument", argsobj->getClass()->name);
     return false;
   }
 
   RootedValue framev(cx, argsobj->as<NativeObject>().getReservedSlot(
                              JSSLOT_DEBUGARGUMENTS_FRAME));
-  RootedDebuggerFrame thisobj(
-      cx, DebuggerFrame::check(cx, framev, DebuggerFrame::MinState::OnStack));
-  if (!thisobj) {
+  RootedDebuggerFrame thisobj(cx, DebuggerFrame::check(cx, framev));
+  if (!thisobj || !EnsureOnStack(cx, thisobj)) {
     return false;
   }
 
   FrameIter frameIter(*thisobj->frameIterData());
   AbstractFramePtr frame = frameIter.abstractFramePtr();
 
   // TODO handle wasm frame arguments -- they are not yet reflectable.
   MOZ_ASSERT(!frame.isWasmDebugFrame(), "a wasm frame args");
@@ -1567,26 +1595,34 @@ DebuggerArguments* DebuggerArguments::cr
     }
     getobj->setExtendedSlot(0, Int32Value(i));
   }
 
   return obj;
 }
 
 bool DebuggerFrame::CallData::argumentsGetter() {
+  if (!ensureOnStack()) {
+    return false;
+  }
+
   RootedDebuggerArguments result(cx);
   if (!DebuggerFrame::getArguments(cx, frame, &result)) {
     return false;
   }
 
   args.rval().setObjectOrNull(result);
   return true;
 }
 
 bool DebuggerFrame::CallData::getScript() {
+  if (!ensureOnStackOrSuspended()) {
+    return false;
+  }
+
   RootedDebuggerScript scriptObject(cx);
 
   Debugger* debug = Debugger::fromChildJSObject(frame);
   if (frame->isOnStack()) {
     FrameIter iter(*frame->frameIterData());
     AbstractFramePtr framePtr = iter.abstractFramePtr();
 
     if (framePtr.isWasmDebugFrame()) {
@@ -1605,16 +1641,20 @@ bool DebuggerFrame::CallData::getScript(
     return false;
   }
 
   args.rval().setObject(*scriptObject);
   return true;
 }
 
 bool DebuggerFrame::CallData::offsetGetter() {
+  if (!ensureOnStackOrSuspended()) {
+    return false;
+  }
+
   size_t result;
   if (!DebuggerFrame::getOffset(cx, frame, result)) {
     return false;
   }
 
   args.rval().setNumber(double(result));
   return true;
 }
@@ -1706,16 +1746,20 @@ bool DebuggerFrame::CallData::onPopSette
 
   frame->setOnPopHandler(cx, handler);
 
   args.rval().setUndefined();
   return true;
 }
 
 bool DebuggerFrame::CallData::evalMethod() {
+  if (!ensureOnStack()) {
+    return false;
+  }
+
   if (!args.requireAtLeast(cx, "Debugger.Frame.prototype.eval", 1)) {
     return false;
   }
 
   AutoStableStringChars stableChars(cx);
   if (!ValueToStableChars(cx, "Debugger.Frame.prototype.eval", args[0],
                           stableChars)) {
     return false;
@@ -1729,16 +1773,20 @@ bool DebuggerFrame::CallData::evalMethod
 
   Rooted<Completion> comp(cx);
   JS_TRY_VAR_OR_RETURN_FALSE(
       cx, comp, DebuggerFrame::eval(cx, frame, chars, nullptr, options));
   return comp.get().buildCompletionValue(cx, frame->owner(), args.rval());
 }
 
 bool DebuggerFrame::CallData::evalWithBindingsMethod() {
+  if (!ensureOnStack()) {
+    return false;
+  }
+
   if (!args.requireAtLeast(cx, "Debugger.Frame.prototype.evalWithBindings",
                            2)) {
     return false;
   }
 
   AutoStableStringChars stableChars(cx);
   if (!ValueToStableChars(cx, "Debugger.Frame.prototype.evalWithBindings",
                           args[0], stableChars)) {
--- a/js/src/debugger/Frame.h
+++ b/js/src/debugger/Frame.h
@@ -180,29 +180,17 @@ class DebuggerFrame : public NativeObjec
                                             HandleDebuggerFrame frame,
                                             OnStepHandler* handler);
 
   static MOZ_MUST_USE JS::Result<Completion> eval(
       JSContext* cx, HandleDebuggerFrame frame,
       mozilla::Range<const char16_t> chars, HandleObject bindings,
       const EvalOptions& options);
 
-  enum class MinState {
-    // The frame is guaranteed to have FrameIter::Data.
-    OnStack,
-
-    // The frame is guaranteed to have FrameIter::Data or GeneratorInfo.
-    OnStackOrSuspended,
-
-    // The frame may have FrameIter::Data, GeneratorInfo, or neither.
-    OnStackOrSuspendedOrTerminated,
-  };
-
-  static MOZ_MUST_USE DebuggerFrame* check(JSContext* cx, HandleValue thisv,
-                                           MinState minState);
+  static MOZ_MUST_USE DebuggerFrame* check(JSContext* cx, HandleValue thisv);
 
   bool isOnStack() const;
 
   // Like isOnStack, but works even in the midst of a relocating GC.
   bool isOnStackMaybeForwarded() const;
 
   OnStepHandler* onStepHandler() const;
   OnPopHandler* onPopHandler() const;