Bug 1358073 - Call constructors when creating and copying scope data. r=shu
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 18 May 2017 19:58:11 -0400
changeset 359240 df21c5eed7c11b531228a02d66624960b59c7c5e
parent 359239 7828515b45be925c7a5c9cf7a951109abbc55921
child 359241 ea227f8d9ce481676a4f30d1a76bdb1bc7787d2e
push id31850
push userryanvm@gmail.com
push dateFri, 19 May 2017 15:47:16 +0000
treeherdermozilla-central@c800b6dfca67 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu
bugs1358073
milestone55.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 1358073 - Call constructors when creating and copying scope data. r=shu
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);
+                js_delete(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);
+                js_delete(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;
         }
     }