Bug 780309 - Implement InternalHandle and use it for Bindings. r=terrence
authorSteve Fink <sfink@mozilla.com>
Mon, 10 Sep 2012 11:26:46 -0700
changeset 106822 b1830b933f15845bd19d2e411a911dc2a4644413
parent 106821 de0930a5e8ee11278535b30a766934a6bd559991
child 106823 f917cdb9b3ea5f3bf65f7a9fbc8806d58cb18e58
push id74
push usershu@rfrn.org
push dateTue, 18 Sep 2012 19:23:47 +0000
reviewersterrence
bugs780309
milestone18.0a1
Bug 780309 - Implement InternalHandle and use it for Bindings. r=terrence
js/src/frontend/BytecodeCompiler.cpp
js/src/frontend/Parser.cpp
js/src/frontend/Parser.h
js/src/gc/Root.h
js/src/jsapi.h
js/src/jsgc.cpp
js/src/jsscript.cpp
js/src/jsscript.h
--- a/js/src/frontend/BytecodeCompiler.cpp
+++ b/js/src/frontend/BytecodeCompiler.cpp
@@ -109,17 +109,18 @@ frontend::CompileScript(JSContext *cx, H
     bool savedCallerFun = options.compileAndGo && callerFrame && callerFrame->isFunctionFrame();
     Rooted<JSScript*> script(cx, JSScript::Create(cx, NullPtr(), savedCallerFun,
                                                   options, staticLevel, ss, 0, length));
     if (!script)
         return NULL;
 
     // Global/eval script bindings are always empty (all names are added to the
     // scope dynamically via JSOP_DEFFUN/VAR).
-    if (!script->bindings.initWithTemporaryStorage(cx, 0, 0, NULL))
+    InternalHandle<Bindings*> bindings(script, &script->bindings);
+    if (!Bindings::initWithTemporaryStorage(cx, bindings, 0, 0, NULL))
         return NULL;
 
     // We can specialize a bit for the given scope chain if that scope chain is the global object.
     JSObject *globalScope = scopeChain && scopeChain == &scopeChain->global() ? (JSObject*) scopeChain : NULL;
     JS_ASSERT_IF(globalScope, globalScope->isNative());
     JS_ASSERT_IF(globalScope, JSCLASS_HAS_GLOBAL_FLAG_AND_SLOTS(globalScope->getClass()));
 
     BytecodeEmitter bce(/* parent = */ NULL, &parser, &sc, script, callerFrame, !!globalScope,
@@ -322,17 +323,18 @@ frontend::CompileFunctionBody(JSContext 
     if (!FoldConstants(cx, pn, &parser))
         return false;
 
     Rooted<JSScript*> script(cx, JSScript::Create(cx, NullPtr(), false, options,
                                                   staticLevel, ss, 0, length));
     if (!script)
         return false;
 
-    if (!funpc.generateFunctionBindings(cx, &script->bindings))
+    InternalHandle<Bindings*> bindings(script, &script->bindings);
+    if (!funpc.generateFunctionBindings(cx, bindings))
         return false;
 
     BytecodeEmitter funbce(/* parent = */ NULL, &parser, &funsc, script, /* callerFrame = */ NULL,
                            /* hasGlobalScope = */ false, options.lineno);
     if (!funbce.init())
         return false;
 
     if (!NameFunctions(cx, pn))
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -291,32 +291,35 @@ AppendPackedBindings(const ParseContext 
                        (pc->sc->bindingsAccessedDynamically() &&
                         pc->decls().lookupFirst(name) == dn);
 
         *dst = Binding(name, kind, aliased);
     }
 }
 
 bool
-ParseContext::generateFunctionBindings(JSContext *cx, Bindings *bindings) const
+ParseContext::generateFunctionBindings(JSContext *cx, InternalHandle<Bindings*> bindings) const
 {
     JS_ASSERT(sc->inFunction());
 
     unsigned count = args_.length() + vars_.length();
     Binding *packedBindings = cx->tempLifoAlloc().newArrayUninitialized<Binding>(count);
     if (!packedBindings) {
         js_ReportOutOfMemory(cx);
         return false;
     }
 
     AppendPackedBindings(this, args_, packedBindings);
     AppendPackedBindings(this, vars_, packedBindings + args_.length());
 
-    if (!bindings->initWithTemporaryStorage(cx, args_.length(), vars_.length(), packedBindings))
+    if (!Bindings::initWithTemporaryStorage(cx, bindings, args_.length(), vars_.length(),
+                                            packedBindings))
+    {
         return false;
+    }
 
     if (bindings->hasAnyAliasedBindings() || sc->funHasExtensibleScope())
         sc->funbox()->fun()->flags |= JSFUN_HEAVYWEIGHT;
 
     return true;
 }
 
 Parser::Parser(JSContext *cx, const CompileOptions &options,
@@ -1252,17 +1255,19 @@ LeaveFunction(ParseNode *fn, Parser *par
                 outer_dn->pn_dflags |= dn->pn_dflags & ~PND_PLACEHOLDER;
             }
 
             /* Mark the outer dn as escaping. */
             outer_dn->pn_dflags |= PND_CLOSED;
         }
     }
 
-    if (!funpc->generateFunctionBindings(cx, &funbox->bindings))
+    InternalHandle<Bindings*> bindings =
+        InternalHandle<Bindings*>::fromMarkedLocation(&funbox->bindings);
+    if (!funpc->generateFunctionBindings(cx, bindings))
         return false;
 
     funpc->lexdeps.releaseMap(cx);
     return true;
 }
 
 /*
  * DefineArg is called for both the arguments of a regular function definition
--- a/js/src/frontend/Parser.h
+++ b/js/src/frontend/Parser.h
@@ -146,17 +146,17 @@ struct ParseContext                 /* t
      *    the use/def nodes, but the emitter occasionally needs 'bindings' for
      *    various scope-related queries.
      *  - Bindings provide the initial js::Shape to use when creating a dynamic
      *    scope object (js::CallObject) for the function. This shape is used
      *    during dynamic name lookup.
      *  - Sometimes a script's bindings are accessed at runtime to retrieve the
      *    contents of the lexical scope (e.g., from the debugger).
      */
