Bug 1317824 - Check debugger resumption value of generator and async function. r=jimb

js/src/doc/Debugger/Conventions.md

@@ -110,6 +110,10 @@ resumption value has one of the following forms:
     the `new` expression returns the frame's `this` value. Similarly, if
     the function is the constructor for a subclass, then a non-object
     value may result in a TypeError.
+    If the frame is a generator or async function, then <i>value</i> must
+    conform to the iterator protocol: it must be a non-proxy object of the form
+    <code>{ done: <i>boolean</i>, value: <i>v</i> }</code>, where
+    both `done` and `value` are ordinary properties.
 
 <code>{ throw: <i>value</i> }</code>
 :   Throw <i>value</i> as an exception from the current bytecode

js/src/jit-test/tests/debug/onExceptionUnwind-resumption-async.js

@@ -0,0 +1,130 @@
+load(libdir + "asserts.js");
+
+var g = newGlobal();
+var dbg = Debugger(g);
+
+g.eval(`
+async function f() {
+    return e;
+}
+`);
+
+// To continue testing after uncaught exception, remember the exception and
+// return normal completeion.
+var currentFrame;
+var uncaughtException;
+dbg.uncaughtExceptionHook = function(e) {
+    uncaughtException = e;
+    return {
+        return: currentFrame.eval("({ done: true, value: 'uncaught' })").return
+    };
+};
+function testUncaughtException() {
+    uncaughtException = undefined;
+    var val = g.eval(`
+var val;
+f().then(v => { val = v });
+drainJobQueue();
+val;
+`);
+    assertEq(val, "uncaught");
+    assertEq(uncaughtException instanceof TypeError, true);
+}
+
+// Just continue
+dbg.onExceptionUnwind = function(frame) {
+    return undefined;
+};
+g.eval(`
+var E;
+f().catch(e => { exc = e });
+drainJobQueue();
+assertEq(exc instanceof ReferenceError, true);
+`);
+
+// Should return object.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: "foo"
+    };
+};
+testUncaughtException();
+
+// The object should have `done` property and `value` property.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("({})").return
+    };
+};
+testUncaughtException();
+
+// The object should have `done` property.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("({ value: 10 })").return
+    };
+};
+testUncaughtException();
+
+// The object should have `value` property.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("({ done: true })").return
+    };
+};
+testUncaughtException();
+
+// `done` property should be a boolean value.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("({ done: 10, value: 10 })").return
+    };
+};
+testUncaughtException();
+
+// `done` property shouldn't be an accessor.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("({ get done() { return true; }, value: 10 })").return
+    };
+};
+testUncaughtException();
+
+// `value` property shouldn't be an accessor.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("({ done: true, get value() { return 10; } })").return
+    };
+};
+testUncaughtException();
+
+// The object shouldn't be a Proxy.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("new Proxy({ done: true, value: 10 }, {})").return
+    };
+};
+testUncaughtException();
+
+// Correct resumption value.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("({ done: true, value: 10 })").return
+    };
+};
+var val = g.eval(`
+var val;
+f().then(v => { val = v });
+drainJobQueue();
+val;
+`);
+assertEq(val, 10);

js/src/jit-test/tests/debug/onExceptionUnwind-resumption-generator.js

