Bug 1358073 - Call constructors when creating and copying scope data. r=shu a=jcristau
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 18 May 2017 19:58:11 -0400
changeset 396433 6f7e81f34df34225cea2c2a48fd85d399dfb01f7
parent 396432 e62e4db3e31d37fc93c4b3b99f450eba48d78a6a
child 396434 b8920ed272589b76cdf291a7ea7f03ec8b7e96a6
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu, jcristau
bugs1358073
milestone54.0
Bug 1358073 - Call constructors when creating and copying scope data. r=shu a=jcristau
js/src/vm/Scope.cpp
--- a/js/src/vm/Scope.cpp
+++ b/js/src/vm/Scope.cpp
@@ -150,24 +150,33 @@ CopyScopeData(JSContext* cx, Handle<type
     uint32_t length = 0;
     ConcreteScope::getDataNamesAndLength(data, &names, &length);
     for (size_t i = 0; i < length; i++) {
         if (JSAtom* name = names[i].name())
             cx->markAtom(name);
     }
 
     size_t dataSize = ConcreteScope::sizeOfData(data->length);
+    size_t headerSize = sizeof(typename ConcreteScope::Data);
+    MOZ_ASSERT(dataSize >= headerSize);
+    size_t extraSize = dataSize - headerSize;
+
     uint8_t* copyBytes = cx->zone()->pod_malloc<uint8_t>(dataSize);
     if (!copyBytes) {
         ReportOutOfMemory(cx);
         return nullptr;
     }
 
-    mozilla::PodCopy<uint8_t>(copyBytes, reinterpret_cast<uint8_t*>(data.get()), dataSize);
     auto dataCopy = reinterpret_cast<typename ConcreteScope::Data*>(copyBytes);
+    new (dataCopy) typename ConcreteScope::Data(*data);
+
+    uint8_t* extra = reinterpret_cast<uint8_t*>(data.get()) + headerSize;
+    uint8_t* extraCopy = copyBytes + headerSize;
+
+    mozilla::PodCopy<uint8_t>(extraCopy, extra, extraSize);
     return UniquePtr<typename ConcreteScope::Data>(dataCopy);
 }
 
 template <typename ConcreteScope>
 static UniquePtr<typename ConcreteScope::Data>
 CopyScopeData(JSContext* cx, BindingIter& bi, Handle<typename ConcreteScope::Data*> data,
               const Class* cls, uint32_t baseShapeFlags, MutableHandleShape envShape)
 {
@@ -196,16 +205,18 @@ CopyScopeData(JSContext* cx, BindingIter
 template <typename ConcreteScope>
 static UniquePtr<typename ConcreteScope::Data>
 NewEmptyScopeData(JSContext* cx, uint32_t length = 0)
 {
     uint8_t* bytes = cx->zone()->pod_calloc<uint8_t>(ConcreteScope::sizeOfData(length));
     if (!bytes)
         ReportOutOfMemory(cx);
     auto data = reinterpret_cast<typename ConcreteScope::Data*>(bytes);
+    if (data)
+        new (data) typename ConcreteScope::Data();
     return UniquePtr<typename ConcreteScope::Data>(data);
 }
 
 static bool
 XDRBindingName(XDRState<XDR_ENCODE>* xdr, BindingName* bindingName)
 {
     JSContext* cx = xdr->cx();
 
@@ -238,16 +249,25 @@ XDRBindingName(XDRState<XDR_DECODE>* xdr
     if (hasAtom && !XDRAtom(xdr, &atom))
         return false;
 
     *bindingName = BindingName(atom, closedOver);
 
     return true;
 }
 
+template <typename ConcreteScopeData>
+static void
+DeleteScopeData(ConcreteScopeData* data)
+{
+    // Some scope Data classes have GCManagedDeletePolicy because then contain
+    // GCPtrs. Dispose of them in the appropriate way.
+    JS::DeletePolicy<ConcreteScopeData>()(data);
+}
+
 template <typename ConcreteScope, XDRMode mode>
 /* static */ bool
 Scope::XDRSizedBindingNames(XDRState<mode>* xdr, Handle<ConcreteScope*> scope,
                             MutableHandle<typename ConcreteScope::Data*> data)
 {
     MOZ_ASSERT(!data);
 
     JSContext* cx = xdr->cx();
@@ -265,17 +285,17 @@ Scope::XDRSizedBindingNames(XDRState<mod
         if (!data)
             return false;
         data->length = length;
     }
 
     for (uint32_t i = 0; i < length; i++) {
         if (!XDRBindingName(xdr, &data->names[i])) {
             if (mode == XDR_DECODE) {
-                js_free(data);
+                DeleteScopeData(data.get());
                 data.set(nullptr);
             }
 
             return false;
         }
     }
 
     return true;
@@ -405,17 +425,20 @@ Scope::clone(JSContext* cx, HandleScope 
     }
 
     return nullptr;
 }
 
 void
 Scope::finalize(FreeOp* fop)
 {
+    MOZ_ASSERT(CurrentThreadIsGCSweeping());
     if (data_) {
+        // We don't need to call the destructors for any GCPtrs in Data because
+        // this only happens during a GC.
         fop->free_(reinterpret_cast<void*>(data_));
         data_ = 0;
     }
 }
 
 size_t
 Scope::sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const
 {
@@ -535,19 +558,19 @@ LexicalScope::XDR(XDRState<mode>* xdr, S
 {
     JSContext* cx = xdr->cx();
 
     Rooted<Data*> data(cx);
     if (!XDRSizedBindingNames<LexicalScope>(xdr, scope.as<LexicalScope>(), &data))
         return false;
 
     {
-        auto freeOnLeave = MakeScopeExit([&data]() {
+        auto deleteOnLeave = MakeScopeExit([&data]() {
             if (mode == XDR_DECODE)
-                js_free(data);
+                DeleteScopeData(data.get());
         });
 
         uint32_t firstFrameSlot;
         uint32_t nextFrameSlot;
         if (mode == XDR_ENCODE) {
             firstFrameSlot = scope->as<LexicalScope>().firstFrameSlot();
             nextFrameSlot = data->nextFrameSlot;
         }
@@ -719,19 +742,19 @@ FunctionScope::XDR(XDRState<mode>* xdr, 
                    MutableHandleScope scope)
 {
     JSContext* cx = xdr->cx();
     Rooted<Data*> data(cx);
     if (!XDRSizedBindingNames<FunctionScope>(xdr, scope.as<FunctionScope>(), &data))
         return false;
 
     {
-        auto freeOnLeave = MakeScopeExit([&data]() {
+        auto deleteOnLeave = MakeScopeExit([&data]() {
             if (mode == XDR_DECODE)
-                js_free(data);
+                DeleteScopeData(data.get());
         });
 
         uint8_t needsEnvironment;
         uint8_t hasParameterExprs;
         uint32_t nextFrameSlot;
         if (mode == XDR_ENCODE) {
             needsEnvironment = scope->hasEnvironment();
             hasParameterExprs = data->hasParameterExprs;
@@ -748,17 +771,17 @@ FunctionScope::XDR(XDRState<mode>* xdr, 
         if (!xdr->codeUint32(&nextFrameSlot))
             return false;
 
         if (mode == XDR_DECODE) {
             if (!data->length) {
                 MOZ_ASSERT(!data->nonPositionalFormalStart);
                 MOZ_ASSERT(!data->varStart);
                 MOZ_ASSERT(!data->nextFrameSlot);
-                js_free(data);
+                DeleteScopeData(data.get());
                 data = nullptr;
             }
 
             scope.set(create(cx, data, hasParameterExprs, needsEnvironment, fun, enclosing));
             if (!scope)
                 return false;
 
             // nextFrameSlot is used only for this correctness check.
@@ -847,19 +870,19 @@ VarScope::XDR(XDRState<mode>* xdr, Scope
               MutableHandleScope scope)
 {
     JSContext* cx = xdr->cx();
     Rooted<Data*> data(cx);
     if (!XDRSizedBindingNames<VarScope>(xdr, scope.as<VarScope>(), &data))
         return false;
 
     {
-        auto freeOnLeave = MakeScopeExit([&data]() {
+        auto deleteOnLeave = MakeScopeExit([&data]() {
             if (mode == XDR_DECODE)
-                js_free(data);
+                DeleteScopeData(data.get());
         });
 
         uint8_t needsEnvironment;
         uint32_t firstFrameSlot;
         uint32_t nextFrameSlot;
         if (mode == XDR_ENCODE) {
             needsEnvironment = scope->hasEnvironment();
             firstFrameSlot = scope->as<VarScope>().firstFrameSlot();
@@ -870,17 +893,17 @@ VarScope::XDR(XDRState<mode>* xdr, Scope
         if (!xdr->codeUint32(&firstFrameSlot))
             return false;
         if (!xdr->codeUint32(&nextFrameSlot))
             return false;
 
         if (mode == XDR_DECODE) {
             if (!data->length) {
                 MOZ_ASSERT(!data->nextFrameSlot);
-                js_free(data);
+                DeleteScopeData(data.get());
                 data = nullptr;
             }
 
             scope.set(create(cx, kind, data, firstFrameSlot, needsEnvironment, enclosing));
             if (!scope)
                 return false;
 
             // nextFrameSlot is used only for this correctness check.
@@ -950,34 +973,34 @@ GlobalScope::XDR(XDRState<mode>* xdr, Sc
     MOZ_ASSERT((mode == XDR_DECODE) == !scope);
 
     JSContext* cx = xdr->cx();
     Rooted<Data*> data(cx);
     if (!XDRSizedBindingNames<GlobalScope>(xdr, scope.as<GlobalScope>(), &data))
         return false;
 
     {
-        auto freeOnLeave = MakeScopeExit([&data]() {
+        auto deleteOnLeave = MakeScopeExit([&data]() {
             if (mode == XDR_DECODE)
-                js_free(data);
+                DeleteScopeData(data.get());
         });
 
         if (!xdr->codeUint32(&data->varStart))
             return false;
         if (!xdr->codeUint32(&data->letStart))
             return false;
         if (!xdr->codeUint32(&data->constStart))
             return false;
 
         if (mode == XDR_DECODE) {
             if (!data->length) {
                 MOZ_ASSERT(!data->varStart);
                 MOZ_ASSERT(!data->letStart);
                 MOZ_ASSERT(!data->constStart);
-                js_free(data);
+                DeleteScopeData(data.get());
                 data = nullptr;
             }
 
             scope.set(create(cx, kind, data));
             if (!scope)
                 return false;
         }
     }
@@ -1073,28 +1096,28 @@ template <XDRMode mode>
 /* static */ bool
 EvalScope::XDR(XDRState<mode>* xdr, ScopeKind kind, HandleScope enclosing,
                MutableHandleScope scope)
 {
     JSContext* cx = xdr->cx();
     Rooted<Data*> data(cx);
 
     {
-        auto freeOnLeave = MakeScopeExit([&data]() {
+        auto deleteOnLeave = MakeScopeExit([&data]() {
             if (mode == XDR_DECODE)
-                js_free(data);
+                DeleteScopeData(data.get());
         });
 
         if (!XDRSizedBindingNames<EvalScope>(xdr, scope.as<EvalScope>(), &data))
             return false;
 
         if (mode == XDR_DECODE) {
             if (!data->length) {
                 MOZ_ASSERT(!data->nextFrameSlot);
-                js_free(data);
+                DeleteScopeData(data.get());
                 data = nullptr;
             }
 
             scope.set(create(cx, kind, data, enclosing));
             if (!scope)
                 return false;
         }
     }