Bug 714616: fix write barrier in Array.shift, r=billm
authorDavid Mandelin <dmandelin@mozilla.com>
Mon, 30 Jan 2012 17:13:07 -0800
changeset 87020 47a3904d3523a722725f4f30804478e92b815478
parent 87019 f719c5a97683cde655105608fa3c55be4ee84ad5
child 87021 f123328d6dacc248a608825864ad7437ed847ab6
push id805
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 18:17:35 +0000
treeherdermozilla-aurora@6fb3bf232436 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs714616
milestone12.0a1
Bug 714616: fix write barrier in Array.shift, r=billm
js/src/jit-test/tests/basic/bug714616.js
js/src/jsarray.cpp
js/src/jsobj.h
js/src/jsobjinlines.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug714616.js
@@ -0,0 +1,8 @@
+array1 = new Array();
+size   = 10;
+for (i = 0; i < size; (array1.length)++)
+{
+  array1.push(array1.shift());
+  ++i
+}
+
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -2501,17 +2501,17 @@ mjit::stubs::ArrayShift(VMFrame &f)
     JS_ASSERT(obj->isDenseArray());
 
     /*
      * At this point the length and initialized length have already been
      * decremented and the result fetched, so just shift the array elements
      * themselves.
      */
     uint32_t initlen = obj->getDenseArrayInitializedLength();
-    obj->moveDenseArrayElements(0, 1, initlen);
+    obj->moveDenseArrayElementsUnbarriered(0, 1, initlen);
 }
 #endif /* JS_METHODJIT */
 
 JSBool
 js::array_shift(JSContext *cx, uintN argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     JSObject *obj = ToObject(cx, &args.thisv());
@@ -2528,17 +2528,17 @@ js::array_shift(JSContext *cx, uintN arg
         length--;
 
         if (obj->isDenseArray() && !js_PrototypeHasIndexedProperties(cx, obj) &&
             length < obj->getDenseArrayCapacity() &&
             0 < obj->getDenseArrayInitializedLength()) {
             args.rval() = obj->getDenseArrayElement(0);
             if (args.rval().isMagic(JS_ARRAY_HOLE))
                 args.rval().setUndefined();
-            obj->moveDenseArrayElements(0, 1, length);
+            obj->moveDenseArrayElements(0, 1, obj->getDenseArrayInitializedLength() - 1);
             obj->setDenseArrayInitializedLength(obj->getDenseArrayInitializedLength() - 1);
             obj->setArrayLength(cx, length);
             if (!js_SuppressDeletedProperty(cx, obj, INT_TO_JSID(length)))
                 return JS_FALSE;
             return JS_TRUE;
         }
 
         JSBool hole;
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -1061,16 +1061,17 @@ struct JSObject : js::gc::Cell
     inline const js::Value &getDenseArrayElement(uintN idx);
     inline void setDenseArrayElement(uintN idx, const js::Value &val);
     inline void initDenseArrayElement(uintN idx, const js::Value &val);
     inline void setDenseArrayElementWithType(JSContext *cx, uintN idx, const js::Value &val);
     inline void initDenseArrayElementWithType(JSContext *cx, uintN idx, const js::Value &val);
     inline void copyDenseArrayElements(uintN dstStart, const js::Value *src, uintN count);
     inline void initDenseArrayElements(uintN dstStart, const js::Value *src, uintN count);
     inline void moveDenseArrayElements(uintN dstStart, uintN srcStart, uintN count);
+    inline void moveDenseArrayElementsUnbarriered(uintN dstStart, uintN srcStart, uintN count);
     inline bool denseArrayHasInlineSlots() const;
 
     /* Packed information for this array. */
     inline void markDenseArrayNotPacked(JSContext *cx);
 
     /*
      * ensureDenseArrayElements ensures that the dense array can hold at least
      * index + extra elements. It returns ED_OK on success, ED_FAILED on
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -602,17 +602,17 @@ JSObject::initDenseArrayElements(uintN d
     for (unsigned i = 0; i < count; ++i)
         elements[dstStart + i].init(comp, src[i]);
 }
 
 inline void
 JSObject::moveDenseArrayElements(uintN dstStart, uintN srcStart, uintN count)
 {
     JS_ASSERT(dstStart + count <= getDenseArrayCapacity());
-    JS_ASSERT(srcStart + count <= getDenseArrayCapacity());
+    JS_ASSERT(srcStart + count <= getDenseArrayInitializedLength());
 
     /*
      * Use a custom write barrier here since it's performance sensitive. We
      * only want to barrier the elements that are being overwritten.
      */
     uintN markStart, markEnd;
     if (dstStart > srcStart) {
         markStart = js::Max(srcStart + count, dstStart);
@@ -621,16 +621,23 @@ JSObject::moveDenseArrayElements(uintN d
         markStart = dstStart;
         markEnd = js::Min(dstStart + count, srcStart);
     }
     prepareElementRangeForOverwrite(markStart, markEnd);
 
     memmove(elements + dstStart, elements + srcStart, count * sizeof(js::Value));
 }
 
+inline void
+JSObject::moveDenseArrayElementsUnbarriered(uintN dstStart, uintN srcStart, uintN count)
+{
+    JS_ASSERT(!compartment()->needsBarrier());
+    memmove(elements + dstStart, elements + srcStart, count * sizeof(js::Value));
+}
+
 inline bool
 JSObject::denseArrayHasInlineSlots() const
 {
     JS_ASSERT(isDenseArray());
     return elements == fixedElements();
 }
 
 namespace js {