-    bool generateFunctionBindings(JSContext *cx, Bindings *bindings) const;
+    bool generateFunctionBindings(JSContext *cx, InternalHandle<Bindings*> bindings) const;
 
   public:
     ParseNode       *yieldNode;     /* parse node for a yield expression that might
                                        be an error if we turn out to be inside a
                                        generator expression */
     FunctionBox     *functionList;
 
     // A strict mode error found in this scope or one of its children. It is
--- a/js/src/gc/Root.h
+++ b/js/src/gc/Root.h
@@ -235,16 +235,83 @@ typedef MutableHandle<jsid>         Muta
 /*
  * Raw pointer used as documentation that a parameter does not need to be
  * rooted.
  */
 typedef JSObject *                  RawObject;
 typedef JSString *                  RawString;
 typedef Value                       RawValue;
 
+/*
+ * InternalHandle is a handle to an internal pointer into a gcthing. Use
+ * InternalHandle when you have a pointer to a direct field of a gcthing, or
+ * when you need a parameter type for something that *may* be a pointer to a
+ * direct field of a gcthing.
+ */
+class InternalHandleBase
+{
+  protected:
+    static void * const zeroPointer;
+};
+
+template <typename T>
+class InternalHandle { };
+
+template <typename T>
+class InternalHandle<T*> : public InternalHandleBase
+{
+    void * const *holder;
+    size_t offset;
+
+  public:
+    /*
+     * Create an InternalHandle using a Handle to the gcthing containing the
+     * field in question, and a pointer to the field.
+     */
+    template<typename H>
+    InternalHandle(const Handle<H> &handle, T *field)
+      : holder((void**)handle.address()), offset(uintptr_t(field) - uintptr_t(handle.get()))
+    {
+    }
+
+    /*
+     * Create an InternalHandle to a field within a Rooted<>.
+     */
+    template<typename R>
+    InternalHandle(const Rooted<R> &root, T *field)
+      : holder((void**)root.address()), offset(uintptr_t(field) - uintptr_t(root.get()))
+    {
+    }
+
+    T *get() const { return reinterpret_cast<T*>(uintptr_t(*holder) + offset); }
+
+    const T& operator *() const { return *get(); }
+    T* operator ->() const { return get(); }
+
+    static InternalHandle<T*> fromMarkedLocation(T *fieldPtr) {
+        return InternalHandle(fieldPtr);
+    }
+
+  private:
+    /*
+     * Create an InternalHandle to something that is not a pointer to a
+     * gcthing, and so does not need to be rooted in the first place. Use these
+     * InternalHandles to pass pointers into functions that also need to accept
+     * regular InternalHandles to gcthing fields.
+     *
+     * Make this private to prevent accidental misuse; this is only for
+     * fromMarkedLocation().
+     */
+    InternalHandle(T *field)
+      : holder(reinterpret_cast<void * const *>(&zeroPointer)),
+        offset(uintptr_t(field))
+    {
+    }
+};
+
 extern mozilla::ThreadLocal<JSRuntime *> TlsRuntime;
 
 /*
  * By default, pointers should use the inheritance hierarchy to find their
  * ThingRootKind. Some pointer types are explicitly set in jspubtd.h so that
  * Rooted<T> may be used without the class definition being available.
  */
 template <typename T>
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -1056,17 +1056,16 @@ class JS_PUBLIC_API(AutoGCRooter) {
         IDVECTOR =    -15, /* js::AutoIdVector */
         OBJVECTOR =   -16, /* js::AutoObjectVector */
         STRINGVECTOR =-17, /* js::AutoStringVector */
         SCRIPTVECTOR =-18, /* js::AutoScriptVector */
         PROPDESC =    -19, /* js::PropDesc::AutoRooter */
         SHAPERANGE =  -20, /* js::Shape::Range::AutoRooter */
         STACKSHAPE =  -21, /* js::StackShape::AutoRooter */
         STACKBASESHAPE=-22,/* js::StackBaseShape::AutoRooter */
-        BINDINGS =    -23, /* js::Bindings::AutoRooter */
         GETTERSETTER =-24, /* js::AutoRooterGetterSetter */
         REGEXPSTATICS=-25, /* js::RegExpStatics::AutoRooter */
         NAMEVECTOR =  -26, /* js::AutoNameVector */
         HASHABLEVALUE=-27
     };
 
   private:
     AutoGCRooter ** const stackTop;
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -98,16 +98,18 @@
 #else
 # include <unistd.h>
 #endif
 
 using namespace mozilla;
 using namespace js;
 using namespace js::gc;
 
