Bug 890576 - Disallow resumption values in onNewGlobalObject hooks. r=jimb
authorBobby Holley <bobbyholley@gmail.com>
Thu, 01 Aug 2013 18:38:44 -0700
changeset 153338 71fd6f8348c47fe641919181222a56fa6638dec8
parent 153337 5603000cc1bbdf22a67bbb2a2cf879bc4221211c
child 153339 a3cb2b1f0c4406a113f5e452cbe596d5307ad045
push id2859
push userakeybl@mozilla.com
push dateMon, 16 Sep 2013 19:14:59 +0000
treeherdermozilla-beta@87d3c51cd2bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs890576
milestone25.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 890576 - Disallow resumption values in onNewGlobalObject hooks. r=jimb
js/src/jit-test/tests/debug/Debugger-onNewGlobalObject-09.js
js/src/jit-test/tests/debug/Debugger-onNewGlobalObject-10.js
js/src/js.msg
js/src/vm/Debugger.cpp
--- a/js/src/jit-test/tests/debug/Debugger-onNewGlobalObject-09.js
+++ b/js/src/jit-test/tests/debug/Debugger-onNewGlobalObject-09.js
@@ -1,27 +1,34 @@
-// Resumption values from onNewGlobalObject handlers are respected.
+// Resumption values from onNewGlobalObject handlers are disallowed.
 
 load(libdir + 'asserts.js');
 
 var dbg = new Debugger;
 var log;
 
 dbg.onNewGlobalObject = function (g) { log += 'n'; return undefined; };
 log = '';
 assertEq(typeof newGlobal(), "object");
 assertEq(log, 'n');
 
-// For onNewGlobalObject, { return: V } resumption values are treated like
-// 'undefined': the new global is still returned.
+dbg.uncaughtExceptionHook = function (ex) { assertEq(/disallowed/.test(ex), true); log += 'u'; }
 dbg.onNewGlobalObject = function (g) { log += 'n'; return { return: "snoo" }; };
 log = '';
 assertEq(typeof newGlobal(), "object");
-assertEq(log, 'n');
+assertEq(log, 'nu');
 
 dbg.onNewGlobalObject = function (g) { log += 'n'; return { throw: "snoo" }; };
 log = '';
-assertThrowsValue(function () { newGlobal(); }, "snoo");
-assertEq(log, 'n');
+assertEq(typeof newGlobal(), "object");
+assertEq(log, 'nu');
 
 dbg.onNewGlobalObject = function (g) { log += 'n'; return null; };
 log = '';
-assertEq(evaluate('newGlobal();', { catchTermination: true }), "terminated");
+assertEq(typeof newGlobal(), "object");
+assertEq(log, 'nu');
+
+dbg.uncaughtExceptionHook = function (ex) { assertEq(/foopy/.test(ex), true); log += 'u'; }
+dbg.onNewGlobalObject = function (g) { log += 'n'; throw "foopy"; };
+log = '';
+assertEq(typeof newGlobal(), "object");
+assertEq(log, 'nu');
+
--- a/js/src/jit-test/tests/debug/Debugger-onNewGlobalObject-10.js
+++ b/js/src/jit-test/tests/debug/Debugger-onNewGlobalObject-10.js
@@ -11,13 +11,17 @@ var count;
 
 dbg1.onNewGlobalObject = dbg2.onNewGlobalObject = dbg3.onNewGlobalObject = function (global) {
   count++;
   log += count;
   if (count == 2)
     return { throw: "snoo" };
   return undefined;
 };
+dbg2.uncaughtExceptionHook = function (exn) {
+  assertEq(/disallowed/.test(exn), true);
+  log += 'u';
+};
 
 log = '';
 count = 0;