@@ -0,0 +1,117 @@
+load(libdir + "asserts.js");
+
+var g = newGlobal();
+var dbg = Debugger(g);
+
+g.eval(`
+function* f() {
+    e;
+}
+`);
+
+// To continue testing after uncaught exception, remember the exception and
+// return normal completeion.
+var currentFrame;
+var uncaughtException;
+dbg.uncaughtExceptionHook = function(e) {
+    uncaughtException = e;
+    return {
+        return: currentFrame.eval("({ done: true, value: 'uncaught' })").return
+    };
+};
+function testUncaughtException() {
+    uncaughtException = undefined;
+    var obj = g.eval(`f().next()`);
+    assertEq(obj.done, true);
+    assertEq(obj.value, 'uncaught');
+    assertEq(uncaughtException instanceof TypeError, true);
+}
+
+// Just continue
+dbg.onExceptionUnwind = function(frame) {
+    return undefined;
+};
+assertThrowsInstanceOf(() => g.eval(`f().next();`), g.ReferenceError);
+
+// Should return object.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: "foo"
+    };
+};
+testUncaughtException();
+
+// The object should have `done` property and `value` property.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("({})").return
+    };
+};
+testUncaughtException();
+
+// The object should have `done` property.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("({ value: 10 })").return
+    };
+};
+testUncaughtException();
+
+// The object should have `value` property.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("({ done: true })").return
+    };
+};
+testUncaughtException();
+
+// `done` property should be a boolean value.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("({ done: 10, value: 10 })").return
+    };
+};
+testUncaughtException();
+
+// `done` property shouldn't be an accessor.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("({ get done() { return true; }, value: 10 })").return
+    };
+};
+testUncaughtException();
+
+// `value` property shouldn't be an accessor.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("({ done: true, get value() { return 10; } })").return
+    };
+};
+testUncaughtException();
+
+// The object shouldn't be a Proxy.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("new Proxy({ done: true, value: 10 }, {})").return
+    };
+};
+testUncaughtException();
+
+// Correct resumption value.
+dbg.onExceptionUnwind = function(frame) {
+    currentFrame = frame;
+    return {
+        return: frame.eval("({ done: true, value: 10 })").return
+    };
+};
+var obj = g.eval(`f().next()`);
+assertEq(obj.done, true);
+assertEq(obj.value, 10);

js/src/jit-test/tests/debug/resumption-06.js

@@ -7,7 +7,7 @@ load(libdir + 'iteration.js')
 var g = newGlobal();
 g.debuggeeGlobal = this;
 g.eval("var dbg = new Debugger(debuggeeGlobal);" +
-       "dbg.onDebuggerStatement = function () { return {return: '!'}; };");
+       "dbg.onDebuggerStatement = function (frame) { return { return: frame.eval(\"({ done: true, value: '!' })\").return }; };");
 
 function* gen() {
     yield '1';
@@ -16,6 +16,6 @@ function* gen() {
 }
 var iter = gen();
 assertIteratorNext(iter, '1');
-assertEq(iter.next(), '!');
+assertIteratorDone(iter, '!');
 iter.next();
 assertEq(0, 1);

js/src/js.msg

@@ -432,10 +432,12 @@ MSG_DEF(JSMSG_SC_SAB_DISABLED,         0, JSEXN_TYPEERR, "SharedArrayBuffer not
 
 // Debugger
 MSG_DEF(JSMSG_ASSIGN_FUNCTION_OR_NULL, 1, JSEXN_TYPEERR, "value assigned to {0} must be a function or null")
+MSG_DEF(JSMSG_DEBUG_BAD_AWAIT,         0, JSEXN_TYPEERR, "await expression received invalid value")
 MSG_DEF(JSMSG_DEBUG_BAD_LINE,          0, JSEXN_TYPEERR, "invalid line number")
 MSG_DEF(JSMSG_DEBUG_BAD_OFFSET,        0, JSEXN_TYPEERR, "invalid script offset")
 MSG_DEF(JSMSG_DEBUG_BAD_REFERENT,      2, JSEXN_TYPEERR, "{0} does not refer to {1}")
 MSG_DEF(JSMSG_DEBUG_BAD_RESUMPTION,    0, JSEXN_TYPEERR, "debugger resumption value must be undefined, {throw: val}, {return: val}, or null")
+MSG_DEF(JSMSG_DEBUG_BAD_YIELD,         0, JSEXN_TYPEERR, "generator yielded invalid value")
 MSG_DEF(JSMSG_DEBUG_CANT_DEBUG_GLOBAL, 0, JSEXN_TYPEERR, "passing non-debuggable global to addDebuggee")
 MSG_DEF(JSMSG_DEBUG_CCW_REQUIRED,      1, JSEXN_TYPEERR, "{0}: argument must be an object from a different compartment")
 MSG_DEF(JSMSG_DEBUG_COMPARTMENT_MISMATCH, 2, JSEXN_TYPEERR, "{0}: descriptor .{1} property is an object in a different compartment than the target object")

js/src/vm/AsyncFunction.cpp

@@ -9,6 +9,7 @@
 #include "jscompartment.h"
 
 #include "builtin/Promise.h"
+#include "vm/GeneratorObject.h"
 #include "vm/GlobalObject.h"
 #include "vm/Interpreter.h"
 #include "vm/SelfHosting.h"
@@ -232,3 +233,8 @@ js::IsWrappedAsyncFunction(JSFunction* fun)
     return fun->maybeNative() == WrappedAsyncFunction;
 }
 
+MOZ_MUST_USE bool
+js::CheckAsyncResumptionValue(JSContext* cx, HandleValue v)
+{
+    return CheckStarGeneratorResumptionValue(cx, v);
+}

js/src/vm/AsyncFunction.h

@@ -32,6 +32,9 @@ MOZ_MUST_USE bool
 AsyncFunctionAwaitedRejected(JSContext* cx, Handle<PromiseObject*> resultPromise,
                              HandleValue generatorVal, HandleValue reason);
 
+MOZ_MUST_USE bool
+CheckAsyncResumptionValue(JSContext* cx, HandleValue v);
+
 } // namespace js
 
 #endif /* vm_AsyncFunction_h */

