Bug 1298568 - Ensure Scopes that can have data always have non-null data on clone. (r=Waldo)
authorShu-yu Guo <shu@rfrn.org>
Wed, 31 Aug 2016 14:56:29 -0700
changeset 312134 3c3194673109d9e704e9c67f4318043ebf153b92
parent 312133 dec6a438ea4366589d038dd181f3da2904015eac
child 312135 d1194f91ff50d7200e5479272afb215bcb90a05f
push id30632
push userryanvm@gmail.com
push dateThu, 01 Sep 2016 02:33:28 +0000
treeherdermozilla-central@b7f7ae14590a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs1298568
milestone51.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 1298568 - Ensure Scopes that can have data always have non-null data on clone. (r=Waldo)
js/public/GCPolicyAPI.h
js/src/vm/Scope.cpp
js/src/vm/Scope.h
--- a/js/public/GCPolicyAPI.h
+++ b/js/public/GCPolicyAPI.h
@@ -139,18 +139,21 @@ struct GCPolicy<JS::Heap<T>>
 };
 
 // GCPolicy<UniquePtr<T>> forwards the contained pointer to GCPolicy<T>.
 template <typename T, typename D>
 struct GCPolicy<mozilla::UniquePtr<T, D>>
 {
     static mozilla::UniquePtr<T,D> initial() { return mozilla::UniquePtr<T,D>(); }
     static void trace(JSTracer* trc, mozilla::UniquePtr<T,D>* tp, const char* name) {
-        GCPolicy<T>::trace(trc, tp->get(), name);
+        if (tp->get())
+            GCPolicy<T>::trace(trc, tp->get(), name);
     }
     static bool needsSweep(mozilla::UniquePtr<T,D>* tp) {
-        return GCPolicy<T>::needsSweep(tp->get());
+        if (tp->get())
+            return GCPolicy<T>::needsSweep(tp->get());
+        return false;
     }
 };
 
 } // namespace JS
 
 #endif // GCPolicyAPI_h