-assertThrowsValue(function () { newGlobal(); }, "snoo");
-assertEq(log, '12');
+assertEq(typeof newGlobal(), "object");
+assertEq(log, '12u3');
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -311,17 +311,17 @@ MSG_DEF(JSMSG_NEED_DEBUG_MODE,        25
 MSG_DEF(JSMSG_STRICT_CODE_LET_EXPR_STMT, 258, 0, JSEXN_ERR, "strict mode code may not contain unparenthesized let expression statements")
 MSG_DEF(JSMSG_CANT_CHANGE_EXTENSIBILITY, 259, 0, JSEXN_TYPEERR, "can't change object's extensibility")
 MSG_DEF(JSMSG_SC_BAD_SERIALIZED_DATA, 260, 1, JSEXN_INTERNALERR, "bad serialized structured data ({0})")
 MSG_DEF(JSMSG_SC_UNSUPPORTED_TYPE,    261, 0, JSEXN_TYPEERR, "unsupported type for structured data")
 MSG_DEF(JSMSG_SC_RECURSION,           262, 0, JSEXN_INTERNALERR, "recursive object")
 MSG_DEF(JSMSG_UNUSED263,              263, 0, JSEXN_NONE, "")
 MSG_DEF(JSMSG_BAD_CLONE_VERSION,      264, 0, JSEXN_ERR, "unsupported structured clone version")
 MSG_DEF(JSMSG_CANT_CLONE_OBJECT,      265, 0, JSEXN_TYPEERR, "can't clone object")
-MSG_DEF(JSMSG_UNUSED266,              266, 0, JSEXN_NONE, "")
+MSG_DEF(JSMSG_DEBUG_RESUMPTION_VALUE_DISALLOWED, 266, 0, JSEXN_TYPEERR, "resumption values are disallowed in this hook")
 MSG_DEF(JSMSG_STRICT_FUNCTION_STATEMENT, 267, 0, JSEXN_SYNTAXERR, "in strict mode code, functions may be declared only at top level or immediately within another function")
 MSG_DEF(JSMSG_INVALID_FOR_IN_INIT,    268, 0, JSEXN_SYNTAXERR, "for-in loop let declaration may not have an initializer")
 MSG_DEF(JSMSG_CLEARED_SCOPE,          269, 0, JSEXN_TYPEERR, "attempt to run compile-and-go script on a cleared scope")
 MSG_DEF(JSMSG_MALFORMED_ESCAPE,       270, 1, JSEXN_SYNTAXERR, "malformed {0} character escape sequence")
 MSG_DEF(JSMSG_BAD_GENEXP_BODY,        271, 1, JSEXN_SYNTAXERR, "illegal use of {0} in generator expression")
 MSG_DEF(JSMSG_YIELD_WITHOUT_OPERAND,  272, 0, JSEXN_SYNTAXERR, "yield without a value is deprecated, and illegal in ES6 (use 'yield undefined' instead)")
 MSG_DEF(JSMSG_UNNAMED_FUNCTION_STMT,  273, 0, JSEXN_SYNTAXERR, "function statement requires a name")
 MSG_DEF(JSMSG_CCW_REQUIRED,           274, 1, JSEXN_TYPEERR, "{0}: argument must be an object from a different compartment")
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -1307,18 +1307,37 @@ Debugger::fireNewGlobalObject(JSContext 
 
     Value argv[1];
     AutoArrayRooter argvRooter(cx, ArrayLength(argv), argv);
     argv[0].setObject(*global);
     if (!wrapDebuggeeValue(cx, argvRooter.handleAt(0)))
         return handleUncaughtException(ac, false);
 
     RootedValue rv(cx);
+
+    // onNewGlobalObject is infallible, and thus is only allowed to return
+    // undefined as a resumption value. If it returns anything else, we throw.
+    // And if that happens, or if the hook itself throws, we invoke the
+    // uncaughtExceptionHook so that we never leave an exception pending on the
+    // cx. This allows JS_NewGlobalObject to avoid handling failures from debugger
+    // hooks.
     bool ok = Invoke(cx, ObjectValue(*object), ObjectValue(*hook), 1, argv, &rv);
-    return parseResumptionValue(ac, ok, rv, vp);
+    if (ok && !rv.isUndefined()) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
+                             JSMSG_DEBUG_RESUMPTION_VALUE_DISALLOWED);
+        ok = false;
+    }
+    // NB: Even though we don't care about what goes into it, we have to pass vp
+    // to handleUncaughtException so that it parses resumption values from the
+    // uncaughtExceptionHook and tells the caller whether we should execute the
+    // rest of the onNewGlobalObject hooks or not.
+    JSTrapStatus status = ok ? JSTRAP_CONTINUE
+                             : handleUncaughtException(ac, vp, true);
+    JS_ASSERT(!cx->isExceptionPending());
+    return status;
 }
 
 bool
 Debugger::slowPathOnNewGlobalObject(JSContext *cx, Handle<GlobalObject *> global)
 {
     JS_ASSERT(!JS_CLIST_IS_EMPTY(&cx->runtime()->onNewGlobalObjectWatchers));
 
     /*