js/src/vm/Debugger.cpp

@@ -32,7 +32,9 @@
 #include "js/Vector.h"
 #include "proxy/ScriptedProxyHandler.h"
 #include "vm/ArgumentsObject.h"
+#include "vm/AsyncFunction.h"
 #include "vm/DebuggerMemory.h"
+#include "vm/GeneratorObject.h"
 #include "vm/SPSProfiler.h"
 #include "vm/TraceLogging.h"
 #include "vm/WrapperObject.h"
@@ -1557,6 +1559,24 @@ static bool
 CheckResumptionValue(JSContext* cx, AbstractFramePtr frame, const Maybe<HandleValue>& maybeThisv,
                      JSTrapStatus status, MutableHandleValue vp)
 {
+    if (status == JSTRAP_RETURN && frame && frame.isFunctionFrame()) {
+        // Don't let a { return: ... } resumption value make a generator or
+        // async function violate the iterator protocol. The return value from
+        // such a frame must have the form { done: <bool>, value: <anything> }.
+        RootedFunction callee(cx, frame.callee());
+        if (callee->isAsync()) {
+            if (!CheckAsyncResumptionValue(cx, vp)) {
+                JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_DEBUG_BAD_AWAIT);
+                return false;
+            }
+        } else if (callee->isStarGenerator()) {
+            if (!CheckStarGeneratorResumptionValue(cx, vp)) {
+                JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_DEBUG_BAD_YIELD);
+                return false;
+            }
+        }
+    }
+
     if (maybeThisv.isSome()) {
         const HandleValue& thisv = maybeThisv.ref();
         if (status == JSTRAP_RETURN && vp.isPrimitive()) {

js/src/vm/GeneratorObject.cpp

@@ -6,6 +6,8 @@
 
 #include "vm/GeneratorObject.h"
 
+#include "jsobj.h"
+
 #include "jsatominlines.h"
 #include "jsscriptinlines.h"
 
@@ -334,3 +336,32 @@ GlobalObject::initStarGenerators(JSContext* cx, Handle<GlobalObject*> global)
     global->setReservedSlot(STAR_GENERATOR_FUNCTION_PROTO, ObjectValue(*genFunctionProto));
     return true;
 }
+
+MOZ_MUST_USE bool
+js::CheckStarGeneratorResumptionValue(JSContext* cx, HandleValue v)
+{
+    // yield/return value should be an Object.
+    if (!v.isObject())
+        return false;
+
+    JSObject* obj = &v.toObject();
+
+    // It should have `done` data property with boolean value.
+    Value doneVal;
+    if (!GetPropertyPure(cx, obj, NameToId(cx->names().done), &doneVal))
+        return false;
+    if (!doneVal.isBoolean())
+        return false;
+
+    // It should have `value` data property, but the type doesn't matter
+    JSObject* ignored;
+    Shape* shape;
+    if (!LookupPropertyPure(cx, obj, NameToId(cx->names().value), &ignored, &shape))
+        return false;
+    if (!shape)
+        return false;
+    if (!shape->hasDefaultGetter())
+        return false;
+
+    return true;
+}

js/src/vm/GeneratorObject.h

@@ -216,6 +216,9 @@ bool GeneratorThrowOrClose(JSContext* cx, AbstractFramePtr frame, Handle<Generat
                            HandleValue val, uint32_t resumeKind);
 void SetReturnValueForClosingGenerator(JSContext* cx, AbstractFramePtr frame);
 
+MOZ_MUST_USE bool
+CheckStarGeneratorResumptionValue(JSContext* cx, HandleValue v);
+
 } // namespace js
 
 template<>