diff --git a/js/src/jit-test/tests/debug/Frame-onStep-generator-resumption-01.js b/js/src/jit-test/tests/debug/Frame-onStep-generator-resumption-01.js index d8283decd9ad..ad61ccd9e4a8 100644 --- a/js/src/jit-test/tests/debug/Frame-onStep-generator-resumption-01.js +++ b/js/src/jit-test/tests/debug/Frame-onStep-generator-resumption-01.js @@ -1,4 +1,4 @@ -// The debugger can force an early return from any instruction before the initial yield. +// The debugger can't force return from a generator before the initial yield. let g = newGlobal({newCompartment: true}); g.eval(` @@ -7,36 +7,36 @@ g.eval(` } `); -function test(ttl) { - let dbg = new Debugger(g); - let exiting = false; // we ran out of time-to-live and have forced return - let done = false; // we reached the initial yield without forced return - dbg.onEnterFrame = frame => { - assertEq(frame.callee.name, "f"); - frame.onEnterFrame = undefined; - frame.onStep = () => { - if (ttl == 0) { - exiting = true; - // Forced return here causes the generator object, if any, not - // to be exposed. - return {return: "ponies"}; - } - ttl--; - }; - frame.onPop = completion => { - if (!exiting) - done = true; - }; - }; +let dbg = new Debugger(g); +let steps = 0; +let uncaughtErrorsReported = 0; +dbg.onEnterFrame = frame => { + assertEq(frame.callee.name, "f"); + dbg.onEnterFrame = undefined; + frame.onStep = () => { + steps++; - let result = g.f(); - if (done) - assertEq(result instanceof g.f, true); - else - assertEq(result, "ponies"); + // This test case never resumes the generator after the initial + // yield. Therefore the initial yield has not happened yet. So this + // force-return will be an error. + return {return: "ponies"}; + }; - dbg.enabled = false; - return done; -} + // Having an onPop hook exercises some assertions that don't happen + // otherwise. + frame.onPop = completion => {}; +}; -for (let ttl = 0; !test(ttl); ttl++) {} +dbg.uncaughtExceptionHook = (reason) => { + // When onEnterFrame returns an invalid resumption value, + // the error is reported here. + assertEq(reason instanceof TypeError, true); + uncaughtErrorsReported++; + return undefined; // Cancel the force-return. Let the debuggee continue. +}; + +let result = g.f(); +assertEq(result instanceof g.f, true); + +assertEq(steps > 0, true); +assertEq(uncaughtErrorsReported, steps); diff --git a/js/src/jit-test/tests/debug/Frame-onStep-generator-resumption-02.js b/js/src/jit-test/tests/debug/Frame-onStep-generator-resumption-02.js new file mode 100644 index 000000000000..25f9186c77a5 --- /dev/null +++ b/js/src/jit-test/tests/debug/Frame-onStep-generator-resumption-02.js @@ -0,0 +1,78 @@ +// Like Frame-onStep-generator-resumption-01.js, but bail out by throwing. + +let g = newGlobal({newCompartment: true}); +g.eval(` + function* f() { + yield 1; + } +`); + +// Try force-returning from one of the instructions in `f` before the initial +// yield. In detail: +// +// * This test calls `g.f()` under the Debugger. +// * It uses the Debugger to step `ttl` times. +// If we reach the initial yield before stepping the `ttl`th time, we're done. +// * Otherwise, the test tries to force-return from `f`. +// * That's an error, so the uncaughtExceptionHook is called. +// * The uncaughtExceptionHook returns a `throw` completion value. +// +// Returns `true` if we reached the initial yield, false otherwise. +// +// Note that this function is called in a loop so that every possible relevant +// value of `ttl` is tried once. +function test(ttl) { + let dbg = new Debugger(g); + let exiting = false; // we ran out of time-to-live and have forced return + let done = false; // we reached the initial yield without forced return + let reported = false; // a TypeError was reported. + + dbg.onEnterFrame = frame => { + assertEq(frame.callee.name, "f"); + dbg.onEnterFrame = undefined; + frame.onStep = () => { + if (ttl == 0) { + exiting = true; + // This test case never resumes the generator after the initial + // yield. Therefore the initial yield has not happened yet. So this + // force-return will be an error. + return {return: "ponies"}; + } + ttl--; + }; + frame.onPop = completion => { + if (!exiting) + done = true; + }; + }; + + dbg.uncaughtExceptionHook = (exc) => { + // When onStep returns an invalid resumption value, + // the error is reported here. + assertEq(exc instanceof TypeError, true); + reported = true; + return {throw: "FAIL"}; // Bail out of the test. + }; + + let result; + let caught = undefined; + try { + result = g.f(); + } catch (exc) { + caught = exc; + } + + if (done) { + assertEq(reported, false); + assertEq(result instanceof g.f, true); + assertEq(caught, undefined); + } else { + assertEq(reported, true); + assertEq(caught, "FAIL"); + } + + dbg.enabled = false; + return done; +} + +for (let ttl = 0; !test(ttl); ttl++) {} diff --git a/js/src/jit-test/tests/debug/Frame-onStep-generator-resumption-03.js b/js/src/jit-test/tests/debug/Frame-onStep-generator-resumption-03.js new file mode 100644 index 000000000000..3b2e8d2b3a65 --- /dev/null +++ b/js/src/jit-test/tests/debug/Frame-onStep-generator-resumption-03.js @@ -0,0 +1,49 @@ +// Don't crash on {return:} from onStep in a generator, before the initial suspend. + +// This test tries to force-return from each bytecode instruction in a +// generator, up to the initial suspend. + +load(libdir + "asserts.js"); + +let g = newGlobal({newCompartment: true}); +g.values = [1, 2, 3]; +g.eval(`function* f(arr=values) { yield* arr; }`); + +let dbg = Debugger(g); + +function test(ttl) { + let hits = 0; + dbg.onEnterFrame = frame => { + assertEq(frame.callee.name, "f"); + frame.onStep = () => { + if (--ttl === 0) + return {return: 123}; + }; + }; + dbg.uncaughtExceptionHook = exc => { + return {throw: "debugger error: " + exc}; + }; + + let val = undefined; + let caught = undefined; + try { + val = g.f(); + } catch (exc) { + caught = exc; + } + + if (val === undefined) { + // Tried to force-return before the initial suspend. + assertEq(caught, "debugger error: TypeError: can't force return from a generator before the initial yield"); + assertEq(ttl, 0); + return "pass"; + } else { + // Reached the initial suspend without forcing a return. + assertEq(typeof val, "object"); + assertEq(val instanceof g.f, true); + assertEq(ttl, 1); + return "done"; + } +} + +for (let i = 1; test(i) === "pass"; i++) {} diff --git a/js/src/jit-test/tests/debug/bug-1477084.js b/js/src/jit-test/tests/debug/bug-1477084.js new file mode 100644 index 000000000000..2e45a8cc0598 --- /dev/null +++ b/js/src/jit-test/tests/debug/bug-1477084.js @@ -0,0 +1,30 @@ +// Don't assert trying to force return before the initial yield of an async function. + +var g = newGlobal({newCompartment: true}); +g.parent = this; +g.parentExc = new Error("pants"); +g.eval(` + var dbg = new Debugger; + var pw = dbg.addDebuggee(parent); + var hits = 0; + dbg.onExceptionUnwind = function (frame) { + dbg.onExceptionUnwind = undefined; + return {return: undefined}; + }; + dbg.uncaughtExceptionHook = exc => { + hits++; + assertEq(exc instanceof TypeError, true); + assertEq(/force return.*before the initial yield/.test(exc.message), true); + return {throw: pw.makeDebuggeeValue(parentExc)}; + }; +`); + +async function* method({ x: callbackfn = unresolvableReference }) {} +try { + method(); +} catch (exc) { + g.dbg.enabled = false; + assertEq(exc, g.parentExc); +} + +assertEq(g.hits, 1); diff --git a/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-05.js b/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-05.js index 23295f176340..a0d9887c80fd 100644 --- a/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-05.js +++ b/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-05.js @@ -1,32 +1,41 @@ -// A Debugger can {return:} from the first onEnterFrame for an async generator. -// (The exact behavior is undocumented; we're testing that it doesn't crash.) +// A Debugger can't force-return from the first onEnterFrame for an async generator. ignoreUnhandledRejections(); let g = newGlobal({newCompartment: true}); -g.hit2 = false; g.eval(`async function* f(x) { await x; return "ponies"; }`); let dbg = new Debugger; let gw = dbg.addDebuggee(g); -let hits = 0; +let log = ""; +let completion = undefined; let resumption = undefined; +dbg.uncaughtExceptionHook = exc => { + log += "2"; + assertEq(exc.message, "can't force return from a generator before the initial yield"); + assertEq(exc.constructor, TypeError); + return undefined; // Squelch the error and let the debuggee continue. +}; dbg.onEnterFrame = frame => { if (frame.type == "call" && frame.callee.name === "f") { - frame.onPop = completion => { - assertEq(completion.return, resumption.return); - hits++; + frame.onPop = c => { + // We get here after the uncaughtExcpetionHook fires + // and the debuggee frame has run to the first await. + completion = c; + assertEq(completion.return.class, "AsyncGenerator"); + assertEq(completion.return !== resumption.return, true); + log += "3"; }; - // If we force-return a generator object here, the caller will never - // receive an async generator object. - resumption = frame.eval(`(function* f2() { hit2 = true; })()`); - assertEq(resumption.return.class, "Generator"); + // Try force-returning an actual object of the expected type. + dbg.onEnterFrame = undefined; // don't recurse + resumption = frame.eval('f(0)'); + assertEq(resumption.return.class, "AsyncGenerator"); + log += "1"; return resumption; } }; let it = g.f(0); -assertEq(hits, 1); -assertEq(gw.makeDebuggeeValue(it), resumption.return); -assertEq(g.hit2, false); +assertEq(log, "123"); +assertEq(gw.makeDebuggeeValue(it), completion.return); diff --git a/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-07.js b/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-07.js index 8bcbf830f2dc..578d93ff9904 100644 --- a/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-07.js +++ b/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-07.js @@ -1,54 +1,62 @@ -// A Debugger can {return:} from the first onEnterFrame for an async generator. -// (The exact behavior is undocumented; we're testing that it doesn't crash.) +// A Debugger can't force-return from the first onEnterFrame for an async generator. ignoreUnhandledRejections(); let g = newGlobal({newCompartment: true}); g.eval(`async function* f(x) { await x; return "ponies"; }`); -g.eval(`async function* f2(x) { await x; return "moar ponies"; }`); let dbg = new Debugger; +dbg.uncaughtExceptionHook = exc => {}; // ignore errors let gw = dbg.addDebuggee(g); let hits = 0; let resumption = undefined; + +dbg.onEnterFrame = frame => { + dbg.onEnterFrame = undefined; + assertEq(frame.type, "call"); + assertEq(frame.callee.name, "f"); + frame.onPop = completion => { + hits++; + }; + + // Try to force-return. It's too early. This results in a call to the + // uncaughtExceptionHook but is otherwise ignored. + return {return: "rainbows"}; +}; +let it = g.f(0); // onPop #1: the initial yield +assertEq(hits, 1); +let p = it.next(); // onPop #2: await x +assertEq(hits, 2); +drainJobQueue(); // onPop #3: return "ponies", #4: the final yield +assertEq(hits, 4); + +let pw = gw.makeDebuggeeValue(p); +assertEq(pw.isPromise, true); +assertEq(pw.promiseState, "fulfilled"); +assertEq(pw.promiseValue.getProperty("value").return, "ponies"); +assertEq(pw.promiseValue.getProperty("done").return, true); + +// ---- + +g.eval(`async function* f2(x) { await x; return "moar ponies"; }`); + let savedAsyncGen = undefined; dbg.onEnterFrame = frame => { - if (frame.type == "call" && frame.callee.name === "f2") { - frame.onPop = completion => { - if (savedAsyncGen === undefined) { - savedAsyncGen = completion.return; - } - }; - } - if (frame.type == "call" && frame.callee.name === "f") { - frame.onPop = completion => { - hits++; - }; - - return {return: savedAsyncGen}; - } + dbg.onEnterFrame = undefined; + assertEq(frame.type, "call"); + assertEq(frame.callee.name, "f2"); + frame.onPop = completion => { + if (savedAsyncGen === undefined) { + savedAsyncGen = completion.return; + } + }; }; - let it2 = g.f2(123); -let it = g.f(0); - let p2 = it2.next(); -let p = it.next(); - -assertEq(hits, 1); - drainJobQueue(); -assertEq(hits, 1); - let pw2 = gw.makeDebuggeeValue(p2); assertEq(pw2.isPromise, true); assertEq(pw2.promiseState, "fulfilled"); assertEq(pw2.promiseValue.getProperty("value").return, "moar ponies"); assertEq(pw2.promiseValue.getProperty("done").return, true); - -let pw = gw.makeDebuggeeValue(p); -assertEq(pw.isPromise, true); -assertEq(pw.promiseState, "fulfilled"); -assertEq(pw.promiseValue.getProperty("value").return, undefined); -assertEq(pw.promiseValue.getProperty("done").return, true); diff --git a/js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-05.js b/js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-05.js index 921af6223f08..f1f5a3860a10 100644 --- a/js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-05.js +++ b/js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-05.js @@ -1,5 +1,4 @@ -// {return:} from the initial onEnterFrame for a generator replaces the -// generator object that's normally returned to the caller. +// {return:} from the initial onEnterFrame for a generator is an error. load(libdir + "asserts.js"); @@ -15,5 +14,9 @@ dbg.onEnterFrame = frame => { hits++; return {return: 123}; }; -assertEq(g.f(), 123); +dbg.uncaughtExceptionHook = exc => { + assertEq(exc instanceof TypeError, true); + return {throw: "REJECTED"}; +} +assertThrowsValue(g.f, "REJECTED"); assertEq(hits, 1); diff --git a/js/src/jit-test/tests/debug/onStep-generator-resumption-01.js b/js/src/jit-test/tests/debug/onStep-generator-resumption-01.js deleted file mode 100644 index 7b86b497b43a..000000000000 --- a/js/src/jit-test/tests/debug/onStep-generator-resumption-01.js +++ /dev/null @@ -1,36 +0,0 @@ -// Don't crash on {return:} from onStep in a generator, before the initial suspend. - -// This test forces a return from each bytecode instruction in a generator, up -// to the initial suspend. - -load(libdir + "asserts.js"); - -let g = newGlobal({newCompartment: true}); -g.values = [1, 2, 3]; -g.eval(`function* f(arr=values) { yield* arr; }`); - -let dbg = Debugger(g); - -function test(ttl) { - let hits = 0; - dbg.onEnterFrame = frame => { - assertEq(frame.callee.name, "f"); - frame.onStep = () => { - if (--ttl === 0) - return {return: 123}; - }; - }; - let val = g.f(); - if (typeof val === "object") { - // Reached the initial suspend without forcing a return. - assertEq(ttl, 1); - return "done"; - } - - // Forced a return before the initial suspend. - assertEq(val, 123); - assertEq(ttl, 0); - return "pass"; -} - -for (let i = 1; test(i) === "pass"; i++) {} diff --git a/js/src/js.msg b/js/src/js.msg index 401df785ad17..c7407cc75dca 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -488,6 +488,7 @@ MSG_DEF(JSMSG_DEBUG_PROTO, 2, JSEXN_TYPEERR, "{0}.prototype is not a MSG_DEF(JSMSG_DEBUG_WRONG_OWNER, 1, JSEXN_TYPEERR, "{0} belongs to a different Debugger") MSG_DEF(JSMSG_DEBUG_OPTIMIZED_OUT, 1, JSEXN_ERR, "variable `{0}' has been optimized out") MSG_DEF(JSMSG_DEBUG_OPTIMIZED_OUT_FUN, 0, JSEXN_ERR, "function is optimized out") +MSG_DEF(JSMSG_DEBUG_FORCED_RETURN_DISALLOWED, 0, JSEXN_TYPEERR, "can't force return from a generator before the initial yield") MSG_DEF(JSMSG_DEBUG_RESUMPTION_VALUE_DISALLOWED, 0, JSEXN_TYPEERR, "resumption values are disallowed in this hook") MSG_DEF(JSMSG_DEBUG_VARIABLE_NOT_FOUND,0, JSEXN_TYPEERR, "variable not found in environment") MSG_DEF(JSMSG_DEBUG_WRAPPER_IN_WAY, 3, JSEXN_TYPEERR, "{0} is {1}{2}a global object, but a direct reference is required") diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index 8ab1bc02141b..7ba49197f7da 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -963,7 +963,8 @@ class MOZ_RAII AutoSetGeneratorRunning { asyncGenState_(static_cast(0)), genObj_(cx, genObj) { if (genObj) { - if (!genObj->isClosed() && genObj->isSuspended()) { + if (!genObj->isClosed() && !genObj->isBeforeInitialYield() && + genObj->isSuspended()) { // Yielding or awaiting. resumeIndex_ = genObj->resumeIndex(); genObj->setRunning(); @@ -1577,6 +1578,7 @@ static bool CheckResumptionValue(JSContext* cx, AbstractFramePtr frame, if (maybeThisv.isSome()) { const HandleValue& thisv = maybeThisv.ref(); if (resumeMode == ResumeMode::Return && vp.isPrimitive()) { + // Forcing return from a class constructor. There are rules. if (vp.isUndefined()) { if (thisv.isMagic(JS_UNINITIALIZED_LEXICAL)) { return ThrowUninitializedThis(cx, frame); @@ -1590,9 +1592,34 @@ static bool CheckResumptionValue(JSContext* cx, AbstractFramePtr frame, } } } + + // Check for forcing return from a generator before the initial yield. This + // is not supported because some engine-internal code assumes a call to a + // generator will return a GeneratorObject; see bug 1477084. + if (resumeMode == ResumeMode::Return && frame && frame.isFunctionFrame() && + frame.callee()->isGenerator()) { + Rooted genObj(cx); + { + AutoRealm ar(cx, frame.callee()); + genObj = GetGeneratorObjectForFrame(cx, frame); + } + + if (!genObj || genObj->isBeforeInitialYield()) { + JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, + JSMSG_DEBUG_FORCED_RETURN_DISALLOWED); + return false; + } + } + return true; } +// Last-minute sanity adjustments to resumption. +// +// This is called last, as we leave the debugger. It must happen outside the +// control of the uncaughtExceptionHook, because this code assumes we won't +// change our minds and continue execution--we must not close the generator +// object unless we're really going to force-return. static void AdjustGeneratorResumptionValue(JSContext* cx, AbstractFramePtr frame, ResumeMode& resumeMode, @@ -1612,10 +1639,11 @@ static void AdjustGeneratorResumptionValue(JSContext* cx, }; // Treat `{return: }` like a `return` statement. Simulate what the - // debuggee would do for an ordinary `return` statement--using a few bytecode - // instructions--and it's simpler to do the work manually than to count on - // that bytecode sequence existing in the debuggee, somehow jump to it, and - // then avoid re-entering the debugger from it. + // debuggee would do for an ordinary `return` statement, using a few bytecode + // instructions. It's simpler to do the work manually than to count on that + // bytecode sequence existing in the debuggee, somehow jump to it, and then + // avoid re-entering the debugger from it. + // // Similarly treat `{throw: }` like a `throw` statement. if (frame.callee()->isGenerator()) { // Throw doesn't require any special processing for (async) generators. @@ -1623,30 +1651,34 @@ static void AdjustGeneratorResumptionValue(JSContext* cx, return; } - // For (async) generators, that means doing the work below. + // Forcing return from a (possibly async) generator. Rooted genObj( cx, GetGeneratorObjectForFrame(cx, frame)); - if (genObj) { - // 1. `return ` creates and returns a new object in non-async - // generators, `{value: , done: true}`. - if (!frame.callee()->isAsync() && !genObj->isBeforeInitialYield()) { - JSObject* pair = CreateIterResultObject(cx, vp, true); - if (!pair) { - getAndClearExceptionThenThrow(); - return; - } - vp.setObject(*pair); - } - // 2. The generator must be closed. - genObj->setClosed(); - } else { - // We're before the initial yield. Carry on with the forced return. - // The debuggee will see a call to a generator returning the - // non-generator value *vp. + // We already went through CheckResumptionValue, which would have replaced + // this invalid resumption value with an error if we were trying to force + // return before the initial yield. + MOZ_RELEASE_ASSERT(genObj && !genObj->isBeforeInitialYield()); + + // 1. `return ` creates and returns a new object, + // `{value: , done: true}`. + // + // For non-async generators, the iterator result object is created in + // bytecode, so we have to simulate that here. For async generators, our + // C++ implementation of AsyncGeneratorResolve will do this. So don't do it + // twice: + if (!frame.callee()->isAsync()) { + JSObject* pair = CreateIterResultObject(cx, vp, true); + if (!pair) { + getAndClearExceptionThenThrow(); + return; + } + vp.setObject(*pair); } + + // 2. The generator must be closed. + genObj->setClosed(); } else if (frame.callee()->isAsync()) { - // For async functions, that means doing the work below. if (AbstractGeneratorObject* genObj = GetGeneratorObjectForFrame(cx, frame)) { // Throw doesn't require any special processing for async functions when