--- a/js/src/vm/Scope.cpp
+++ b/js/src/vm/Scope.cpp
@@ -269,16 +269,33 @@ Scope::XDRSizedBindingNames(XDRState<mod
 Scope::create(ExclusiveContext* cx, ScopeKind kind, HandleScope enclosing, HandleShape envShape)
 {
     Scope* scope = Allocate<Scope>(cx);
     if (scope)
         new (scope) Scope(kind, enclosing, envShape);
     return scope;
 }
 
+template <typename T, typename D>
+/* static */ Scope*
+Scope::create(ExclusiveContext* cx, ScopeKind kind, HandleScope enclosing,
+              HandleShape envShape, mozilla::UniquePtr<T, D> data)
+{
+    Scope* scope = create(cx, kind, enclosing, envShape);
+    if (!scope)
+        return nullptr;
+
+    // It is an invariant that all Scopes that have data (currently, all
+    // ScopeKinds except With) must have non-null data.
+    MOZ_ASSERT(data);
+    scope->initData(Move(data));
+
+    return scope;
+}
+
 uint32_t
 Scope::chainLength() const
 {
     uint32_t length = 0;
     for (ScopeIter si(const_cast<Scope*>(this)); si; si++)
         length++;
     return length;
 }
@@ -313,71 +330,64 @@ Scope::clone(JSContext* cx, HandleScope 
 {
     RootedShape envShape(cx);
     if (scope->environmentShape()) {
         envShape = scope->maybeCloneEnvironmentShape(cx);
         if (!envShape)
             return nullptr;
     }
 
-    Scope* scopeClone = create(cx, scope->kind_, enclosing, envShape);
-    if (!scopeClone)
-        return nullptr;
-
     switch (scope->kind_) {
       case ScopeKind::Function:
         MOZ_CRASH("Use FunctionScope::clone.");
         break;
 
       case ScopeKind::FunctionBodyVar:
       case ScopeKind::ParameterExpressionVar: {
         Rooted<VarScope::Data*> original(cx, &scope->as<VarScope>().data());
         UniquePtr<VarScope::Data> dataClone = CopyScopeData<VarScope>(cx, original);
         if (!dataClone)
             return nullptr;
-        scopeClone->initData(Move(dataClone));
-        break;
+        return create(cx, scope->kind_, enclosing, envShape, Move(dataClone));
       }
 
       case ScopeKind::Lexical:
       case ScopeKind::Catch:
       case ScopeKind::NamedLambda:
       case ScopeKind::StrictNamedLambda: {
         Rooted<LexicalScope::Data*> original(cx, &scope->as<LexicalScope>().data());
         UniquePtr<LexicalScope::Data> dataClone = CopyScopeData<LexicalScope>(cx, original);
         if (!dataClone)
             return nullptr;
-        scopeClone->initData(Move(dataClone));
-        break;
+        return create(cx, scope->kind_, enclosing, envShape, Move(dataClone));
       }
 
       case ScopeKind::With:
-        break;
+        return create(cx, scope->kind_, enclosing, envShape);
 
       case ScopeKind::Eval:
       case ScopeKind::StrictEval: {
         Rooted<EvalScope::Data*> original(cx, &scope->as<EvalScope>().data());
         UniquePtr<EvalScope::Data> dataClone = CopyScopeData<EvalScope>(cx, original);
         if (!dataClone)
             return nullptr;
-        scopeClone->initData(Move(dataClone));
-        break;
+        return create(cx, scope->kind_, enclosing, envShape, Move(dataClone));
       }
 
       case ScopeKind::Global:
       case ScopeKind::NonSyntactic:
         MOZ_CRASH("Use GlobalScope::clone.");
         break;
 
       case ScopeKind::Module:
         MOZ_CRASH("NYI");
         break;
     }
 
-    return scopeClone;
+    return nullptr;
 }
 
 void
 Scope::finalize(FreeOp* fop)
 {
     if (data_) {
         fop->free_(reinterpret_cast<void*>(data_));
         data_ = 0;
@@ -472,25 +482,21 @@ LexicalScope::create(ExclusiveContext* c
     Rooted<UniquePtr<Data>> copy(cx,
         CopyScopeData<LexicalScope>(cx, bi, data,
                                     &LexicalEnvironmentObject::class_,
                                     BaseShape::NOT_EXTENSIBLE | BaseShape::DELEGATE,
                                     &envShape));
     if (!copy)
         return nullptr;
 
-    Scope* scope = Scope::create(cx, kind, enclosing, envShape);
+    Scope* scope = Scope::create(cx, kind, enclosing, envShape, Move(copy.get()));
     if (!scope)
         return nullptr;
-
     MOZ_ASSERT(scope->as<LexicalScope>().firstFrameSlot() == firstFrameSlot);
-
-    LexicalScope* lexicalScope = &scope->as<LexicalScope>();
-    lexicalScope->initData(Move(copy.get()));
-    return lexicalScope;
+    return &scope->as<LexicalScope>();
 }
 
 /* static */ Shape*
 LexicalScope::getEmptyExtensibleEnvironmentShape(ExclusiveContext* cx)
 {
     const Class* cls = &LexicalEnvironmentObject::class_;
     return EmptyEnvironmentShape(cx, cls, JSSLOT_FREE(cls), BaseShape::DELEGATE);
 }
@@ -769,23 +775,20 @@ VarScope::create(ExclusiveContext* cx, S
     //   - Extensible scopes (i.e., due to direct eval)
     //   - Being a generator
     if (!envShape && needsEnvironment) {
         envShape = getEmptyEnvironmentShape(cx);
         if (!envShape)
             return nullptr;
     }
 
-    Scope* scope = Scope::create(cx, kind, enclosing, envShape);
+    Scope* scope = Scope::create(cx, kind, enclosing, envShape, Move(copy.get()));
     if (!scope)
         return nullptr;
-
-    VarScope* varScope = &scope->as<VarScope>();
-    varScope->initData(Move(copy.get()));
-    return varScope;
+    return &scope->as<VarScope>();
 }
 
 /* static */ Shape*
 VarScope::getEmptyEnvironmentShape(ExclusiveContext* cx)
 {
     const Class* cls = &VarEnvironmentObject::class_;
     return EmptyEnvironmentShape(cx, cls, JSSLOT_FREE(cls), VarScopeEnvShapeFlags);
 }
@@ -875,40 +878,34 @@ GlobalScope::copyData(ExclusiveContext *
 GlobalScope::create(ExclusiveContext* cx, ScopeKind kind, Handle<Data*> data)
 {
     // The data that's passed in may be from the frontend and LifoAlloc'd.
     // Copy it now that we're creating a permanent VM scope.
     Rooted<UniquePtr<Data>> copy(cx, copyData(cx, data));
     if (!copy)
         return nullptr;
 
-    Scope* scope = Scope::create(cx, kind, nullptr, nullptr);
+    Scope* scope = Scope::create(cx, kind, nullptr, nullptr, Move(copy.get()));
     if (!scope)
         return nullptr;
-
-    GlobalScope* globalScope = &scope->as<GlobalScope>();
-    globalScope->initData(Move(copy.get()));
-    return globalScope;
+    return &scope->as<GlobalScope>();
 }
 
 /* static */ GlobalScope*
 GlobalScope::clone(JSContext* cx, Handle<GlobalScope*> scope, ScopeKind kind)
 {
     Rooted<Data*> dataOriginal(cx, &scope->as<GlobalScope>().data());
     Rooted<UniquePtr<Data>> dataClone(cx, CopyScopeData<GlobalScope>(cx, dataOriginal));
     if (!dataClone)
         return nullptr;
 
-    Scope* scopeClone = Scope::create(cx, kind, nullptr, nullptr);
+    Scope* scopeClone = Scope::create(cx, kind, nullptr, nullptr, Move(dataClone.get()));
     if (!scopeClone)
         return nullptr;
-
-    GlobalScope* globalScopeClone = &scopeClone->as<GlobalScope>();
-    globalScopeClone->initData(Move(dataClone.get()));
-    return globalScopeClone;
+    return &scopeClone->as<GlobalScope>();
 }
 
 template <XDRMode mode>
 /* static */ bool
 GlobalScope::XDR(XDRState<mode>* xdr, ScopeKind kind, MutableHandleScope scope)
 {
     MOZ_ASSERT((mode == XDR_DECODE) == !scope);
 
@@ -993,23 +990,20 @@ EvalScope::create(ExclusiveContext* cx, 
     // Strict eval and direct eval in parameter expressions always get their own
     // var environment even if there are no bindings.
     if (!envShape && scopeKind == ScopeKind::StrictEval) {
         envShape = getEmptyEnvironmentShape(cx);
         if (!envShape)
             return nullptr;
     }
 
-    Scope* scope = Scope::create(cx, scopeKind, enclosing, envShape);
+    Scope* scope = Scope::create(cx, scopeKind, enclosing, envShape, Move(copy.get()));
     if (!scope)
         return nullptr;
-
-    EvalScope* evalScope = &scope->as<EvalScope>();
-    evalScope->initData(Move(copy.get()));
-    return evalScope;
+    return &scope->as<EvalScope>();
 }
 
 /* static */ Scope*
 EvalScope::nearestVarScopeForDirectEval(Scope* scope)
 {
     for (ScopeIter si(scope); si; si++) {
         switch (si.kind()) {
           case ScopeKind::Function:
--- a/js/src/vm/Scope.h
+++ b/js/src/vm/Scope.h
@@ -200,16 +200,20 @@ class Scope : public js::gc::TenuredCell
         enclosing_(enclosing),
         environmentShape_(environmentShape),
         data_(0)
     { }
 
     static Scope* create(ExclusiveContext* cx, ScopeKind kind, HandleScope enclosing,
                          HandleShape envShape);
 
+    template <typename T, typename D>
+    static Scope* create(ExclusiveContext* cx, ScopeKind kind, HandleScope enclosing,
+                         HandleShape envShape, mozilla::UniquePtr<T, D> data);
+
     template <typename ConcreteScope, XDRMode mode>
     static bool XDRSizedBindingNames(XDRState<mode>* xdr, Handle<ConcreteScope*> scope,
                                      MutableHandle<typename ConcreteScope::Data*> data);
 
     Shape* maybeCloneEnvironmentShape(JSContext* cx);
 
     template <typename T, typename D>
     void initData(mozilla::UniquePtr<T, D> data) {