Bug 1317824 - Check debugger resumption value of generator and async function. r=jimb, a=jcristau
authorTooru Fujisawa <arai_a@mac.com>
Thu, 17 Nov 2016 14:19:51 +0900
changeset 352525 1d71e28b8a2de1b47c024c01c5fe9f9c7703c13f
parent 352524 7561e199d0f47c3e164d519e3e2989c98212e382
child 352526 bc93261f3106f1423872fd20fff63e09c4a71f61
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)
reviewersjimb, jcristau
bugs1317824
milestone52.0a2
Bug 1317824 - Check debugger resumption value of generator and async function. r=jimb, a=jcristau
js/src/doc/Debugger/Conventions.md
js/src/jit-test/tests/debug/onExceptionUnwind-resumption-async.js
js/src/jit-test/tests/debug/onExceptionUnwind-resumption-generator.js
js/src/jit-test/tests/debug/resumption-06.js
js/src/js.msg
js/src/vm/AsyncFunction.cpp
js/src/vm/AsyncFunction.h
js/src/vm/Debugger.cpp
js/src/vm/GeneratorObject.cpp
js/src/vm/GeneratorObject.h
--- a/js/src/doc/Debugger/Conventions.md
+++ b/js/src/doc/Debugger/Conventions.md
@@ -105,16 +105,20 @@ resumption value has one of the followin
     <i>Value</i> must be a debuggee value. (Most handler functions support
     this, except those whose descriptions say otherwise.) If the function
     was called as a constructor (that is, via a `new` expression), then
     <i>value</i> serves as the value returned by the function's body, not
     that produced by the `new` expression: if the value is not an object,
     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
     instruction. <i>Value</i> must be a debuggee value.
 
 `null`
 :   Terminate the debuggee, as if it had been cancelled by the "slow script"
     dialog box.
new file mode 100644
--- /dev/null
+++ b/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);
new file mode 100644
--- /dev/null
+++ b/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);
--- a/js/src/jit-test/tests/debug/resumption-06.js
+++ b/js/src/jit-test/tests/debug/resumption-06.js
@@ -2,20 +2,20 @@
 // Forced return from a star generator frame.
 
 load(libdir + 'asserts.js')
 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';
     debugger;  // Force return here. The value is ignored.
     yield '2';
 }
 var iter = gen();
 assertIteratorNext(iter, '1');
