Bug 1114036 - Rewrite Debugger::parseResumptionValue to be more like PropDesc::initialize. r=jimb.
authorJason Orendorff <jorendorff@mozilla.com>
Sat, 20 Dec 2014 08:53:40 -0600
changeset 239112 af81a26dae604b133805465193d647cc86a49910
parent 239111 d7b40d5a4577f965c6fe1bc7f99dc9321dccdf57
child 239113 7a7a248c492f7ab9b424e92eff84df52f089755b
push id487
push userbcampen@mozilla.com
push dateMon, 26 Jan 2015 23:32:56 +0000
reviewersjimb
bugs1114036
milestone38.0a1
Bug 1114036 - Rewrite Debugger::parseResumptionValue to be more like PropDesc::initialize. r=jimb.
js/src/jit-test/tests/debug/resumption-error-01.js
js/src/jit-test/tests/debug/resumption-error-02.js
js/src/vm/Debugger.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/resumption-error-01.js
@@ -0,0 +1,7 @@
+// A resumption value can't have both {return:} and {throw:} properties.
+
+var g = newGlobal();
+var dbg = Debugger(g);
+dbg.onDebuggerStatement = stack => ({return: 1, throw: 2});
+dbg.uncaughtExceptionHook = exc => ({return: "corrected"});
+assertEq(g.eval("debugger; false;"), "corrected");
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/resumption-error-02.js
@@ -0,0 +1,16 @@
+// Error handling if parsing a resumption value throws.
+
+var g = newGlobal();
+var dbg = Debugger(g);
+var rv;
+dbg.onDebuggerStatement = stack => rv;
+dbg.uncaughtExceptionHook = function (exc) {
+    assertEq(exc, "BANG");
+    return {return: "recovered"};
+};
+
+rv = {get throw() { throw "BANG"; }};
+assertEq(g.eval("debugger; false;"), "recovered");
+
+rv = new Proxy({}, {has() { throw "BANG"; }});
+assertEq(g.eval("debugger; false;"), "recovered");
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -1079,69 +1079,86 @@ Debugger::receiveCompletionValue(Maybe<A
 
     JSTrapStatus status;
     RootedValue value(cx);
     resultToCompletion(cx, ok, val, &status, &value);
     ac.reset();
     return newCompletionValue(cx, status, value, vp);
 }
 
+static bool
+GetStatusProperty(JSContext *cx, HandleObject obj, PropertyName *name, JSTrapStatus status,
+                  JSTrapStatus *statusOut, MutableHandleValue vp, int *hits)
+{
+    bool found;
+    if (!HasProperty(cx, obj, name, &found))
+        return false;
+    if (found) {
+        ++*hits;
+        *statusOut = status;
+        if (!GetProperty(cx, obj, obj, name, vp))
+            return false;
+    }
+    return true;
+}
+
+static bool
+ParseResumptionValueAsObject(JSContext *cx, HandleValue rv, JSTrapStatus *statusp,
+                             MutableHandleValue vp)
+{
+    int hits = 0;
+    if (rv.isObject()) {
+        RootedObject obj(cx, &rv.toObject());
+        if (!GetStatusProperty(cx, obj, cx->names().return_, JSTRAP_RETURN, statusp, vp, &hits))
+            return false;
+        if (!GetStatusProperty(cx, obj, cx->names().throw_, JSTRAP_THROW, statusp, vp, &hits))
+            return false;
+    }
+
+    if (hits != 1) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_DEBUG_BAD_RESUMPTION);
+        return false;
+    }
+    return true;
+}
+
 JSTrapStatus
 Debugger::parseResumptionValue(Maybe<AutoCompartment> &ac, bool ok, const Value &rv, MutableHandleValue vp,
                                bool callHook)
 {
     vp.setUndefined();
     if (!ok)
         return handleUncaughtException(ac, vp, callHook);
     if (rv.isUndefined()) {
         ac.reset();
         return JSTRAP_CONTINUE;
     }
     if (rv.isNull()) {
         ac.reset();
         return JSTRAP_ERROR;
     }
 
-    /* Check that rv is {return: val} or {throw: val}. */
     JSContext *cx = ac->context()->asJSContext();
-    Rooted<JSObject*> obj(cx);
-    RootedShape shape(cx);
-    RootedId returnId(cx, NameToId(cx->names().return_));
-    RootedId throwId(cx, NameToId(cx->names().throw_));
-    bool okResumption = rv.isObject();
-    if (okResumption) {
-        obj = &rv.toObject();
-        okResumption = obj->is<PlainObject>();
-    }
-    if (okResumption) {
-        shape = obj->lastProperty();
-        okResumption = shape->previous() &&
-             !shape->previous()->previous() &&
-             (shape->propid() == returnId || shape->propid() == throwId) &&
-             shape->isDataDescriptor();
-    }
-    if (!okResumption) {
-        JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_DEBUG_BAD_RESUMPTION);
+    JSTrapStatus status = JSTRAP_CONTINUE;
+    RootedValue v(cx);
+    RootedValue rvRoot(cx, rv);
+    if (!ParseResumptionValueAsObject(cx, rvRoot, &status, &v) ||
+        !unwrapDebuggeeValue(cx, &v))
+    {
         return handleUncaughtException(ac, vp, callHook);
     }
 
-    HandleNativeObject nobj = obj.as<NativeObject>();
-
-    RootedValue v(cx, vp.get());
-    if (!NativeGetExistingProperty(cx, obj, nobj, shape, &v) || !unwrapDebuggeeValue(cx, &v))
-        return handleUncaughtException(ac, &v, callHook);
-
     ac.reset();
     if (!cx->compartment()->wrap(cx, &v)) {
         vp.setUndefined();
         return JSTRAP_ERROR;
     }
     vp.set(v);
 
-    return shape->propid() == returnId ? JSTRAP_RETURN : JSTRAP_THROW;
+    return status;
 }
 
 static bool
 CallMethodIfPresent(JSContext *cx, HandleObject obj, const char *name, int argc, Value *argv,
                     MutableHandleValue rval)
 {
     rval.setUndefined();
     JSAtom *atom = Atomize(cx, name, strlen(name));