Bug 1617527. Remove the same-compartment checks when setting an exception on a JSContext r=sfink
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 26 Feb 2020 21:17:15 +0000
changeset 515737 9364941c6d538953131262f5d2c78163df803b3d
parent 515736 5fc409bc353096febfdcbbc09d788c6d3944ac47
child 515738 d5935fae130083bca5260f3cc21cc1a77eee62ee
push id37161
push useraciure@mozilla.com
push dateThu, 27 Feb 2020 03:39:37 +0000
treeherdermozilla-central@7a5cb26a2d51 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1617527
milestone75.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 1617527. Remove the same-compartment checks when setting an exception on a JSContext r=sfink The exception is not guaranteed to be in the context compartment anyway (e.g. changing the compartment does not wrap the exception), so there isn't much point checking it up front. And the exception stack is always stored as an unwrapped object, so again there's no point checking it on set, since we just plan to UncheckedUnwrap it. Differential Revision: https://phabricator.services.mozilla.com/D64004
js/src/jsapi.cpp
js/src/vm/JSContext.cpp
js/xpconnect/tests/unit/test_bug1617527.js
js/xpconnect/tests/unit/xpcshell.ini
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -4936,17 +4936,19 @@ JS_PUBLIC_API bool JS_GetPendingExceptio
   }
   return cx->getPendingException(vp);
 }
 
 JS_PUBLIC_API void JS_SetPendingException(JSContext* cx, HandleValue value,
                                           JS::ExceptionStackBehavior behavior) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
-  cx->releaseCheck(value);
+  // We don't check the compartment of `value` here, because we're not
+  // doing anything with it other than storing it, and stored
+  // exception values can be in an abitrary compartment.
 
   if (behavior == JS::ExceptionStackBehavior::Capture) {
     cx->setPendingExceptionAndCaptureStack(value);
   } else {
     cx->setPendingException(value, nullptr);
   }
 }
 
@@ -4955,18 +4957,21 @@ JS_PUBLIC_API void JS_ClearPendingExcept
   cx->clearPendingException();
 }
 
 JS_PUBLIC_API void JS::SetPendingExceptionAndStack(JSContext* cx,
                                                    HandleValue value,
                                                    HandleObject stack) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
-  cx->releaseCheck(value);
-  cx->releaseCheck(stack);
+  // We don't check the compartments of `value` and `stack` here,
+  // because we're not doing anything with them other than storing
+  // them, and stored exception values can be in an abitrary
+  // compartment while stored stack values are always the unwrapped
+  // object anyway.
 
   RootedSavedFrame nstack(cx);
   if (stack) {
     nstack = &UncheckedUnwrap(stack)->as<SavedFrame>();
   }
   cx->setPendingException(value, nstack);
 }
 
--- a/js/src/vm/JSContext.cpp
+++ b/js/src/vm/JSContext.cpp
@@ -1440,17 +1440,16 @@ void JSContext::setPendingException(Hand
   } while (false);
 #endif  // defined(NIGHTLY_BUILD)
 
   // overRecursed_ is set after the fact by ReportOverRecursed.
   this->overRecursed_ = false;
   this->throwing = true;
   this->unwrappedException() = v;
   this->unwrappedExceptionStack() = stack;
-  check(v);
 }
 
 void JSContext::setPendingExceptionAndCaptureStack(HandleValue value) {
   RootedObject stack(this);
   if (!CaptureStack(this, &stack)) {
     clearPendingException();
   }
 
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/unit/test_bug1617527.js
@@ -0,0 +1,17 @@
+/* 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/. */
+
+function run_test() {
+  let sb1 = new Cu.Sandbox("https://example.org");
+  let throwingFunc = Cu.evalInSandbox("new Function('throw new Error')", sb1);
+  // NOTE: Different origin from the other sandbox.
+  let sb2 = new Cu.Sandbox("https://example.com");
+  Cu.exportFunction(function() {
+    // Call a different-compartment throwing function.
+    throwingFunc();
+  }, sb2, { defineAs: "func" });
+  let threw = Cu.evalInSandbox("var threw; try { func(); threw = false; } catch (e) { threw = true } threw",
+                                sb2);
+  Assert.ok(threw);
+}
--- a/js/xpconnect/tests/unit/xpcshell.ini
+++ b/js/xpconnect/tests/unit/xpcshell.ini
@@ -57,16 +57,17 @@ support-files =
 [test_bug1082450.js]
 [test_bug1081990.js]
 [test_bug1110546.js]
 [test_bug1131707.js]
 [test_bug1150771.js]
 [test_bug1151385.js]
 [test_bug1170311.js]
 [test_bug1244222.js]
+[test_bug1617527.js]
 [test_bug_442086.js]
 [test_callFunctionWithAsyncStack.js]
 [test_cenums.js]
 [test_compileScript.js]
 [test_deepFreezeClone.js]
 [test_defineModuleGetter.js]
 [test_file.js]
 skip-if = os == 'android' && processor == 'x86_64'