+void * const JS::InternalHandleBase::zeroPointer = NULL;
+
 namespace js {
 namespace gc {
 
 /*
  * Lower limit after which we limit the heap growth
  */
 const size_t GC_ALLOCATION_THRESHOLD = 30 * 1024 * 1024;
 
@@ -2397,22 +2399,16 @@ AutoGCRooter::trace(JSTracer *trc)
         }
         if ((rooter->base->flags & BaseShape::HAS_SETTER_OBJECT) && rooter->base->rawSetter) {
             MarkObjectRoot(trc, (JSObject**) &rooter->base->rawSetter,
                            "StackBaseShape::AutoRooter setter");
         }
         return;
       }
 
-      case BINDINGS: {
-        Bindings::AutoRooter *rooter = static_cast<Bindings::AutoRooter *>(this);
-        rooter->trace(trc);
-        return;
-      }
-
       case GETTERSETTER: {
         AutoRooterGetterSetter::Inner *rooter = static_cast<AutoRooterGetterSetter::Inner *>(this);
         if ((rooter->attrs & JSPROP_GETTER) && *rooter->pgetter)
             MarkObjectRoot(trc, (JSObject**) rooter->pgetter, "AutoRooterGetterSetter getter");
         if ((rooter->attrs & JSPROP_SETTER) && *rooter->psetter)
             MarkObjectRoot(trc, (JSObject**) rooter->psetter, "AutoRooterGetterSetter setter");
         return;
       }
