Bug 1475559 part 5 - Pass a global object to nsScriptErrorWithStack. r=bz

The stack object might be a CCW and we want to make it impossible to get a CCW's global. Now the caller has to supply a same-compartment global object, so we no longer rely on getting the CCW's global.

dom/base/nsJSEnvironment.cpp

@@ -220,27 +220,40 @@ namespace xpc {
 // This handles JS Exceptions (via ExceptionStackOrNull), as well as DOM and XPC
 // Exceptions.
 //
-// Note that the returned object is _not_ wrapped into the compartment of
-// exceptionValue.
-JSObject*
+// Note that the returned stackObj and stackGlobal are _not_ wrapped into the
+// compartment of exceptionValue.
+void
 FindExceptionStackForConsoleReport(nsPIDOMWindowInner* win,
-                                   JS::HandleValue exceptionValue)
+                                   JS::HandleValue exceptionValue,
+                                   JS::MutableHandleObject stackObj,
+                                   JS::MutableHandleObject stackGlobal)
 {
+  stackObj.set(nullptr);
+  stackGlobal.set(nullptr);
+
   if (!exceptionValue.isObject()) {
-    return nullptr;
+    return;
   }
 
   if (win && win->AsGlobal()->IsDying()) {
     // Pretend like we have no stack, so we don't end up keeping the global
     // alive via the stack.
-    return nullptr;
+    return;
   }
 
   JS::RootingContext* rcx = RootingCx();
   JS::RootedObject exceptionObject(rcx, &exceptionValue.toObject());
-  JSObject* stackObject = JS::ExceptionStackOrNull(exceptionObject);
-  if (stackObject) {
-    return stackObject;
+  if (JSObject* excStack = JS::ExceptionStackOrNull(exceptionObject)) {
+    // At this point we know exceptionObject is a possibly-wrapped
+    // js::ErrorObject that has excStack as stack. excStack might also be a CCW,
+    // but excStack must be same-compartment with the unwrapped ErrorObject.
+    // Return the ErrorObject's global as stackGlobal. This matches what we do
+    // in the ErrorObject's |.stack| getter and ensures stackObj and stackGlobal
+    // are same-compartment.
+    JSObject* unwrappedException = js::UncheckedUnwrap(exceptionObject);
+    stackObj.set(excStack);
+    stackGlobal.set(JS::GetNonCCWObjectGlobal(unwrappedException));
+    return;
   }
 
   // It is not a JS Exception, try DOM Exception.
@@ -250,20 +263,22 @@ FindExceptionStackForConsoleReport(nsPIDOMWindowInner* win,
     // Not a DOM Exception, try XPC Exception.
     UNWRAP_OBJECT(Exception, exceptionObject, exception);
     if (!exception) {
-      return nullptr;
+      return;
     }
   }
 
   nsCOMPtr<nsIStackFrame> stack = exception->GetLocation();
   if (!stack) {
-    return nullptr;
+    return;
   }
   JS::RootedValue value(rcx);
   stack->GetNativeSavedFrame(&value);
   if (value.isObject()) {
-    return &value.toObject();
+    stackObj.set(&value.toObject());
+    MOZ_ASSERT(JS::IsUnwrappedSavedFrame(stackObj));
+    stackGlobal.set(JS::GetNonCCWObjectGlobal(stackObj));
+    return;
   }
-  return nullptr;
 }
 
 } /* namespace xpc */
@@ -466,9 +481,11 @@ public:
     }
 
     if (status != nsEventStatus_eConsumeNoDefault) {
-      JS::Rooted<JSObject*> stack(rootingCx,
-        xpc::FindExceptionStackForConsoleReport(win, mError));
-      mReport->LogToConsoleWithStack(stack);
+      JS::Rooted<JSObject*> stack(rootingCx);
+      JS::Rooted<JSObject*> stackGlobal(rootingCx);
+      xpc::FindExceptionStackForConsoleReport(win, mError,
+                                              &stack, &stackGlobal);
+      mReport->LogToConsoleWithStack(stack, stackGlobal);
     }
 
     return NS_OK;

dom/bindings/nsScriptError.h

@@ -91,7 +91,7 @@ private:
 
 class nsScriptErrorWithStack : public nsScriptErrorBase {
 public:
-  explicit nsScriptErrorWithStack(JS::HandleObject);
+  nsScriptErrorWithStack(JS::HandleObject aStack, JS::HandleObject aStackGlobal);
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(nsScriptErrorWithStack)
@@ -102,8 +102,10 @@ public:
 private:
   virtual ~nsScriptErrorWithStack();
   // Complete stackframe where the error happened.
-  // Must be SavedFrame object.
-  JS::Heap<JSObject*>  mStack;
+  // Must be a (possibly wrapped) SavedFrame object.
+  JS::Heap<JSObject*> mStack;
+  // Global object that must be same-compartment with mStack.
+  JS::Heap<JSObject*> mStackGlobal;
 };
 
 #endif /* mozilla_dom_nsScriptError_h */

