Bug 963738 - Fix some false positives in the Array analysis; r=jonco
authorTerrence Cole <terrence@mozilla.com>
Tue, 28 Jan 2014 12:05:35 -0800
changeset 181888 ce4d2dd81858c50964fc4ef0db7a97dc3a2ba2e9
parent 181887 b1974150d1a511db41e06cb1067d77962740ff16
child 181889 b3ccfb70c31bbc2da246dbad4d6dc09c07a0f9d8
push id3343
push userffxbld
push dateMon, 17 Mar 2014 21:55:32 +0000
treeherdermozilla-beta@2f7d3415f79f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs963738
milestone29.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 963738 - Fix some false positives in the Array analysis; r=jonco
content/base/src/nsObjectLoadingContent.cpp
dom/base/nsJSEnvironment.cpp
js/src/jit/BaselineIC.cpp
js/src/jit/VMFunctions.cpp
js/src/jsapi-tests/testArgumentsObject.cpp
js/src/jsapi-tests/testNewObject.cpp
js/src/jsapi.h
js/src/jscntxt.h
js/src/jsproxy.cpp
js/src/jsreflect.cpp
--- a/content/base/src/nsObjectLoadingContent.cpp
+++ b/content/base/src/nsObjectLoadingContent.cpp
@@ -3216,17 +3216,17 @@ nsObjectLoadingContent::LegacyCall(JSCon
   }
 
   if (!pi_obj) {
     aRv.Throw(NS_ERROR_NOT_AVAILABLE);
     return JS::UndefinedValue();
   }
 
   JS::Rooted<JS::Value> retval(aCx);