@@ -2449,22 +2445,16 @@ AutoGCRooter::traceAll(JSTracer *trc)
 void
 Shape::Range::AutoRooter::trace(JSTracer *trc)
 {
     if (r->cursor)
         MarkShapeRoot(trc, const_cast<Shape**>(&r->cursor), "Shape::Range::AutoRooter");
 }
 
 void
-Bindings::AutoRooter::trace(JSTracer *trc)
-{
-    bindings->trace(trc);
-}
-
-void
 RegExpStatics::AutoRooter::trace(JSTracer *trc)
 {
     if (statics->matchPairsInput)
         MarkStringRoot(trc, reinterpret_cast<JSString**>(&statics->matchPairsInput),
                        "RegExpStatics::AutoRooter matchPairsInput");
     if (statics->pendingInput)
         MarkStringRoot(trc, reinterpret_cast<JSString**>(&statics->pendingInput),
                        "RegExpStatics::AutoRooter pendingInput");
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -55,58 +55,60 @@ Bindings::argumentsVarIndex(JSContext *c
     PropertyName *arguments = cx->runtime->atomState.argumentsAtom;
     BindingIter bi(*this);
     while (bi->name() != arguments)
         bi++;
     return bi.frameIndex();
 }
 
 bool
-Bindings::initWithTemporaryStorage(JSContext *cx, unsigned numArgs, unsigned numVars, Binding *bindingArray)
+Bindings::initWithTemporaryStorage(JSContext *cx, InternalHandle<Bindings*> self,
+                                   unsigned numArgs, unsigned numVars,
+                                   Binding *bindingArray)
 {
-    JS_ASSERT(!callObjShape_);
-    JS_ASSERT(bindingArrayAndFlag_ == TEMPORARY_STORAGE_BIT);
+    JS_ASSERT(!self->callObjShape_);
+    JS_ASSERT(self->bindingArrayAndFlag_ == TEMPORARY_STORAGE_BIT);
 
     if (numArgs > UINT16_MAX || numVars > UINT16_MAX) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
-                             numArgs_ > numVars_ ?
+                             self->numArgs_ > self->numVars_ ?
                              JSMSG_TOO_MANY_FUN_ARGS :
                              JSMSG_TOO_MANY_LOCALS);
         return false;
     }
 
     JS_ASSERT(!(uintptr_t(bindingArray) & TEMPORARY_STORAGE_BIT));
-    bindingArrayAndFlag_ = uintptr_t(bindingArray) | TEMPORARY_STORAGE_BIT;
-    numArgs_ = numArgs;
-    numVars_ = numVars;
+    self->bindingArrayAndFlag_ = uintptr_t(bindingArray) | TEMPORARY_STORAGE_BIT;
+    self->numArgs_ = numArgs;
+    self->numVars_ = numVars;
 
     /*
      * Get the initial shape to use when creating CallObjects for this script.
      * Since unaliased variables are, by definition, only accessed by local
      * operations and never through the scope chain, only give shapes to
      * 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);
-    callObjShape_ = EmptyShape::getInitialShape(cx, &CallClass, NULL, cx->global(),
-                                                allocKind, BaseShape::VAROBJ);
+    self->callObjShape_ = EmptyShape::getInitialShape(cx, &CallClass, NULL, cx->global(),
+                                                      allocKind, BaseShape::VAROBJ);
 
 #ifdef DEBUG
     HashSet<PropertyName *> added(cx);
     if (!added.init())
         return false;
 #endif
 
-    BindingIter bi(*this);
+    BindingIter bi(*self);
     unsigned slot = CallObject::RESERVED_SLOTS;
-    for (unsigned i = 0, n = count(); i < n; i++, bi++) {
+    for (unsigned i = 0, n = self->count(); i < n; i++, bi++) {
         if (!bi->aliased())
             continue;
 
 #ifdef DEBUG
         /* The caller ensures no duplicate aliased names. */
         JS_ASSERT(!added.has(bi->name()));
         if (!added.put(bi->name()))
             return false;