dom/bindings/nsScriptErrorWithStack.cpp

@@ -44,6 +44,7 @@ NS_IMPL_CYCLE_COLLECTION_CLASS(nsScriptErrorWithStack)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsScriptErrorWithStack)
   tmp->mStack = nullptr;
+  tmp->mStackGlobal = nullptr;
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsScriptErrorWithStack)
@@ -51,6 +52,7 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(nsScriptErrorWithStack)
   NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mStack)
+  NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mStackGlobal)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(nsScriptErrorWithStack)
@@ -62,10 +64,16 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsScriptErrorWithStack)
   NS_INTERFACE_MAP_ENTRY(nsIScriptError)
 NS_INTERFACE_MAP_END
 
-nsScriptErrorWithStack::nsScriptErrorWithStack(JS::HandleObject aStack)
+nsScriptErrorWithStack::nsScriptErrorWithStack(JS::HandleObject aStack,
+                                               JS::HandleObject aStackGlobal)
   : mStack(aStack)
+  , mStackGlobal(aStackGlobal)
 {
     MOZ_ASSERT(NS_IsMainThread(), "You can't use this class on workers.");
+
+    MOZ_ASSERT(JS_IsGlobalObject(mStackGlobal));
+    js::AssertSameCompartment(mStack, mStackGlobal);
+
     mozilla::HoldJSObjects(this);
 }
 
@@ -94,7 +102,7 @@ nsScriptErrorWithStack::ToString(nsACString& /*UTF8*/ aResult)
     }
 
     AutoJSAPI jsapi;
-    if (!jsapi.Init(mStack)) {
+    if (!jsapi.Init(mStackGlobal)) {
         return NS_ERROR_FAILURE;
     }
 

dom/ipc/ContentParent.cpp

@@ -4055,7 +4055,10 @@ ContentParent::RecvScriptErrorInternal(const nsString& aMessage,
     }
 
     JS::RootedObject stackObj(cx, &stack.toObject());
-    msg = new nsScriptErrorWithStack(stackObj);
+    MOZ_ASSERT(JS::IsUnwrappedSavedFrame(stackObj));
+
+    JS::RootedObject stackGlobal(cx, JS::GetNonCCWObjectGlobal(stackObj));
+    msg = new nsScriptErrorWithStack(stackObj, stackGlobal);
   } else {
     msg = new nsScriptError();
   }

dom/script/ScriptSettings.cpp