-  bool ok = JS::Call(aCx, thisVal, pi_obj, args.Length(), rooter.array,
+  bool ok = JS::Call(aCx, thisVal, pi_obj, rooter.length(), rooter.start(),
                      &retval);
   if (!ok) {
     aRv.Throw(NS_ERROR_FAILURE);
     return JS::UndefinedValue();
   }
 
   Telemetry::Accumulate(Telemetry::PLUGIN_CALLED_DIRECTLY, true);
   return retval;
--- a/dom/base/nsJSEnvironment.cpp
+++ b/dom/base/nsJSEnvironment.cpp
@@ -1177,17 +1177,17 @@ nsJSContext::SetProperty(JS::Handle<JSOb
   // got the arguments, now attach them.
 
   for (uint32_t i = 0; i < argc; ++i) {
     if (!JS_WrapValue(mContext, array.handleAt(i))) {
       return NS_ERROR_FAILURE;
     }
   }
 
-  JSObject *args = ::JS_NewArrayObject(mContext, argc, array.array);
+  JSObject *args = ::JS_NewArrayObject(mContext, argc, array.start());
   if (!args) {
     return NS_ERROR_FAILURE;
   }
   JS::Value vargs = OBJECT_TO_JSVAL(args);
 
   return JS_DefineProperty(mContext, aTarget, aPropName, vargs,
   						   nullptr, nullptr, 0) ? NS_OK : NS_ERROR_FAILURE;
 }
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -1600,20 +1600,20 @@ DoCallNativeGetter(JSContext *cx, Handle
                    MutableHandleValue result)
 {
     JS_ASSERT(callee->isNative());
     JSNative natfun = callee->native();
 
     Value vp[2] = { ObjectValue(*callee.get()), ObjectValue(*obj.get()) };
     AutoValueArray rootVp(cx, vp, 2);
 
-    if (!natfun(cx, 0, vp))
-        return false;
-
-    result.set(vp[0]);
+    if (!natfun(cx, 0, rootVp.start()))
+        return false;
+
+    result.set(rootVp[0]);
     return true;
 }
 
 typedef bool (*DoCallNativeGetterFn)(JSContext *, HandleFunction, HandleObject, MutableHandleValue);
 static const VMFunction DoCallNativeGetterInfo =
     FunctionInfo<DoCallNativeGetterFn>(DoCallNativeGetter);
 
 //
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -360,56 +360,56 @@ bool
 ArrayPopDense(JSContext *cx, HandleObject obj, MutableHandleValue rval)
 {
     JS_ASSERT(obj->is<ArrayObject>());
 
     AutoDetectInvalidation adi(cx, rval.address());
 
     Value argv[] = { UndefinedValue(), ObjectValue(*obj) };
     AutoValueArray ava(cx, argv, 2);
-    if (!js::array_pop(cx, 0, argv))
+    if (!js::array_pop(cx, 0, ava.start()))
         return false;
 
     // If the result is |undefined|, the array was probably empty and we
     // have to monitor the return value.
-    rval.set(argv[0]);
+    rval.set(ava[0]);
     if (rval.isUndefined())
         types::TypeScript::Monitor(cx, rval);
     return true;
 }
 
 bool
 ArrayPushDense(JSContext *cx, HandleObject obj, HandleValue v, uint32_t *length)
 {
     JS_ASSERT(obj->is<ArrayObject>());
 
     Value argv[] = { UndefinedValue(), ObjectValue(*obj), v };
     AutoValueArray ava(cx, argv, 3);
-    if (!js::array_push(cx, 1, argv))
+    if (!js::array_push(cx, 1, ava.start()))
         return false;
 
-    *length = argv[0].toInt32();
+    *length = ava[0].toInt32();
     return true;
 }
 
 bool
 ArrayShiftDense(JSContext *cx, HandleObject obj, MutableHandleValue rval)
 {
     JS_ASSERT(obj->is<ArrayObject>());
 
     AutoDetectInvalidation adi(cx, rval.address());
 
     Value argv[] = { UndefinedValue(), ObjectValue(*obj) };
     AutoValueArray ava(cx, argv, 2);
-    if (!js::array_shift(cx, 0, argv))
+    if (!js::array_shift(cx, 0, ava.start()))
         return false;
 
     // If the result is |undefined|, the array was probably empty and we
     // have to monitor the return value.
-    rval.set(argv[0]);
+    rval.set(ava[0]);
     if (rval.isUndefined())
         types::TypeScript::Monitor(cx, rval);
     return true;
 }
 
 JSObject *
 ArrayConcatDense(JSContext *cx, HandleObject obj1, HandleObject obj2, HandleObject objRes)
 {
@@ -421,19 +421,19 @@ ArrayConcatDense(JSContext *cx, HandleOb
         // Fast path if we managed to allocate an object inline.
         if (!js::array_concat_dense(cx, arr1, arr2, arrRes))
             return nullptr;
         return arrRes;
     }
 
     Value argv[] = { UndefinedValue(), ObjectValue(*arr1), ObjectValue(*arr2) };
     AutoValueArray ava(cx, argv, 3);
-    if (!js::array_concat(cx, 1, argv))
+    if (!js::array_concat(cx, 1, ava.start()))
         return nullptr;
-    return &argv[0].toObject();
+    return &ava[0].toObject();
 }
 
 bool
 CharCodeAt(JSContext *cx, HandleString str, int32_t index, uint32_t *code)
 {
     jschar c;
     if (!str->getChar(cx, index, &c))
         return false;
--- a/js/src/jsapi-tests/testArgumentsObject.cpp
+++ b/js/src/jsapi-tests/testArgumentsObject.cpp
@@ -80,33 +80,34 @@ template<size_t ArgCount> bool
 ExhaustiveTest(const char funcode[])
 {
     RootedValue v(cx);
     EVAL(funcode, v.address());
 
     EVAL(CALL_CODES[ArgCount], v.address());
     Rooted<ArgumentsObject*> argsobj(cx, &JSVAL_TO_OBJECT(v)->as<ArgumentsObject>());
 
-    Value elems[MAX_ELEMS];
+    Value elems_[MAX_ELEMS];
+    AutoValueArray elems(cx, elems_, MAX_ELEMS);
 
     for (size_t i = 0; i <= ArgCount; i++) {
         for (size_t j = 0; j <= ArgCount - i; j++) {
             ClearElements(elems);
-            CHECK(argsobj->maybeGetElements(i, j, elems));
+            CHECK(argsobj->maybeGetElements(i, j, elems.start()));
             for (size_t k = 0; k < j; k++)
                 CHECK_SAME(elems[k], INT_TO_JSVAL(i + k));
             for (size_t k = j; k < MAX_ELEMS - 1; k++)
                 CHECK_SAME(elems[k], JSVAL_NULL);
             CHECK_SAME(elems[MAX_ELEMS - 1], INT_TO_JSVAL(42));
         }
     }
 
     return true;
 }
 
 static void
-ClearElements(Value elems[MAX_ELEMS])
+ClearElements(AutoValueArray &elems)
 {
-    for (size_t i = 0; i < MAX_ELEMS - 1; i++)
-        elems[i] = NullValue();
-    elems[MAX_ELEMS - 1] = Int32Value(42);
+    for (size_t i = 0; i < elems.length() - 1; i++)
+        elems[i].setNull();
+    elems[elems.length() - 1].setInt32(42);
 }
 END_TEST(testArgumentsObject)
--- a/js/src/jsapi-tests/testNewObject.cpp
+++ b/js/src/jsapi-tests/testNewObject.cpp
@@ -50,49 +50,45 @@ constructHook(JSContext *cx, unsigned ar
     args[2].setUndefined();
 
     return true;
 }
 
 BEGIN_TEST(testNewObject_1)
 {
     static const size_t N = 1000;
-    jsval argv[N];
-
-    // Root the global argv test array. Only the first 2 entries really need to
-    // be rooted, since we're only putting integers in the rest.
-    CHECK(JS_AddNamedValueRoot(cx, &argv[0], "argv0"));
-    CHECK(JS_AddNamedValueRoot(cx, &argv[1], "argv1"));
+    JS::Value argv_[N];
+    JS::AutoArrayRooter argv(cx, N, argv_);
 
     JS::RootedValue v(cx);
     EVAL("Array", v.address());
     JS::RootedObject Array(cx, JSVAL_TO_OBJECT(v));
 
     // With no arguments.
     JS::RootedObject obj(cx, JS_New(cx, Array, 0, nullptr));
     CHECK(obj);
     JS::RootedValue rt(cx, OBJECT_TO_JSVAL(obj));
     CHECK(JS_IsArrayObject(cx, obj));
     uint32_t len;
     CHECK(JS_GetArrayLength(cx, obj, &len));
     CHECK_EQUAL(len, 0);
 
     // With one argument.
-    argv[0] = INT_TO_JSVAL(4);
-    obj = JS_New(cx, Array, 1, argv);
+    argv[0].setInt32(4);
+    obj = JS_New(cx, Array, 1, argv.start());
     CHECK(obj);
     rt = OBJECT_TO_JSVAL(obj);
     CHECK(JS_IsArrayObject(cx, obj));
     CHECK(JS_GetArrayLength(cx, obj, &len));
     CHECK_EQUAL(len, 4);
 
     // With N arguments.
     for (size_t i = 0; i < N; i++)
-        argv[i] = INT_TO_JSVAL(i);
-    obj = JS_New(cx, Array, N, argv);
+        argv[i].setInt32(i);
+    obj = JS_New(cx, Array, N, argv.start());
     CHECK(obj);
     rt = OBJECT_TO_JSVAL(obj);
     CHECK(JS_IsArrayObject(cx, obj));
     CHECK(JS_GetArrayLength(cx, obj, &len));
     CHECK_EQUAL(len, N);
     CHECK(JS_GetElement(cx, obj, N - 1, &v));
     CHECK_SAME(v, INT_TO_JSVAL(N - 1));
 
@@ -102,19 +98,16 @@ BEGIN_TEST(testNewObject_1)
         0,
         JS_PropertyStub, JS_DeletePropertyStub, JS_PropertyStub, JS_StrictPropertyStub,
         JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub, nullptr,
         nullptr, nullptr, constructHook
     };
     JS::RootedObject ctor(cx, JS_NewObject(cx, &cls, JS::NullPtr(), JS::NullPtr()));
     CHECK(ctor);
     JS::RootedValue rt2(cx, OBJECT_TO_JSVAL(ctor));
-    obj = JS_New(cx, ctor, 3, argv);
+    obj = JS_New(cx, ctor, 3, argv.start());
     CHECK(obj);
     CHECK(JS_GetElement(cx, ctor, 0, &v));
     CHECK_SAME(v, JSVAL_ZERO);
 
-    JS_RemoveValueRoot(cx, &argv[0]);
-    JS_RemoveValueRoot(cx, &argv[1]);
-
     return true;
 }
 END_TEST(testNewObject_1)
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -178,17 +178,18 @@ class AutoStringRooter : private AutoGCR
 
     friend void AutoGCRooter::trace(JSTracer *trc);
 
   private:
     JSString *str_;
     MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
-class AutoArrayRooter : private AutoGCRooter {
+class AutoArrayRooter : private AutoGCRooter
+{
   public:
     AutoArrayRooter(JSContext *cx, size_t len, Value *vec
                     MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
       : AutoGCRooter(cx, len), array(vec), skip(cx, array, len)
     {
         MOZ_GUARD_OBJECT_NOTIFIER_INIT;
         JS_ASSERT(tag_ >= 0);
     }
@@ -198,35 +199,48 @@ class AutoArrayRooter : private AutoGCRo
         JS_ASSERT(tag_ >= 0);
     }
 
     void changeArray(Value *newArray, size_t newLength) {
         changeLength(newLength);
         array = newArray;
     }
 
-    Value *array;
-
-    MutableHandleValue handleAt(size_t i)
-    {
+    Value *start() {
+        return array;
+    }
+
+    size_t length() {
+        JS_ASSERT(tag_ >= 0);
+        return size_t(tag_);
+    }
+
+    MutableHandleValue handleAt(size_t i) {
         JS_ASSERT(i < size_t(tag_));
         return MutableHandleValue::fromMarkedLocation(&array[i]);
     }
-    HandleValue handleAt(size_t i) const
-    {
+    HandleValue handleAt(size_t i) const {
+        JS_ASSERT(i < size_t(tag_));
+        return HandleValue::fromMarkedLocation(&array[i]);
+    }
+    MutableHandleValue operator[](size_t i) {
+        JS_ASSERT(i < size_t(tag_));
+        return MutableHandleValue::fromMarkedLocation(&array[i]);
+    }
+    HandleValue operator[](size_t i) const {
         JS_ASSERT(i < size_t(tag_));
         return HandleValue::fromMarkedLocation(&array[i]);
     }
 
     friend void AutoGCRooter::trace(JSTracer *trc);
 
   private:
+    Value *array;
+    js::SkipRoot skip;
     MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
-
-    js::SkipRoot skip;
 };
 
 template<class T>
 class AutoVectorRooter : protected AutoGCRooter
 {
     typedef js::Vector<T, 8> VectorImpl;
     VectorImpl vector;
 
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -915,23 +915,29 @@ class AutoValueArray : public AutoGCRoot
       : AutoGCRooter(cx, VALARRAY), start_(start), length_(length), skip(cx, start, length)
     {
         MOZ_GUARD_OBJECT_NOTIFIER_INIT;
     }
 
     Value *start() { return start_; }
     unsigned length() const { return length_; }
 
-    MutableHandleValue handleAt(unsigned i)
-    {
+    MutableHandleValue handleAt(unsigned i) {
         JS_ASSERT(i < length_);
         return MutableHandleValue::fromMarkedLocation(&start_[i]);
     }
-    HandleValue handleAt(unsigned i) const
-    {
+    HandleValue handleAt(unsigned i) const {
+        JS_ASSERT(i < length_);
+        return HandleValue::fromMarkedLocation(&start_[i]);
+    }
+    MutableHandleValue operator[](unsigned i) {
+        JS_ASSERT(i < length_);
+        return MutableHandleValue::fromMarkedLocation(&start_[i]);
+    }
+    HandleValue operator[](unsigned i) const {
         JS_ASSERT(i < length_);
         return HandleValue::fromMarkedLocation(&start_[i]);
     }
 
     MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
 class AutoObjectObjectHashMap : public AutoHashMapRooter<JSObject *, JSObject *>
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -971,17 +971,17 @@ ScriptedIndirectProxyHandler::get(JSCont
     RootedValue value(cx, StringValue(str));
     Value argv[] = { ObjectOrNullValue(receiver), value };
     AutoValueArray ava(cx, argv, 2);
     RootedValue fval(cx);
     if (!GetDerivedTrap(cx, handler, cx->names().get, &fval))
         return false;
     if (!js_IsCallable(fval))
         return BaseProxyHandler::get(cx, proxy, receiver, id, vp);
-    return Trap(cx, handler, fval, 2, argv, vp);
+    return Trap(cx, handler, fval, 2, ava.start(), vp);
 }
 
 bool
 ScriptedIndirectProxyHandler::set(JSContext *cx, HandleObject proxy, HandleObject receiver,
                                   HandleId id, bool strict, MutableHandleValue vp)
 {
     RootedObject handler(cx, GetIndirectProxyHandlerObject(proxy));
     RootedValue idv(cx, IdToValue(id));
@@ -991,17 +991,17 @@ ScriptedIndirectProxyHandler::set(JSCont
     RootedValue value(cx, StringValue(str));
     Value argv[] = { ObjectOrNullValue(receiver), value, vp.get() };
     AutoValueArray ava(cx, argv, 3);
     RootedValue fval(cx);
     if (!GetDerivedTrap(cx, handler, cx->names().set, &fval))
         return false;
     if (!js_IsCallable(fval))
         return BaseProxyHandler::set(cx, proxy, receiver, id, strict, vp);
-    return Trap(cx, handler, fval, 3, argv, &value);
+    return Trap(cx, handler, fval, 3, ava.start(), &value);
 }
 
 bool
 ScriptedIndirectProxyHandler::keys(JSContext *cx, HandleObject proxy, AutoIdVector &props)
 {
     RootedObject handler(cx, GetIndirectProxyHandlerObject(proxy));
     RootedValue value(cx);
     if (!GetDerivedTrap(cx, handler, cx->names().keys, &value))
--- a/js/src/jsreflect.cpp
+++ b/js/src/jsreflect.cpp
@@ -148,27 +148,26 @@ GetPropertyDefault(JSContext *cx, Handle
  */
 class NodeBuilder
 {
     JSContext   *cx;
     TokenStream *tokenStream;
     bool        saveLoc;               /* save source location information?     */
     char const  *src;                  /* source filename or null               */
     RootedValue srcval;                /* source filename JS value or null      */
-    Value       callbacks[AST_LIMIT];  /* user-specified callbacks              */
-    AutoValueArray callbacksRoots;     /* for rooting |callbacks|               */
+    Value       callbacks_[AST_LIMIT]; /* user-specified callbacks              */
+    AutoValueArray callbacks;          /* for rooting |callbacks|               */
     RootedValue userv;                 /* user-specified builder object or null */
-    RootedValue undefinedVal;          /* a rooted undefined val, used by opt() */
 
   public:
     NodeBuilder(JSContext *c, bool l, char const *s)
         : cx(c), tokenStream(nullptr), saveLoc(l), src(s), srcval(c),
-          callbacksRoots(c, callbacks, AST_LIMIT), userv(c), undefinedVal(c, UndefinedValue())
+          callbacks(c, callbacks_, AST_LIMIT), userv(c)
     {
-        MakeRangeGCSafe(callbacks, mozilla::ArrayLength(callbacks));
+        MakeRangeGCSafe(callbacks.start(), callbacks.length());
     }
 
     bool init(HandleObject userobj = js::NullPtr()) {
         if (src) {
             if (!atomValue(src, &srcval))
                 return false;
         } else {
             srcval.setNull();
@@ -201,17 +200,17 @@ class NodeBuilder
             }
 
             if (!funv.isObject() || !funv.toObject().is<JSFunction>()) {
                 js_ReportValueErrorFlags(cx, JSREPORT_ERROR, JSMSG_NOT_FUNCTION,
                                          JSDVG_SEARCH_STACK, funv, js::NullPtr(), nullptr, nullptr);
                 return false;
             }
 
-            callbacks[i] = funv;
+            callbacks[i].set(funv);
         }
 
         return true;
     }
 
     void setTokenStream(TokenStream *ts) {
         tokenStream = ts;
     }
@@ -307,22 +306,22 @@ class NodeBuilder
         }
 
         Value argv[] = { v1, v2, v3, v4, v5 };
         AutoValueArray ava(cx, argv, 5);
         return Invoke(cx, userv, fun, ArrayLength(argv), argv, dst);
     }
 
     // WARNING: Returning a Handle is non-standard, but it works in this case
-    // because both |v| and |undefinedVal| are definitely rooted on a previous
-    // stack frame (i.e. we're just choosing between two already-rooted
-    // values).
+    // because both |v| and |UndefinedHandleValue| are definitely rooted on a
+    // previous stack frame (i.e. we're just choosing between two
+    // already-rooted values).
     HandleValue opt(HandleValue v) {
         JS_ASSERT_IF(v.isMagic(), v.whyMagic() == JS_SERIALIZE_NO_NODE);
-        return v.isMagic(JS_SERIALIZE_NO_NODE) ? undefinedVal : v;
+        return v.isMagic(JS_SERIALIZE_NO_NODE) ? JS::UndefinedHandleValue : v;
     }
 
     bool atomValue(const char *s, MutableHandleValue dst) {
         /*
          * Bug 575416: instead of Atomize, lookup constant atoms in tbl file
          */
         RootedAtom atom(cx, Atomize(cx, s, strlen(s)));
         if (!atom)