@@ -118,18 +120,18 @@ Bindings::initWithTemporaryStorage(JSCon
             return false;
 
         RootedId id(cx, NameToId(bi->name()));
         unsigned attrs = JSPROP_PERMANENT | JSPROP_ENUMERATE |
                          (bi->kind() == CONSTANT ? JSPROP_READONLY : 0);
         unsigned frameIndex = bi.frameIndex();
         StackShape child(nbase, id, slot++, 0, attrs, Shape::HAS_SHORTID, frameIndex);
 
-        callObjShape_ = callObjShape_->getChildBinding(cx, child);
-        if (!callObjShape_)
+        self->callObjShape_ = self->callObjShape_->getChildBinding(cx, child);
+        if (!self->callObjShape_)
             return false;
     }
     JS_ASSERT(!bi);
 
     return true;
 }
 
 uint8_t *
@@ -139,32 +141,34 @@ Bindings::switchToScriptStorage(Binding 
     JS_ASSERT(!(uintptr_t(newBindingArray) & TEMPORARY_STORAGE_BIT));
 
     PodCopy(newBindingArray, bindingArray(), count());
     bindingArrayAndFlag_ = uintptr_t(newBindingArray);
     return reinterpret_cast<uint8_t *>(newBindingArray + count());
 }
 
 bool
-Bindings::clone(JSContext *cx, uint8_t *dstScriptData, HandleScript srcScript)
+Bindings::clone(JSContext *cx, InternalHandle<Bindings*> self,
+                uint8_t *dstScriptData,
+                HandleScript srcScript)
 {
     /* The clone has the same bindingArray_ offset as 'src'. */
     Bindings &src = srcScript->bindings;
     ptrdiff_t off = (uint8_t *)src.bindingArray() - srcScript->data;
     JS_ASSERT(off >= 0);
     JS_ASSERT(off <= (srcScript->code - srcScript->data));
     Binding *dstPackedBindings = (Binding *)(dstScriptData + off);
 
     /*
      * Since atoms are shareable throughout the runtime, we can simply copy
      * the source's bindingArray directly.
      */
-    if (!initWithTemporaryStorage(cx, src.numArgs(), src.numVars(), src.bindingArray()))
+    if (!initWithTemporaryStorage(cx, self, src.numArgs(), src.numVars(), src.bindingArray()))
         return false;
-    switchToScriptStorage(dstPackedBindings);
+    self->switchToScriptStorage(dstPackedBindings);
     return true;
 }
 
 template<XDRMode mode>
 static bool
 XDRScriptBindings(XDRState<mode> *xdr, LifoAllocScope &las, unsigned numArgs, unsigned numVars,
                   HandleScript script)
 {
@@ -205,17 +209,18 @@ XDRScriptBindings(XDRState<mode> *xdr, L
 
             PropertyName *name = atoms[i].toString()->asAtom().asPropertyName();
             BindingKind kind = BindingKind(u8 >> 1);
             bool aliased = bool(u8 & 1);
 
             bindingArray[i] = Binding(name, kind, aliased);
         }
 
-        if (!script->bindings.initWithTemporaryStorage(cx, numArgs, numVars, bindingArray))
+        InternalHandle<Bindings*> bindings(script, &script->bindings);
+        if (!Bindings::initWithTemporaryStorage(cx, bindings, numArgs, numVars, bindingArray))
             return false;
     }
 
     return true;
 }
 
 bool
 Bindings::bindingIsAliased(unsigned bindingIndex)