@@ -588,9 +588,10 @@ AutoJSAPI::ReportException()
         JS::RootingContext* rcx = JS::RootingContext::get(cx());
         DispatchScriptErrorEvent(inner, rcx, xpcReport, exn);
       } else {
-        JS::Rooted<JSObject*> stack(cx(),
-          xpc::FindExceptionStackForConsoleReport(inner, exn));
-        xpcReport->LogToConsoleWithStack(stack);
+        JS::Rooted<JSObject*> stack(cx());
+        JS::Rooted<JSObject*> stackGlobal(cx());
+        xpc::FindExceptionStackForConsoleReport(inner, exn, &stack, &stackGlobal);
+        xpcReport->LogToConsoleWithStack(stack, stackGlobal);
       }
     } else {
       // On a worker, we just use the worker error reporting mechanism and don't

js/xpconnect/src/XPCComponents.cpp

@@ -2038,10 +2038,11 @@ nsXPCComponents_Utils::ReportError(HandleValue error, HandleValue stack, JSConte
     nsCOMPtr<nsIScriptError> scripterr;
 
     if (errorObj) {
-        JS::RootedObject stackVal(cx,
-          FindExceptionStackForConsoleReport(win, error));
+        JS::RootedObject stackVal(cx);
+        JS::RootedObject stackGlobal(cx);
+        FindExceptionStackForConsoleReport(win, error, &stackVal, &stackGlobal);
         if (stackVal) {
-            scripterr = new nsScriptErrorWithStack(stackVal);
+            scripterr = new nsScriptErrorWithStack(stackVal, stackGlobal);
         }
     }
 
@@ -2050,12 +2051,17 @@ nsXPCComponents_Utils::ReportError(HandleValue error, HandleValue stack, JSConte
 
     if (!scripterr) {
         RootedObject stackObj(cx);
+        RootedObject stackGlobal(cx);
         if (stack.isObject()) {
             if (!JS::IsMaybeWrappedSavedFrame(&stack.toObject())) {
                 return NS_ERROR_INVALID_ARG;
             }
 
+            // |stack| might be a wrapper, but it must be same-compartment with
+            // the current global.
             stackObj = &stack.toObject();
+            stackGlobal = JS::CurrentGlobalOrNull(cx);
+            js::AssertSameCompartment(stackObj, stackGlobal);
 
             if (GetSavedFrameLine(cx, stackObj, &lineNo) != SavedFrameResult::Ok) {
                 JS_ClearPendingException(cx);
@@ -2078,12 +2084,14 @@ nsXPCComponents_Utils::ReportError(HandleValue error, HandleValue stack, JSConte
                 nsresult rv = frame->GetNativeSavedFrame(&stack);
                 if (NS_SUCCEEDED(rv) && stack.isObject()) {
                   stackObj = &stack.toObject();
+                  MOZ_ASSERT(JS::IsUnwrappedSavedFrame(stackObj));
+                  stackGlobal = JS::GetNonCCWObjectGlobal(stackObj);
                 }
             }
         }
 
         if (stackObj) {
-            scripterr = new nsScriptErrorWithStack(stackObj);
+            scripterr = new nsScriptErrorWithStack(stackObj, stackGlobal);
         }
     }
 

js/xpconnect/src/nsXPConnect.cpp

@@ -319,11 +319,21 @@ xpc::ErrorReport::LogToStderr()
 void
 xpc::ErrorReport::LogToConsole()
 {
-  LogToConsoleWithStack(nullptr);
+    LogToConsoleWithStack(nullptr, nullptr);
 }
+
 void
-xpc::ErrorReport::LogToConsoleWithStack(JS::HandleObject aStack)
+xpc::ErrorReport::LogToConsoleWithStack(JS::HandleObject aStack,
+                                        JS::HandleObject aStackGlobal)
 {
+    if (aStack) {
+        MOZ_ASSERT(aStackGlobal);
+        MOZ_ASSERT(JS_IsGlobalObject(aStackGlobal));
+        js::AssertSameCompartment(aStack, aStackGlobal);
+    } else {
+        MOZ_ASSERT(!aStackGlobal);
+    }
+
     LogToStderr();
 
     MOZ_LOG(gJSDiagnostics,
@@ -344,7 +354,7 @@ xpc::ErrorReport::LogToConsoleWithStack(JS::HandleObject aStack)
       // As we cache messages in the console service,
       // we have to ensure not leaking them after the related
       // context is destroyed and we only track document lifecycle for now.
-      errorObject = new nsScriptErrorWithStack(aStack);
+      errorObject = new nsScriptErrorWithStack(aStack, aStackGlobal);
     } else {
       errorObject = new nsScriptError();
     }

js/xpconnect/src/xpcpublic.h

@@ -605,8 +605,10 @@ class ErrorReport : public ErrorBase {
     void LogToConsole();
     // Log to console, using the given stack object (which should be a stack of
     // the sort that JS::CaptureCurrentStack produces).  aStack is allowed to be
-    // null.
-    void LogToConsoleWithStack(JS::HandleObject aStack);
+    // null. If aStack is non-null, aStackGlobal must be a non-null global
+    // object that's same-compartment with aStack. Note that aStack might be a
+    // CCW.
+    void LogToConsoleWithStack(JS::HandleObject aStack, JS::HandleObject aStackGlobal);
 
     // Produce an error event message string from the given JSErrorReport.  Note
     // that this may produce an empty string if aReport doesn't have a
@@ -625,9 +627,9 @@ void
 DispatchScriptErrorEvent(nsPIDOMWindowInner* win, JS::RootingContext* rootingCx,
                          xpc::ErrorReport* xpcReport, JS::Handle<JS::Value> exception);
 
-// Get a stack of the sort that can be passed to
+// Get a stack (as stackObj outparam) of the sort that can be passed to
 // xpc::ErrorReport::LogToConsoleWithStack from the given exception value.  Can
-// return null if the exception value doesn't have an associated stack.  The
+// be nullptr if the exception value doesn't have an associated stack.  The
 // returned stack, if any, may also not be in the same compartment as
 // exceptionValue.
 //
@@ -636,9 +638,16 @@ DispatchScriptErrorEvent(nsPIDOMWindowInner* win, JS::RootingContext* rootingCx,
 // course.  If it's not null, this function may return a null stack object if
 // the window is far enough gone, because in those cases we don't want to have
 // the stack in the console message keeping the window alive.
-JSObject*
+//
+// If this function sets stackObj to a non-null value, stackGlobal is set to
+// either the JS exception object's global or the global of the SavedFrame we
+// got from a DOM or XPConnect exception. In all cases, stackGlobal is an
+// unwrapped global object and is same-compartment with stackObj.
+void
 FindExceptionStackForConsoleReport(nsPIDOMWindowInner* win,
-                                   JS::HandleValue exceptionValue);
+                                   JS::HandleValue exceptionValue,
+                                   JS::MutableHandleObject stackObj,
+                                   JS::MutableHandleObject stackGlobal);
 
 // Return a name for the realm.
 // This function makes reasonable efforts to make this name both mostly human-readable