Bug 1053999 - Rationalize stack recursion checks, r=bholley, a=lsblakk
authorSteve Fink <sfink@mozilla.com>
Mon, 17 Nov 2014 21:26:36 -0800
changeset 233953 c8ebeea28f1998ab512216aa7f853d50fe9cb104
parent 233952 8651ad90ff2f7240a322595531b84564ef987ad1
child 233954 b4cd2a209317c33c5e0851a26e5ba43f67290b79
push id1
push usersledru@mozilla.com
push dateThu, 04 Dec 2014 17:57:20 +0000
reviewersbholley, lsblakk
bugs1053999
milestone35.0a2
Bug 1053999 - Rationalize stack recursion checks, r=bholley, a=lsblakk
dom/base/nsGlobalWindow.cpp
dom/bindings/BindingUtils.cpp
js/src/jscompartment.cpp
js/src/jsfriendapi.h
js/xpconnect/src/XPCWrappedNative.cpp
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -2403,19 +2403,20 @@ nsGlobalWindow::SetNewDocument(nsIDocume
     currentInner->mNavigator->OnNavigation();
   }
 
   nsRefPtr<nsGlobalWindow> newInnerWindow;
   bool createdInnerWindow = false;
 
   bool thisChrome = IsChromeWindow();
 
-  // Check if we're near the stack limit before we get anywhere near the
-  // transplanting code. We use a conservative check since we'll use a little
-  // more space before we actually hit the critical "can't fail" path.
+  // Check if we're anywhere near the stack limit before we reach the
+  // transplanting code, since it has no good way to handle errors. This uses
+  // the untrusted script limit, which is not strictly necessary since no
+  // actual script should run.
   JS_CHECK_RECURSION_CONSERVATIVE(cx, return NS_ERROR_FAILURE);
 
   nsCOMPtr<WindowStateHolder> wsh = do_QueryInterface(aState);
   NS_ASSERTION(!aState || wsh, "What kind of weird state are you giving me here?");
 
   JS::Rooted<JSObject*> newInnerGlobal(cx);
   if (reUseInnerWindow) {
     // We're reusing the current inner window.
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -1682,19 +1682,20 @@ private:
   JS::Rooted<JSObject*> mNewReflector;
 };
 
 nsresult
 ReparentWrapper(JSContext* aCx, JS::Handle<JSObject*> aObjArg)
 {
   js::AssertSameCompartment(aCx, aObjArg);
 
-  // Check if we're near the stack limit before we get anywhere near the
-  // transplanting code. We use a conservative check since we'll use a little
-  // more space before we actually hit the critical "can't fail" path.
+  // Check if we're anywhere near the stack limit before we reach the
+  // transplanting code, since it has no good way to handle errors. This uses
+  // the untrusted script limit, which is not strictly necessary since no
+  // actual script should run.
   JS_CHECK_RECURSION_CONSERVATIVE(aCx, return NS_ERROR_FAILURE);
 
   JS::Rooted<JSObject*> aObj(aCx, aObjArg);
   const DOMJSClass* domClass = GetDOMClass(aObj);
 
   JS::Rooted<JSObject*> oldParent(aCx, JS_GetParent(aObj));
   JS::Rooted<JSObject*> newParent(aCx, domClass->mGetParent(aCx, aObj));
 
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -396,17 +396,17 @@ JSCompartment::wrap(JSContext *cx, Mutab
         if (!GetBuiltinConstructor(cx, JSProto_StopIteration, &stopIteration))
             return false;
         obj.set(stopIteration);
         return true;
     }
 
     // Invoke the prewrap callback. We're a bit worried about infinite
     // recursion here, so we do a check - see bug 809295.
-    JS_CHECK_CHROME_RECURSION(cx, return false);
+    JS_CHECK_SYSTEM_RECURSION(cx, return false);
     if (cb->preWrap) {
         obj.set(cb->preWrap(cx, global, obj, objectPassedToWrap));
         if (!obj)
             return false;
     }
     MOZ_ASSERT(obj == GetOuterObject(cx, obj));
 
     if (obj->compartment() == this)
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -15,37 +15,22 @@
 #include "jsapi.h" // For JSAutoByteString.  See bug 1033916.
 #include "jsbytecode.h"
 #include "jspubtd.h"
 
 #include "js/CallArgs.h"
 #include "js/CallNonGenericMethod.h"
 #include "js/Class.h"
 
-/*
- * This macro checks if the stack pointer has exceeded a given limit. If
- * |tolerance| is non-zero, it returns true only if the stack pointer has
- * exceeded the limit by more than |tolerance| bytes. The WITH_INTOLERANCE
- * versions use a negative tolerance (i.e., the limit is reduced by
- * |intolerance| bytes).
- */
 #if JS_STACK_GROWTH_DIRECTION > 0
-# define JS_CHECK_STACK_SIZE_WITH_TOLERANCE(limit, sp, tolerance)  \
-    ((uintptr_t)(sp) < (limit)+(tolerance))
-# define JS_CHECK_STACK_SIZE_WITH_INTOLERANCE(limit, sp, intolerance)  \
-    ((uintptr_t)(sp) < (limit)-(intolerance))
+# define JS_CHECK_STACK_SIZE(limit, sp) ((uintptr_t)(sp) < (limit))
 #else
-# define JS_CHECK_STACK_SIZE_WITH_TOLERANCE(limit, sp, tolerance)  \
-    ((uintptr_t)(sp) > (limit)-(tolerance))
-# define JS_CHECK_STACK_SIZE_WITH_INTOLERANCE(limit, sp, intolerance)  \
-    ((uintptr_t)(sp) > (limit)+(intolerance))
+# define JS_CHECK_STACK_SIZE(limit, sp) ((uintptr_t)(sp) > (limit))
 #endif
 
-#define JS_CHECK_STACK_SIZE(limit, lval) JS_CHECK_STACK_SIZE_WITH_TOLERANCE(limit, lval, 0)
-
 class JSAtom;
 struct JSErrorFormatString;
 class JSLinearString;
 struct JSJitInfo;
 struct JSErrorReport;
 
 namespace JS {
 template <class T>
@@ -1000,41 +985,58 @@ IsObjectInContextCompartment(JSObject *o
 #define JSITER_HIDDEN     0x10  /* also enumerate non-enumerable properties */
 #define JSITER_SYMBOLS    0x20  /* also include symbol property keys */
 #define JSITER_SYMBOLSONLY 0x40 /* exclude string property keys */
 
 JS_FRIEND_API(bool)
 RunningWithTrustedPrincipals(JSContext *cx);
 
 inline uintptr_t
-GetNativeStackLimit(JSContext *cx)
+GetNativeStackLimit(JSContext *cx, StackKind kind, int extraAllowance = 0)
+{
+    PerThreadDataFriendFields *mainThread =
+      PerThreadDataFriendFields::getMainThread(GetRuntime(cx));
+    uintptr_t limit = mainThread->nativeStackLimit[kind];
+#if JS_STACK_GROWTH_DIRECTION > 0
+    limit += extraAllowance;
+#else
+    limit -= extraAllowance;
+#endif
+    return limit;
+}
+
+inline uintptr_t
+GetNativeStackLimit(JSContext *cx, int extraAllowance = 0)
 {
     StackKind kind = RunningWithTrustedPrincipals(cx) ? StackForTrustedScript
                                                       : StackForUntrustedScript;
-    PerThreadDataFriendFields *mainThread =
-      PerThreadDataFriendFields::getMainThread(GetRuntime(cx));
-    return mainThread->nativeStackLimit[kind];
+    return GetNativeStackLimit(cx, kind, extraAllowance);
 }
 
 /*
  * These macros report a stack overflow and run |onerror| if we are close to
- * using up the C stack. The JS_CHECK_CHROME_RECURSION variant gives us a little
- * extra space so that we can ensure that crucial code is able to run.
- * JS_CHECK_RECURSION_CONSERVATIVE gives us a little less space.
+ * using up the C stack. The JS_CHECK_CHROME_RECURSION variant gives us a
+ * little extra space so that we can ensure that crucial code is able to run.
+ * JS_CHECK_RECURSION_CONSERVATIVE allows less space than any other check,
+ * including a safety buffer (as in, it uses the untrusted limit and subtracts
+ * a little more from it).
  */
 
-#define JS_CHECK_RECURSION(cx, onerror)                                         \
+#define JS_CHECK_RECURSION_LIMIT(cx, limit, onerror)                            \
     JS_BEGIN_MACRO                                                              \
         int stackDummy_;                                                        \
-        if (!JS_CHECK_STACK_SIZE(js::GetNativeStackLimit(cx), &stackDummy_)) {  \
+        if (!JS_CHECK_STACK_SIZE(limit, &stackDummy_)) {                        \
             js_ReportOverRecursed(cx);                                          \
             onerror;                                                            \
         }                                                                       \
     JS_END_MACRO
 
+#define JS_CHECK_RECURSION(cx, onerror)                                         \
+    JS_CHECK_RECURSION_LIMIT(cx, js::GetNativeStackLimit(cx), onerror)
+
 #define JS_CHECK_RECURSION_DONT_REPORT(cx, onerror)                             \
     JS_BEGIN_MACRO                                                              \
         int stackDummy_;                                                        \
         if (!JS_CHECK_STACK_SIZE(js::GetNativeStackLimit(cx), &stackDummy_)) {  \
             onerror;                                                            \
         }                                                                       \
     JS_END_MACRO
 
@@ -1048,39 +1050,21 @@ GetNativeStackLimit(JSContext *cx)
 #define JS_CHECK_RECURSION_WITH_SP(cx, sp, onerror)                             \
     JS_BEGIN_MACRO                                                              \
         if (!JS_CHECK_STACK_SIZE(js::GetNativeStackLimit(cx), sp)) {            \
             js_ReportOverRecursed(cx);                                          \
             onerror;                                                            \
         }                                                                       \
     JS_END_MACRO
 
-#define JS_CHECK_CHROME_RECURSION(cx, onerror)                                  \
-    JS_BEGIN_MACRO                                                              \
-        int stackDummy_;                                                        \
-        if (!JS_CHECK_STACK_SIZE_WITH_TOLERANCE(js::GetNativeStackLimit(cx),    \
-                                                &stackDummy_,                   \
-                                                1024 * sizeof(size_t)))         \
-        {                                                                       \
-            js_ReportOverRecursed(cx);                                          \
-            onerror;                                                            \
-        }                                                                       \
-    JS_END_MACRO
+#define JS_CHECK_SYSTEM_RECURSION(cx, onerror)                                  \
+    JS_CHECK_RECURSION_LIMIT(cx, js::GetNativeStackLimit(cx, js::StackForSystemCode), onerror)
 
 #define JS_CHECK_RECURSION_CONSERVATIVE(cx, onerror)                            \
-    JS_BEGIN_MACRO                                                              \
-        int stackDummy_;                                                        \
-        if (!JS_CHECK_STACK_SIZE_WITH_INTOLERANCE(js::GetNativeStackLimit(cx),  \
-                                                  &stackDummy_,                 \
-                                                  1024 * sizeof(size_t)))       \
-        {                                                                       \
-            js_ReportOverRecursed(cx);                                          \
-            onerror;                                                            \
-        }                                                                       \
-    JS_END_MACRO
+    JS_CHECK_RECURSION_LIMIT(cx, js::GetNativeStackLimit(cx, js::StackForUntrustedScript, -1024 * int(sizeof(size_t))), onerror)
 
 JS_FRIEND_API(void)
 StartPCCountProfiling(JSContext *cx);
 
 JS_FRIEND_API(void)
 StopPCCountProfiling(JSContext *cx);
 
 JS_FRIEND_API(void)
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -1084,19 +1084,20 @@ private:
 
 // static
 nsresult
 XPCWrappedNative::ReparentWrapperIfFound(XPCWrappedNativeScope* aOldScope,
                                          XPCWrappedNativeScope* aNewScope,
                                          HandleObject aNewParent,
                                          nsISupports* aCOMObj)
 {
-    // Check if we're near the stack limit before we get anywhere near the
-    // transplanting code. We use a conservative check since we'll use a little
-    // more space before we actually hit the critical "can't fail" path.
+    // Check if we're anywhere near the stack limit before we reach the
+    // transplanting code, since it has no good way to handle errors. This uses
+    // the untrusted script limit, which is not strictly necessary since no
+    // actual script should run.
     AutoJSContext cx;
     JS_CHECK_RECURSION_CONSERVATIVE(cx, return NS_ERROR_FAILURE);
 
     XPCNativeInterface* iface = XPCNativeInterface::GetISupports();
     if (!iface)
         return NS_ERROR_FAILURE;
 
     nsresult rv;