@@ -2059,18 +2064,19 @@ js::CloneScript(JSContext *cx, HandleObj
 
     uint8_t *data = AllocScriptData(cx, size);
     if (!data)
         return NULL;
 
     /* Bindings */
 
     Bindings bindings;
-    Bindings::AutoRooter bindingRooter(cx, &bindings);
-    if (!bindings.clone(cx, data, src))
+    InternalHandle<Bindings*> bindingsHandle =
+        InternalHandle<Bindings*>::fromMarkedLocation(&bindings);
+    if (!Bindings::clone(cx, bindingsHandle, data, src))
         return NULL;
 
     /* Objects */
 
     AutoObjectVector objects(cx);
     if (nobjects != 0) {
         HeapPtrObject *vector = src->objects()->vector;
         for (unsigned i = 0; i < nobjects; i++) {
@@ -2125,16 +2131,17 @@ js::CloneScript(JSContext *cx, HandleObj
            .setVersion(src->getVersion());
     JSScript *dst = JSScript::Create(cx, enclosingScope, src->savedCallerFun,
                                      options, src->staticLevel,
                                      src->scriptSource(), src->sourceStart, src->sourceEnd);
     if (!dst) {
         js_free(data);
         return NULL;
     }
+    AutoAssertNoGC nogc;
 
     dst->bindings = bindings;
 
     /* This assignment must occur before all the Rebase calls. */
     dst->data = data;
     memcpy(data, src->data, size);
 
     dst->code = Rebase<jsbytecode>(dst, src, src->code);
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -155,24 +155,27 @@ class Bindings
     inline Bindings();
 
     /*
      * Initialize a Bindings with a pointer into temporary storage.
      * bindingArray must have length numArgs+numVars. Before the temporary
      * storage is release, switchToScriptStorage must be called, providing a
      * pointer into the Binding array stored in script->data.
      */
-    bool initWithTemporaryStorage(JSContext *cx, unsigned numArgs, unsigned numVars, Binding *bindingArray);
+    static bool initWithTemporaryStorage(JSContext *cx, InternalHandle<Bindings*> self,
+                                         unsigned numArgs, unsigned numVars,
+                                         Binding *bindingArray);
+
     uint8_t *switchToScriptStorage(Binding *newStorage);
 
     /*
      * Clone srcScript's bindings (as part of js::CloneScript). dstScriptData
      * is the pointer to what will eventually be dstScript->data.
      */
-    bool clone(JSContext *cx, uint8_t *dstScriptData, HandleScript srcScript);
+    static bool clone(JSContext *cx, InternalHandle<Bindings*> self, uint8_t *dstScriptData, HandleScript srcScript);
 
     unsigned numArgs() const { return numArgs_; }
     unsigned numVars() const { return numVars_; }
     unsigned count() const { return numArgs() + numVars(); }
 
     /* Return the initial shape of call objects created for this scope. */
     Shape *callObjShape() const { return callObjShape_; }
 
@@ -181,36 +184,16 @@ class Bindings
 
     /* Return whether the binding at bindingIndex is aliased. */
     bool bindingIsAliased(unsigned bindingIndex);
 
     /* Return whether this scope has any aliased bindings. */
     bool hasAnyAliasedBindings() const { return !callObjShape_->isEmptyShape(); }
 
     void trace(JSTracer *trc);
-    class AutoRooter;
-};
-
-class Bindings::AutoRooter : private AutoGCRooter
-{
-  public:
-    explicit AutoRooter(JSContext *cx, Bindings *bindings_
-                        JS_GUARD_OBJECT_NOTIFIER_PARAM)
-      : AutoGCRooter(cx, BINDINGS), bindings(bindings_), skip(cx, bindings_)
-    {
-        JS_GUARD_OBJECT_NOTIFIER_INIT;
-    }
-
-    friend void AutoGCRooter::trace(JSTracer *trc);
-    void trace(JSTracer *trc);
-
-  private:
-    Bindings *bindings;
-    SkipRoot skip;
-    JS_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
 class ScriptCounts
 {
     friend struct ::JSScript;
     friend struct ScriptAndCounts;
     /*
      * This points to a single block that holds an array of PCCounts followed