Bug 899976 - GC: Fix unsafe references related to ToInt* functions - js engine changes r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 02 Aug 2013 13:15:38 +0100
changeset 153407 f09bcd847699540773617fb28836189e921b04ab
parent 153406 c82a3141a4e94a851a272a137c55ec695d0b564f
child 153408 09837c7d38e4cd94826d2d6296942dcd7227386f
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)
reviewerssfink
bugs899976
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 899976 - GC: Fix unsafe references related to ToInt* functions - js engine changes r=sfink
js/public/RootingAPI.h
js/src/ion/AsmJS.cpp
js/src/jsapi.cpp
js/src/jsapi.h
js/src/jsexn.cpp
js/src/jsnum.cpp
js/src/vm/Interpreter.cpp
js/src/vm/TypedArrayObject.cpp
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -503,19 +503,23 @@ class MOZ_STACK_CLASS MutableHandle : pu
      */
     static MutableHandle fromMarkedLocation(T *p) {
         MutableHandle h;
         h.ptr = p;
         return h;
     }
 
     T *address() const { return ptr; }
-    T get() const { return *ptr; }
+    const T& get() const { return *ptr; }
 
-    operator T() const { return get(); }
+    /*
+     * Return a reference so passing a MutableHandle<T> to something that takes
+     * a |const T&| is not a GC hazard.
+     */
+    operator const T&() const { return get(); }
     T operator->() const { return get(); }
 
   private:
     MutableHandle() {}
 
     T *ptr;
 
     template <typename S> void operator=(S v) MOZ_DELETE;
--- a/js/src/ion/AsmJS.cpp
+++ b/js/src/ion/AsmJS.cpp
@@ -5704,22 +5704,22 @@ GenerateFFIInterpreterExit(ModuleCompile
     }
 
     masm.freeStack(reserveSize + sizeof(int32_t));
     masm.ret();
 #endif
 }
 
 static int32_t