-assertEq(iter.next(), '!');
+assertIteratorDone(iter, '!');
 iter.next();
 assertEq(0, 1);
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -425,20 +425,22 @@ MSG_DEF(JSMSG_SC_DUP_TRANSFERABLE,     0
 MSG_DEF(JSMSG_SC_NOT_TRANSFERABLE,     0, JSEXN_TYPEERR, "invalid transferable array for structured clone")
 MSG_DEF(JSMSG_SC_UNSUPPORTED_TYPE,     0, JSEXN_TYPEERR, "unsupported type for structured data")
 MSG_DEF(JSMSG_SC_NOT_CLONABLE,         1, JSEXN_TYPEERR, "{0} cannot be cloned in this context")
 MSG_DEF(JSMSG_SC_SAB_TRANSFER,         0, JSEXN_WARN, "SharedArrayBuffer must not be in the transfer list")
 MSG_DEF(JSMSG_SC_SAB_DISABLED,         0, JSEXN_TYPEERR, "SharedArrayBuffer not cloned - shared memory disabled in receiver")
 
 // 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")
 MSG_DEF(JSMSG_DEBUG_LOOP,              0, JSEXN_TYPEERR, "cannot debug an object in same compartment as debugger or a compartment that is already debugging the debugger")
 MSG_DEF(JSMSG_DEBUG_NOT_DEBUGGEE,      2, JSEXN_ERR, "{0} is not a debuggee {1}")
 MSG_DEF(JSMSG_DEBUG_NOT_DEBUGGING,     0, JSEXN_ERR, "can't set breakpoint: script global is not a debuggee")
 MSG_DEF(JSMSG_DEBUG_NOT_IDLE,          0, JSEXN_ERR, "can't start debugging: a debuggee script is on the stack")
 MSG_DEF(JSMSG_DEBUG_NOT_LIVE,          1, JSEXN_ERR, "{0} is not live")
--- a/js/src/vm/AsyncFunction.cpp
+++ b/js/src/vm/AsyncFunction.cpp
@@ -4,16 +4,17 @@
  * 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/. */
 
 #include "vm/AsyncFunction.h"
 
 #include "jscompartment.h"
 
 #include "builtin/Promise.h"
+#include "vm/GeneratorObject.h"
 #include "vm/GlobalObject.h"
 #include "vm/Interpreter.h"
 #include "vm/SelfHosting.h"
 
 using namespace js;
 using namespace js::gc;
 
 /* static */ bool
@@ -227,8 +228,13 @@ js::GetUnwrappedAsyncFunction(JSFunction
 }
 
 bool
 js::IsWrappedAsyncFunction(JSFunction* fun)
 {
     return fun->maybeNative() == WrappedAsyncFunction;
 }
 
+MOZ_MUST_USE bool
+js::CheckAsyncResumptionValue(JSContext* cx, HandleValue v)
+{
+    return CheckStarGeneratorResumptionValue(cx, v);
+}
--- a/js/src/vm/AsyncFunction.h
+++ b/js/src/vm/AsyncFunction.h
@@ -27,11 +27,14 @@ WrapAsyncFunction(JSContext* cx, HandleF
 MOZ_MUST_USE bool
 AsyncFunctionAwaitedFulfilled(JSContext* cx, Handle<PromiseObject*> resultPromise,
                               HandleValue generatorVal, HandleValue value);
 
 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 */
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -27,17 +27,19 @@
 #include "jit/BaselineDebugModeOSR.h"
 #include "jit/BaselineJIT.h"
 #include "js/Date.h"
 #include "js/GCAPI.h"
 #include "js/UbiNodeBreadthFirst.h"
 #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"
 #include "wasm/WasmInstance.h"
 
 #include "jsgcinlines.h"
 #include "jsobjinlines.h"
 #include "jsopcodeinlines.h"
@@ -1552,16 +1554,34 @@ ParseResumptionValue(JSContext* cx, Hand
     }
     return ParseResumptionValueAsObject(cx, rval, statusp, vp);
 }
 
 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()) {
             if (vp.isUndefined()) {
                 if (thisv.isMagic(JS_UNINITIALIZED_LEXICAL))
                     return ThrowUninitializedThis(cx, frame);
 
                 vp.set(thisv);
--- a/js/src/vm/GeneratorObject.cpp
+++ b/js/src/vm/GeneratorObject.cpp
@@ -1,16 +1,18 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
  * 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/. */
 
 #include "vm/GeneratorObject.h"
 
+#include "jsobj.h"
+
 #include "jsatominlines.h"
 #include "jsscriptinlines.h"
 
 #include "vm/NativeObject-inl.h"
 #include "vm/Stack-inl.h"
 
 using namespace js;
 
@@ -329,8 +331,37 @@ GlobalObject::initStarGenerators(JSConte
     if (!LinkConstructorAndPrototype(cx, genFunction, genFunctionProto))
         return false;
 
     global->setReservedSlot(STAR_GENERATOR_OBJECT_PROTO, ObjectValue(*genObjectProto));
     global->setReservedSlot(STAR_GENERATOR_FUNCTION, ObjectValue(*genFunction));
     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;
+}
--- a/js/src/vm/GeneratorObject.h
+++ b/js/src/vm/GeneratorObject.h
@@ -211,16 +211,19 @@ class StarGeneratorObject : public Gener
   public:
     static const Class class_;
 };
 
 bool GeneratorThrowOrClose(JSContext* cx, AbstractFramePtr frame, Handle<GeneratorObject*> obj,
                            HandleValue val, uint32_t resumeKind);
 void SetReturnValueForClosingGenerator(JSContext* cx, AbstractFramePtr frame);
 
+MOZ_MUST_USE bool
+CheckStarGeneratorResumptionValue(JSContext* cx, HandleValue v);
+
 } // namespace js
 
 template<>
 inline bool
 JSObject::is<js::GeneratorObject>() const
 {
     return is<js::LegacyGeneratorObject>() || is<js::StarGeneratorObject>();
 }