Bug 793823 - Exactly root Bindings when on the stack; r=billm
authorTerrence Cole <terrence@mozilla.com>
Mon, 24 Sep 2012 15:18:34 -0700
changeset 108314 b88bc53d09a20ff4626018ee3013dcb4b2b14793
parent 108313 5c0e68dceca892012ad89952756bd49df7ea217b
child 108315 a7b11eac359f044ea0a14ab09199377caf996c38
push id82
push usershu@rfrn.org
push dateFri, 05 Oct 2012 13:20:22 +0000
reviewersbillm
bugs793823
milestone18.0a1
Bug 793823 - Exactly root Bindings when on the stack; r=billm Currently, we rely on the marking of the origin script to keep the stack binding's data live. This will not work with a moving GC.
js/src/jsgc.cpp
js/src/jspubtd.h
js/src/jsscript.cpp
js/src/jsscript.h
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -954,54 +954,57 @@ InFreeList(ArenaHeader *aheader, uintptr
          * The last possible empty span is an the end of the arena. Here
          * span->end < thing < thingsEnd and so we must have more spans.
          */
         span = span->nextSpan();
     }
 }
 
 #ifdef JSGC_USE_EXACT_ROOTING
+static inline void
+MarkExactStackRooter(JSTracer *trc, Rooted<void*> rooter, ThingRootKind kind)
+{
+    void **addr = (void **)rooter->address();
+    if (!*addr)
+        return;
+
+    switch (kind) {
+      case THING_ROOT_OBJECT:      MarkObjectRoot(trc, (JSObject **)addr, "exact-object"); break;
+      case THING_ROOT_STRING:      MarkStringRoot(trc, (JSSTring **)addr, "exact-string"); break;
+      case THING_ROOT_SCRIPT:      MarkScriptRoot(trc, (JSScript **)addr, "exact-script"); break;
+      case THING_ROOT_SHAPE:       MarkShapeRoot(trc, (Shape **)addr, "exact-shape"); break;
+      case THING_ROOT_BASE_SHAPE:  MarkBaseShapeRoot(trc, (BaseShape **)addr, "exact-baseshape"); break;
+      case THING_ROOT_TYPE:        MarkTypeRoot(trc, (types::Type *)addr, "exact-type"); break;
+      case THING_ROOT_TYPE_OBJECT: MarkTypeObjectRoot(trc, (types::TypeObject **)addr, "exact-typeobject"); break;
+      case THING_ROOT_VALUE:       MarkValueRoot(trc, (Value *)addr, "exact-value"); break;
+      case THING_ROOT_ID:          MarkIdRoot(trc, (jsid *)addr, "exact-id"); break;
+      case THING_ROOT_PROPERTY_ID: MarkIdRoot(trc, &((js::PropertyId *)addr)->asId(), "exact-propertyid"); break;
+      case THING_ROOT_BINDINGS:    ((Bindings *)addr)->trace(trc); break;
+      default: JS_NOT_REACHED("Invalid THING_ROOT kind"); break;
+    }
+}
+
+static inline void
+MarkExactStackRooters(JSTracer *trc, Rooted<void*> rooter, ThingRootKind kind)
+{
+    Rooted<void*> *rooter = cx->thingGCRooters[i];
+    while (rooter) {
+        MarkExactStackRoot(trc, rooter, ThingRootKind(i));
+        rooter = rooter->previous();
+    }
+}
+
 static void
 MarkExactStackRoots(JSTracer *trc)
 {
-    for (ContextIter cx(trc->runtime); !cx.done(); cx.next()) {
-        for (unsigned i = 0; i < THING_ROOT_LIMIT; i++) {
-            Rooted<void*> *rooter = cx->thingGCRooters[i];
-            while (rooter) {
-                void **addr = (void **)rooter->address();
-                if (*addr) {
-                    if (i == THING_ROOT_OBJECT) {
-                        MarkObjectRoot(trc, (JSObject **)addr, "exact stackroot object");
-                    } else if (i == THING_ROOT_STRING) {
-                        MarkStringRoot(trc, (JSString **)addr, "exact stackroot string");
-                    } else if (i == THING_ROOT_ID) {
-                        MarkIdRoot(trc, (jsid *)addr, "exact stackroot id");
-                    } else if (i == THING_ROOT_PROPERTY_ID) {
-                        MarkIdRoot(trc, &((PropertyId *)addr)->asId(), "exact stackroot property id");
-                    } else if (i == THING_ROOT_VALUE) {
-                        MarkValueRoot(trc, (Value *)addr, "exact stackroot value");
-                    } else if (i == THING_ROOT_TYPE) {
-                        MarkTypeRoot(trc, (types::Type *)addr, "exact stackroot type");
-                    } else if (i == THING_ROOT_SHAPE) {
-                        MarkShapeRoot(trc, (Shape **)addr, "exact stackroot shape");
-                    } else if (i == THING_ROOT_BASE_SHAPE) {
-                        MarkBaseShapeRoot(trc, (BaseShape **)addr, "exact stackroot baseshape");
-                    } else if (i == THING_ROOT_TYPE_OBJECT) {
-                        MarkTypeObjectRoot(trc, (types::TypeObject **)addr, "exact stackroot typeobject");
-                    } else if (i == THING_ROOT_SCRIPT) {
-                        MarkScriptRoot(trc, (JSScript **)addr, "exact stackroot script");
-                    } else if (i == THING_ROOT_XML) {
-                        MarkXMLRoot(trc, (JSXML **)addr, "exact stackroot xml");
-                    } else {
-                        JS_NOT_REACHED("Invalid thing root kind.");
-                    }
-                }
-                rooter = rooter->previous();
-            }
+    for (unsigned i = 0; i < THING_ROOT_LIMIT; i++) {
+        for (ContextIter cx(trc->runtime); !cx.done(); cx.next()) {
+            MarkExactStackRooters(trc, cx->thingGCRooters[i], ThingRootKind(i));
         }
+        MarkExactStackRooters(trc, rt->thingGCRooters[i], ThingRootKind(i));
     }
 }
 #endif /* JSGC_USE_EXACT_ROOTING */
 
 enum ConservativeGCTest
 {
     CGCT_VALID,
     CGCT_LOWBITSET, /* excluded because one of the low bits was set */
@@ -4965,21 +4968,31 @@ SetValidateGC(JSContext *cx, bool enable
     rt->gcValidate = enabled;
 }
 
 } /* namespace gc */
 } /* namespace js */
 
 #if defined(DEBUG) && defined(JS_GC_ZEAL) && defined(JSGC_ROOT_ANALYSIS) && !defined(JS_THREADSAFE)
 
+JS_ALWAYS_INLINE bool
+CheckStackRootThing(uintptr_t *w, void *address, ThingRootKind kind)
+{
+    if (kind != THING_ROOT_BINDINGS)
+        return address == static_cast<void*>(w);
+
+    Bindings *bp = static_cast<Bindings*>(address);
+    return w >= (uintptr_t*)bp && w < (uintptr_t*)(bp + 1);
+}
+
 JS_ALWAYS_INLINE void
-CheckStackRootThings(uintptr_t *w, Rooted<void*> *rooter, bool *matched)
+CheckStackRootThings(uintptr_t *w, Rooted<void*> *rooter, ThingRootKind kind, bool *matched)
 {
     while (rooter) {
-        if (rooter->address() == static_cast<void*>(w))
+        if (CheckStackRootThing(w, rooter->address(), kind))
             *matched = true;
         rooter = rooter->previous();
     }
 }
 
 static void
 CheckStackRoot(JSTracer *trc, uintptr_t *w)
 {
@@ -4989,19 +5002,19 @@ CheckStackRoot(JSTracer *trc, uintptr_t 
 #endif
 
     ConservativeGCTest test = MarkIfGCThingWord(trc, *w);
 
     if (test == CGCT_VALID) {
         bool matched = false;
         JSRuntime *rt = trc->runtime;
         for (unsigned i = 0; i < THING_ROOT_LIMIT; i++) {
-            CheckStackRootThings(w, rt->thingGCRooters[i], &matched);
+            CheckStackRootThings(w, rt->thingGCRooters[i], ThingRootKind(i), &matched);
             for (ContextIter cx(rt); !cx.done(); cx.next()) {
-                CheckStackRootThings(w, cx->thingGCRooters[i], &matched);
+                CheckStackRootThings(w, cx->thingGCRooters[i], ThingRootKind(i), &matched);
                 SkipRoot *skip = cx->skipGCRooters;
                 while (skip) {
                     if (skip->contains(reinterpret_cast<uint8_t*>(w), sizeof(w)))
                         matched = true;
                     skip = skip->previous();
                 }
             }
         }
--- a/js/src/jspubtd.h
+++ b/js/src/jspubtd.h
@@ -235,16 +235,17 @@ enum ThingRootKind
     THING_ROOT_TYPE_OBJECT,
     THING_ROOT_STRING,
     THING_ROOT_SCRIPT,
     THING_ROOT_XML,
     THING_ROOT_ID,
     THING_ROOT_PROPERTY_ID,
     THING_ROOT_VALUE,
     THING_ROOT_TYPE,
+    THING_ROOT_BINDINGS,
     THING_ROOT_LIMIT
 };
 
 template <typename T>
 struct RootKind;
 
 /*
  * Specifically mark the ThingRootKind of externally visible types, so that
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -88,19 +88,22 @@ Bindings::initWithTemporaryStorage(JSCon
      * aliased variables. While the debugger may observe any scope object at
      * any time, such accesses are mediated by DebugScopeProxy (see
      * DebugScopeProxy::handleUnaliasedAccess).
      */
 
     JS_STATIC_ASSERT(CallObject::RESERVED_SLOTS == 2);
     gc::AllocKind allocKind = gc::FINALIZE_OBJECT2_BACKGROUND;
     JS_ASSERT(gc::GetGCKindSlots(allocKind) == CallObject::RESERVED_SLOTS);
-    self->callObjShape_ =
+    RootedShape initial(cx,
         EmptyShape::getInitialShape(cx, &CallClass, NULL, cx->global(),
-                                    allocKind, BaseShape::VAROBJ | BaseShape::DELEGATE);
+                                    allocKind, BaseShape::VAROBJ | BaseShape::DELEGATE));
+    if (!initial)
+        return false;
+    self->callObjShape_.init(initial);
 
 #ifdef DEBUG
     HashSet<PropertyName *> added(cx);
     if (!added.init())
         return false;
 #endif
 
     BindingIter bi(*self);
@@ -164,16 +167,22 @@ Bindings::clone(JSContext *cx, InternalH
      * the source's bindingArray directly.
      */
     if (!initWithTemporaryStorage(cx, self, src.numArgs(), src.numVars(), src.bindingArray()))
         return false;
     self->switchToScriptStorage(dstPackedBindings);
     return true;
 }
 
+/* static */ Bindings
+RootMethods<Bindings>::initial()
+{
+    return Bindings();
+}
+
 template<XDRMode mode>
 static bool
 XDRScriptBindings(XDRState<mode> *xdr, LifoAllocScope &las, unsigned numArgs, unsigned numVars,
                   HandleScript script)
 {
     JSContext *cx = xdr->cx();
 
     if (mode == XDR_ENCODE) {
@@ -2085,19 +2094,19 @@ js::CloneScript(JSContext *cx, HandleObj
                                  nobjects, nregexps, ntrynotes, nconsts);
 
     uint8_t *data = AllocScriptData(cx, size);
     if (!data)
         return NULL;
 
     /* Bindings */
 
-    Bindings bindings;
+    Rooted<Bindings> bindings(cx);
     InternalHandle<Bindings*> bindingsHandle =
-        InternalHandle<Bindings*>::fromMarkedLocation(&bindings);
+        InternalHandle<Bindings*>::fromMarkedLocation(bindings.address());
     if (!Bindings::clone(cx, bindingsHandle, data, src))
         return NULL;
 
     /* Objects */
 
     AutoObjectVector objects(cx);
     if (nobjects != 0) {
         HeapPtrObject *vector = src->objects()->vector;
@@ -2605,8 +2614,9 @@ JSScript::formalIsAliased(unsigned argSl
     return bindings.bindingIsAliased(argSlot);
 }
 
 bool
 JSScript::formalLivesInArgumentsObject(unsigned argSlot)
 {
     return argsObjAliasesFormals() && !formalIsAliased(argSlot);
 }
+
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -194,16 +194,25 @@ class Bindings
     bool bindingIsAliased(unsigned bindingIndex);
 
     /* Return whether this scope has any aliased bindings. */
     bool hasAnyAliasedBindings() const { return !callObjShape_->isEmptyShape(); }
 
     void trace(JSTracer *trc);
 };
 
+template <>
+struct RootMethods<Bindings> {
+    static Bindings initial();
+    static ThingRootKind kind() { return THING_ROOT_BINDINGS; }
+    static bool poisoned(const Bindings &bindings) {
+        return IsPoisonedPtr(bindings.callObjShape());
+    }
+};
+
 class ScriptCounts
 {
     friend struct ::JSScript;
     friend struct ScriptAndCounts;
     /*
      * This points to a single block that holds an array of PCCounts followed
      * by an array of doubles.  Each element in the PCCounts array has a
      * pointer into the array of doubles.