Bug 797128 - Minor rooting fix and SkipRoot justification. r=terrence
authorSteve Fink <sfink@mozilla.com>
Fri, 05 Oct 2012 13:14:47 -0700
changeset 109459 40b01e566af01e3f167359b9ce1e69eeec0e7045
parent 109458 e51d8558ad641e9fff3e10e20384a2f365c8a9f9
child 109460 ee13286be5e6b47fb43c6280b74ee9c95b69bdaa
push id23630
push useremorley@mozilla.com
push dateSat, 06 Oct 2012 19:35:27 +0000
treeherdermozilla-central@9f677c2bb33d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs797128
milestone18.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 797128 - Minor rooting fix and SkipRoot justification. r=terrence The first valid SkipRoot I've found. copyFromArray was inexplicably discarding a handle, and with that fixed there are some unrooted internal pointers stored on the stack. But the only things that can trigger GC will also trigger an early exit, so all of the unrooted pointers are dead if a GC happens anyway.
js/src/jstypedarray.cpp
--- a/js/src/jstypedarray.cpp
+++ b/js/src/jstypedarray.cpp
@@ -2056,33 +2056,39 @@ class TypedArrayTemplate
 
         *result = ArrayTypeIsFloatingPoint()
                   ? NativeType(js_NaN)
                   : NativeType(int32_t(0));
         return true;
     }
 
     static bool
-    copyFromArray(JSContext *cx, JSObject *thisTypedArrayObj,
+    copyFromArray(JSContext *cx, HandleObject thisTypedArrayObj,
                   HandleObject ar, uint32_t len, uint32_t offset = 0)
     {
         JS_ASSERT(thisTypedArrayObj->isTypedArray());
         JS_ASSERT(offset <= length(thisTypedArrayObj));
         JS_ASSERT(len <= length(thisTypedArrayObj) - offset);
         if (ar->isTypedArray())
             return copyFromTypedArray(cx, thisTypedArrayObj, ar, offset);
 
+        const Value *src = NULL;
         NativeType *dest = static_cast<NativeType*>(viewData(thisTypedArrayObj)) + offset;
-        SkipRoot skip(cx, &dest);
+
+        // The only way the code below can GC is if nativeFromValue fails, but
+        // in that case we return false immediately, so we do not need to root
+        // |src| and |dest|. These SkipRoots are to protect from the
+        // unconditional MaybeCheckStackRoots done by ToNumber.
+        SkipRoot skipDest(cx, &dest);
+        SkipRoot skipSrc(cx, &src);
 
         if (ar->isDenseArray() && ar->getDenseArrayInitializedLength() >= len) {
             JS_ASSERT(ar->getArrayLength() == len);
 
-            const Value *src = ar->getDenseArrayElements();
-            SkipRoot skipSrc(cx, &src);
+            src = ar->getDenseArrayElements();
             for (uint32_t i = 0; i < len; ++i) {
                 NativeType n;
                 if (!nativeFromValue(cx, src[i], &n))
                     return false;
                 dest[i] = n;
             }
         } else {
             RootedValue v(cx);