-ValueToInt32(JSContext *cx, Value *val)
+ValueToInt32(JSContext *cx, MutableHandleValue val)
 {
     int32_t i32;
-    if (!ToInt32(cx, val[0], &i32))
+    if (!ToInt32(cx, val, &i32))
         return false;
-    val[0] = Int32Value(i32);
+    val.set(Int32Value(i32));
 
     return true;
 }
 
 static int32_t
 ValueToNumber(JSContext *cx, MutableHandleValue val)
 {
     double dbl;
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -6769,17 +6769,17 @@ AutoGCRooter::AutoGCRooter(ContextFriend
     stackTop(&cx->autoGCRooters)
 {
     JS_ASSERT(this != *stackTop);
     *stackTop = this;
 }
 
 #ifdef DEBUG
 JS_PUBLIC_API(void)
-JS::AssertArgumentsAreSane(JSContext *cx, const JS::Value &value)
+JS::AssertArgumentsAreSane(JSContext *cx, HandleValue value)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, value);
 }
 #endif /* DEBUG */
 
 JS_PUBLIC_API(void *)
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -70,19 +70,19 @@ class JS_PUBLIC_API(AutoCheckRequestDept
 
 #ifdef DEBUG
 /*
  * Assert that we're not doing GC on cx, that we're in a request as
  * needed, and that the compartments for cx and v are correct.
  * Also check that GC would be safe at this point.
  */
 JS_PUBLIC_API(void)
-AssertArgumentsAreSane(JSContext *cx, const Value &v);
+AssertArgumentsAreSane(JSContext *cx, JS::Handle<JS::Value> v);
 #else
-inline void AssertArgumentsAreSane(JSContext *cx, const Value &v) {
+inline void AssertArgumentsAreSane(JSContext *cx, JS::Handle<JS::Value> v) {
     /* Do nothing */
 }
 #endif /* DEBUG */
 
 class JS_PUBLIC_API(AutoGCRooter) {
   public:
     AutoGCRooter(JSContext *cx, ptrdiff_t tag);
     AutoGCRooter(js::ContextFriendFields *cx, ptrdiff_t tag);
@@ -1595,110 +1595,95 @@ JS_ValueToInt64(JSContext *cx, jsval v, 
  * rules for ToUint64: http://dev.w3.org/2006/webapi/WebIDL/#es-unsigned-long-long
  */
 extern JS_PUBLIC_API(JSBool)
 JS_ValueToUint64(JSContext *cx, jsval v, uint64_t *ip);
 
 namespace js {
 /* DO NOT CALL THIS.  Use JS::ToInt16. */
 extern JS_PUBLIC_API(bool)
-ToUint16Slow(JSContext *cx, const JS::Value &v, uint16_t *out);
+ToUint16Slow(JSContext *cx, JS::Handle<JS::Value> v, uint16_t *out);
 
 /* DO NOT CALL THIS.  Use JS::ToInt32. */
 extern JS_PUBLIC_API(bool)
-ToInt32Slow(JSContext *cx, const JS::Value &v, int32_t *out);
+ToInt32Slow(JSContext *cx, JS::Handle<JS::Value> v, int32_t *out);
 
 /* DO NOT CALL THIS.  Use JS::ToUint32. */
 extern JS_PUBLIC_API(bool)
-ToUint32Slow(JSContext *cx, const JS::Value &v, uint32_t *out);
+ToUint32Slow(JSContext *cx, JS::Handle<JS::Value> v, uint32_t *out);
 
 /* DO NOT CALL THIS. Use JS::ToInt64. */
 extern JS_PUBLIC_API(bool)
-ToInt64Slow(JSContext *cx, const JS::Value &v, int64_t *out);
+ToInt64Slow(JSContext *cx, JS::Handle<JS::Value> v, int64_t *out);
 
 /* DO NOT CALL THIS. Use JS::ToUint64. */
 extern JS_PUBLIC_API(bool)
-ToUint64Slow(JSContext *cx, const JS::Value &v, uint64_t *out);
+ToUint64Slow(JSContext *cx, JS::Handle<JS::Value> v, uint64_t *out);
 } /* namespace js */
 
 namespace JS {
 
 JS_ALWAYS_INLINE bool
-ToUint16(JSContext *cx, const JS::Value &v, uint16_t *out)
+ToUint16(JSContext *cx, JS::Handle<JS::Value> v, uint16_t *out)
 {
     AssertArgumentsAreSane(cx, v);
-    {
-        js::SkipRoot skip(cx, &v);
-        js::MaybeCheckStackRoots(cx);
-    }
+    js::MaybeCheckStackRoots(cx);
 
     if (v.isInt32()) {
         *out = uint16_t(v.toInt32());
         return true;
     }
     return js::ToUint16Slow(cx, v, out);
 }
 
 JS_ALWAYS_INLINE bool
-ToInt32(JSContext *cx, const JS::Value &v, int32_t *out)
+ToInt32(JSContext *cx, JS::Handle<JS::Value> v, int32_t *out)
 {
     AssertArgumentsAreSane(cx, v);
-    {
-        js::SkipRoot root(cx, &v);
-        js::MaybeCheckStackRoots(cx);
-    }
+    js::MaybeCheckStackRoots(cx);
 
     if (v.isInt32()) {
         *out = v.toInt32();
         return true;
     }
     return js::ToInt32Slow(cx, v, out);
 }
 
 JS_ALWAYS_INLINE bool
-ToUint32(JSContext *cx, const JS::Value &v, uint32_t *out)
+ToUint32(JSContext *cx, JS::Handle<JS::Value> v, uint32_t *out)
 {
     AssertArgumentsAreSane(cx, v);
-    {
-        js::SkipRoot root(cx, &v);
-        js::MaybeCheckStackRoots(cx);
-    }
+    js::MaybeCheckStackRoots(cx);
 
     if (v.isInt32()) {
         *out = uint32_t(v.toInt32());
         return true;
     }
     return js::ToUint32Slow(cx, v, out);
 }
 
 JS_ALWAYS_INLINE bool
-ToInt64(JSContext *cx, const JS::Value &v, int64_t *out)
+ToInt64(JSContext *cx, JS::Handle<JS::Value> v, int64_t *out)
 {
     AssertArgumentsAreSane(cx, v);
-    {
-        js::SkipRoot skip(cx, &v);
-        js::MaybeCheckStackRoots(cx);
-    }
+    js::MaybeCheckStackRoots(cx);
 
     if (v.isInt32()) {
         *out = int64_t(v.toInt32());
         return true;
     }
 
     return js::ToInt64Slow(cx, v, out);
 }
 
 JS_ALWAYS_INLINE bool
-ToUint64(JSContext *cx, const JS::Value &v, uint64_t *out)
+ToUint64(JSContext *cx, JS::Handle<JS::Value> v, uint64_t *out)
 {
     AssertArgumentsAreSane(cx, v);
-    {
-        js::SkipRoot skip(cx, &v);
-        js::MaybeCheckStackRoots(cx);
-    }
+    js::MaybeCheckStackRoots(cx);
 
     if (v.isInt32()) {
         /* Account for sign extension of negatives into the longer 64bit space. */
         *out = uint64_t(int64_t(v.toInt32()));
         return true;
     }
 
     return js::ToUint64Slow(cx, v, out);
--- a/js/src/jsexn.cpp
+++ b/js/src/jsexn.cpp
@@ -1002,28 +1002,27 @@ IsDuckTypedErrorObject(JSContext *cx, Ha
 
     *filename_strp = filename_str;
     return true;
 }
 
 JSBool
 js_ReportUncaughtException(JSContext *cx)
 {
-    jsval roots[6];
     JSErrorReport *reportp, report;
 
     if (!JS_IsExceptionPending(cx))
         return true;
 
     RootedValue exn(cx);
     if (!JS_GetPendingException(cx, exn.address()))
         return false;
 
-    PodArrayZero(roots);
-    AutoArrayRooter tvr(cx, ArrayLength(roots), roots);
+    AutoValueVector roots(cx);
+    roots.resize(6);
 
     /*
      * Because ToString below could error and an exception object could become
      * unrooted, we must root exnObject.  Later, if exnObject is non-null, we
      * need to root other intermediates, so allocate an operand stack segment
      * to protect all of these values.
      */
     RootedObject exnObject(cx);
@@ -1043,28 +1042,22 @@ js_ReportUncaughtException(JSContext *cx
         roots[1] = StringValue(str);
 
     const char *filename_str = js_fileName_str;
     JSAutoByteString filename;
     if (!reportp && exnObject &&
         (exnObject->is<ErrorObject>() || IsDuckTypedErrorObject(cx, exnObject, &filename_str)))
     {
         RootedString name(cx);
-        if (JS_GetProperty(cx, exnObject, js_name_str, tvr.handleAt(2)) &&
-            JSVAL_IS_STRING(roots[2]))
-        {
-            name = JSVAL_TO_STRING(roots[2]);
-        }
+        if (JS_GetProperty(cx, exnObject, js_name_str, roots.handleAt(2)) && roots[2].isString())
+            name = roots[2].toString();
 
         RootedString msg(cx);
-        if (JS_GetProperty(cx, exnObject, js_message_str, tvr.handleAt(3)) &&
-            JSVAL_IS_STRING(roots[3]))
-        {
-            msg = JSVAL_TO_STRING(roots[3]);
-        }
+        if (JS_GetProperty(cx, exnObject, js_message_str, roots.handleAt(3)) && roots[3].isString())
+            msg = roots[3].toString();
 
         if (name && msg) {
             RootedString colon(cx, JS_NewStringCopyZ(cx, ": "));
             if (!colon)
                 return false;
             RootedString nameColon(cx, ConcatStrings<CanGC>(cx, name, colon));
             if (!nameColon)
                 return false;
@@ -1072,32 +1065,32 @@ js_ReportUncaughtException(JSContext *cx
             if (!str)
                 return false;
         } else if (name) {
             str = name;
         } else if (msg) {
             str = msg;
         }
 
-        if (JS_GetProperty(cx, exnObject, filename_str, tvr.handleAt(4))) {
-            JSString *tmp = ToString<CanGC>(cx, HandleValue::fromMarkedLocation(&roots[4]));
+        if (JS_GetProperty(cx, exnObject, filename_str, roots.handleAt(4))) {
+            JSString *tmp = ToString<CanGC>(cx, roots.handleAt(4));
             if (tmp)
                 filename.encodeLatin1(cx, tmp);
         }
 
         uint32_t lineno;
-        if (!JS_GetProperty(cx, exnObject, js_lineNumber_str, tvr.handleAt(5)) ||
-            !ToUint32(cx, roots[5], &lineno))
+        if (!JS_GetProperty(cx, exnObject, js_lineNumber_str, roots.handleAt(5)) ||
+            !ToUint32(cx, roots.handleAt(5), &lineno))
         {
             lineno = 0;
         }
 
         uint32_t column;
-        if (!JS_GetProperty(cx, exnObject, js_columnNumber_str, tvr.handleAt(5)) ||
-            !ToUint32(cx, roots[5], &column))
+        if (!JS_GetProperty(cx, exnObject, js_columnNumber_str, roots.handleAt(5)) ||
+            !ToUint32(cx, roots.handleAt(5), &column))
         {
             column = 0;
         }
 
         reportp = &report;
         PodZero(&report);
         report.filename = filename.ptr();
         report.lineno = (unsigned) lineno;
--- a/js/src/jsnum.cpp
+++ b/js/src/jsnum.cpp
@@ -1529,17 +1529,17 @@ js::ToNumberSlow(JSContext *cx, Value v,
 # pragma optimize("", on)
 #endif
 
 /*
  * Convert a value to an int64_t, according to the WebIDL rules for long long
  * conversion. Return converted value in *out on success, false on failure.
  */
 JS_PUBLIC_API(bool)
-js::ToInt64Slow(JSContext *cx, const Value &v, int64_t *out)
+js::ToInt64Slow(JSContext *cx, const HandleValue v, int64_t *out)
 {
     JS_ASSERT(!v.isInt32());
     double d;
     if (v.isDouble()) {
         d = v.toDouble();
     } else {
         if (!ToNumberSlow(cx, v, &d))
             return false;
@@ -1548,62 +1548,62 @@ js::ToInt64Slow(JSContext *cx, const Val
     return true;
 }
 
 /*
  * Convert a value to an uint64_t, according to the WebIDL rules for unsigned long long
  * conversion. Return converted value in *out on success, false on failure.
  */
 JS_PUBLIC_API(bool)
-js::ToUint64Slow(JSContext *cx, const Value &v, uint64_t *out)
+js::ToUint64Slow(JSContext *cx, const HandleValue v, uint64_t *out)
 {
     JS_ASSERT(!v.isInt32());
     double d;
     if (v.isDouble()) {
         d = v.toDouble();
     } else {
         if (!ToNumberSlow(cx, v, &d))
             return false;
     }
     *out = ToUint64(d);
     return true;
 }
 
 JS_PUBLIC_API(bool)
-js::ToInt32Slow(JSContext *cx, const Value &v, int32_t *out)
+js::ToInt32Slow(JSContext *cx, const HandleValue v, int32_t *out)
 {
     JS_ASSERT(!v.isInt32());
     double d;
     if (v.isDouble()) {
         d = v.toDouble();
     } else {
         if (!ToNumberSlow(cx, v, &d))
             return false;
     }
     *out = ToInt32(d);
     return true;
 }
 
 JS_PUBLIC_API(bool)
-js::ToUint32Slow(JSContext *cx, const Value &v, uint32_t *out)
+js::ToUint32Slow(JSContext *cx, const HandleValue v, uint32_t *out)
 {
     JS_ASSERT(!v.isInt32());
     double d;
     if (v.isDouble()) {
         d = v.toDouble();
     } else {
         if (!ToNumberSlow(cx, v, &d))
             return false;
     }
     *out = ToUint32(d);
     return true;
 }
 
 JS_PUBLIC_API(bool)
-js::ToUint16Slow(JSContext *cx, const Value &v, uint16_t *out)
+js::ToUint16Slow(JSContext *cx, const HandleValue v, uint16_t *out)
 {
     JS_ASSERT(!v.isInt32());
     double d;
     if (v.isDouble()) {
         d = v.toDouble();
     } else if (!ToNumberSlow(cx, v, &d)) {
         return false;
     }
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -1989,19 +1989,19 @@ BEGIN_CASE(JSOP_BINDNAME)
 
     PUSH_OBJECT(*scope);
 }
 END_CASE(JSOP_BINDNAME)
 
 #define BITWISE_OP(OP)                                                        \
     JS_BEGIN_MACRO                                                            \
         int32_t i, j;                                                         \
-        if (!ToInt32(cx, regs.sp[-2], &i))                                    \
+        if (!ToInt32(cx, regs.stackHandleAt(-2), &i))                         \
             goto error;                                                       \
-        if (!ToInt32(cx, regs.sp[-1], &j))                                    \
+        if (!ToInt32(cx, regs.stackHandleAt(-1), &j))                         \
             goto error;                                                       \
         i = i OP j;                                                           \
         regs.sp--;                                                            \
         regs.sp[-1].setInt32(i);                                              \
     JS_END_MACRO
 
 BEGIN_CASE(JSOP_BITOR)
     BITWISE_OP(|);
@@ -2118,19 +2118,19 @@ BEGIN_CASE(JSOP_GE)
     regs.sp[-2].setBoolean(cond);
     regs.sp--;
 }
 END_CASE(JSOP_GE)
 
 #define SIGNED_SHIFT_OP(OP)                                                   \
     JS_BEGIN_MACRO                                                            \
         int32_t i, j;                                                         \
-        if (!ToInt32(cx, regs.sp[-2], &i))                                    \
+        if (!ToInt32(cx, regs.stackHandleAt(-2), &i))                         \
             goto error;                                                       \
-        if (!ToInt32(cx, regs.sp[-1], &j))                                    \
+        if (!ToInt32(cx, regs.stackHandleAt(-1), &j))                         \
             goto error;                                                       \
         i = i OP (j & 31);                                                    \
         regs.sp--;                                                            \
         regs.sp[-1].setInt32(i);                                              \
     JS_END_MACRO
 
 BEGIN_CASE(JSOP_LSH)
     SIGNED_SHIFT_OP(<<);
--- a/js/src/vm/TypedArrayObject.cpp
+++ b/js/src/vm/TypedArrayObject.cpp
@@ -184,33 +184,34 @@ ArrayBufferObject::fun_slice(JSContext *
 
 /*
  * new ArrayBuffer(byteLength)
  */
 JSBool
 ArrayBufferObject::class_constructor(JSContext *cx, unsigned argc, Value *vp)
 {
     int32_t nbytes = 0;
-    if (argc > 0 && !ToInt32(cx, vp[2], &nbytes))
+    CallArgs args = CallArgsFromVp(argc, vp);
+    if (argc > 0 && !ToInt32(cx, args[0], &nbytes))
         return false;
 
     if (nbytes < 0) {
         /*
          * We're just not going to support arrays that are bigger than what will fit
          * as an integer value; if someone actually ever complains (validly), then we
          * can fix.
          */
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_BAD_ARRAY_LENGTH);
         return false;
     }
 
     JSObject *bufobj = create(cx, uint32_t(nbytes));
     if (!bufobj)
         return false;
-    vp->setObject(*bufobj);
+    args.rval().setObject(*bufobj);
     return true;
 }
 
 /*
  * Note that some callers are allowed to pass in a NULL cx, so we allocate with
  * the cx if available and fall back to the runtime.  If oldptr is given,
  * it's expected to be a previously-allocated ObjectElements* pointer that we
  * then realloc.
@@ -1824,67 +1825,66 @@ class TypedArrayObjectTemplate : public 
      * new [Type]Array(otherTypedArray)
      * new [Type]Array(JSArray)
      * new [Type]Array(ArrayBuffer, [optional] byteOffset, [optional] length)
      */
     static JSBool
     class_constructor(JSContext *cx, unsigned argc, Value *vp)
     {
         /* N.B. this is a constructor for protoClass, not fastClass! */
-        JSObject *obj = create(cx, argc, JS_ARGV(cx, vp));
+        CallArgs args = CallArgsFromVp(argc, vp);
+        JSObject *obj = create(cx, args);
         if (!obj)
             return false;
-        vp->setObject(*obj);
+        args.rval().setObject(*obj);
         return true;
     }
 
     static JSObject *
-    create(JSContext *cx, unsigned argc, Value *argv)
+    create(JSContext *cx, const CallArgs& args)
     {
-        /* N.B. there may not be an argv[-2]/argv[-1]. */
-
         /* () or (number) */
         uint32_t len = 0;
-        if (argc == 0 || ValueIsLength(argv[0], &len))
+        if (args.length() == 0 || ValueIsLength(args[0], &len))
             return fromLength(cx, len);
 
         /* (not an object) */
-        if (!argv[0].isObject()) {
+        if (!args[0].isObject()) {
             JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_TYPED_ARRAY_BAD_ARGS);
             return NULL;
         }
 
-        RootedObject dataObj(cx, &argv[0].toObject());
+        RootedObject dataObj(cx, &args.get(0).toObject());
 
         /*
          * (typedArray)
          * (type[] array)
          *
          * Otherwise create a new typed array and copy elements 0..len-1
          * properties from the object, treating it as some sort of array.
          * Note that offset and length will be ignored
          */
         if (!UncheckedUnwrap(dataObj)->is<ArrayBufferObject>())
             return fromArray(cx, dataObj);
 
         /* (ArrayBuffer, [byteOffset, [length]]) */
         int32_t byteOffset = 0;
         int32_t length = -1;
 
-        if (argc > 1) {
-            if (!ToInt32(cx, argv[1], &byteOffset))
+        if (args.length() > 1) {
+            if (!ToInt32(cx, args[1], &byteOffset))
                 return NULL;
             if (byteOffset < 0) {
                 JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
                                      JSMSG_TYPED_ARRAY_NEGATIVE_ARG, "1");
                 return NULL;
             }
 
-            if (argc > 2) {
-                if (!ToInt32(cx, argv[2], &length))
+            if (args.length() > 2) {
+                if (!ToInt32(cx, args[2], &length))
                     return NULL;
                 if (length < 0) {
                     JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
                                          JSMSG_TYPED_ARRAY_NEGATIVE_ARG, "2");
                     return NULL;
                 }
             }
         }