Bug 734196 - Updating the private pointer should not recurse when marking; r=billm
authorTerrence Cole <terrence@mozilla.com>
Thu, 08 Mar 2012 15:05:21 -0800
changeset 88579 1587745454a6e6af7cc18e7b6609b92ebbcecf40
parent 88578 0681b89e5be98b764dad008eabed2575d005bac8
child 88580 cc6f8e883ab58362fea79431280abf4dc9c0a180
push id22208
push usermak77@bonardo.net
push dateFri, 09 Mar 2012 12:34:50 +0000
treeherdermozilla-central@ead9016b4102 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs734196
milestone13.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 734196 - Updating the private pointer should not recurse when marking; r=billm The private pointer is the only pointer update during GC marking that can invoke unrelated code. If this code triggers a write barrier, then this will recurse. This patch adds a way to update private pointers without triggering a barrier, explicitly for use by the GC during marking.
js/src/jit-test/tests/basic/bug734196.js
js/src/jsapi.cpp
js/src/jsobj.h
js/src/jsobjinlines.h
js/src/jstypedarray.cpp
js/src/vm/Debugger.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug734196.js
@@ -0,0 +1,4 @@
+var x = new ArrayBuffer(2);
+gczeal(4);
+actual = [].concat(x).toString();
+var count2 = countHeap();
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -4266,17 +4266,17 @@ prop_iter_trace(JSTracer *trc, JSObject 
     if (obj->getSlot(JSSLOT_ITER_INDEX).toInt32() < 0) {
         /*
          * Native case: just mark the next property to visit. We don't need a
          * barrier here because the pointer is updated via setPrivate, which
          * always takes a barrier.
          */
         Shape *tmp = (Shape *)pdata;
         MarkShapeUnbarriered(trc, &tmp, "prop iter shape");
-        JS_ASSERT(tmp == pdata);
+        obj->setPrivateUnbarriered(tmp);
     } else {
         /* Non-native case: mark each id in the JSIdArray private. */
         JSIdArray *ida = (JSIdArray *) pdata;
         MarkIdRange(trc, ida->length, ida->vector, "prop iter");
     }
 }
 
 static Class prop_iter_class = {
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -769,16 +769,17 @@ struct JSObject : public js::ObjectImpl
 
     inline js::GlobalObject &global() const;
 
     /* Private data accessors. */
 
     inline bool hasPrivate() const;
     inline void *getPrivate() const;
     inline void setPrivate(void *data);
+    inline void setPrivateUnbarriered(void *data);
     inline void initPrivate(void *data);
 
     /* Access private data for an object with a known number of fixed slots. */
     inline void *getPrivate(size_t nfixed) const;
 
     /* N.B. Infallible: NULL means 'no principal', not an error. */
     inline JSPrincipals *principals(JSContext *cx);
 
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -117,16 +117,23 @@ JSObject::setPrivate(void *data)
     void **pprivate = &privateRef(numFixedSlots());
 
     privateWriteBarrierPre(pprivate);
     *pprivate = data;
     privateWriteBarrierPost(pprivate);
 }
 
 inline void
+JSObject::setPrivateUnbarriered(void *data)
+{
+    void **pprivate = &privateRef(numFixedSlots());
+    *pprivate = data;
+}
+
+inline void
 JSObject::initPrivate(void *data)
 {
     privateRef(numFixedSlots()) = data;
 }
 
 inline bool
 JSObject::enumerate(JSContext *cx, JSIterateOp iterop, js::Value *statep, jsid *idp)
 {
--- a/js/src/jstypedarray.cpp
+++ b/js/src/jstypedarray.cpp
@@ -327,17 +327,17 @@ ArrayBuffer::obj_trace(JSTracer *trc, JS
 {
     /*
      * If this object changes, it will get marked via the private data barrier,
      * so it's safe to leave it Unbarriered.
      */
     JSObject *delegate = static_cast<JSObject*>(obj->getPrivate());
     if (delegate) {
         MarkObjectUnbarriered(trc, &delegate, "arraybuffer.delegate");
-        obj->setPrivate(delegate);
+        obj->setPrivateUnbarriered(delegate);
     }
 }
 
 static JSProperty * const PROPERTY_FOUND = reinterpret_cast<JSProperty *>(1);
 
 JSBool
 ArrayBuffer::obj_lookupGeneric(JSContext *cx, JSObject *obj, jsid id,
                                JSObject **objp, JSProperty **propp)
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -3209,17 +3209,17 @@ DebuggerObject_trace(JSTracer *trc, JSOb
 {
     if (!trc->runtime->gcCurrentCompartment) {
         /*
          * There is a barrier on private pointers, so the Unbarriered marking
          * is okay.
          */
         if (JSObject *referent = (JSObject *) obj->getPrivate()) {
             MarkObjectUnbarriered(trc, &referent, "Debugger.Object referent");
-            obj->setPrivate(referent);
+            obj->setPrivateUnbarriered(referent);
         }
     }
 }
 
 Class DebuggerObject_class = {
     "Object",
     JSCLASS_HAS_PRIVATE | JSCLASS_IMPLEMENTS_BARRIERS |
     JSCLASS_HAS_RESERVED_SLOTS(JSSLOT_DEBUGOBJECT_COUNT),
@@ -3853,17 +3853,17 @@ DebuggerEnv_trace(JSTracer *trc, JSObjec
 {
     if (!trc->runtime->gcCurrentCompartment) {
         /*
          * There is a barrier on private pointers, so the Unbarriered marking
          * is okay.
          */
         if (Env *referent = (JSObject *) obj->getPrivate()) {
             MarkObjectUnbarriered(trc, &referent, "Debugger.Environment referent");
-            obj->setPrivate(referent);
+            obj->setPrivateUnbarriered(referent);
         }
     }
 }
 
 Class DebuggerEnv_class = {
     "Environment",
     JSCLASS_HAS_PRIVATE | JSCLASS_IMPLEMENTS_BARRIERS |
     JSCLASS_HAS_RESERVED_SLOTS(JSSLOT_DEBUGENV_COUNT),