From 62970f8ac6d3c9fa8465a8708205078f53d7d1f5 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Wed, 23 May 2018 19:43:36 +0200 Subject: [PATCH] Bug 1461938 part 27 - Use UniquePtr for various compartment pointers. r=jonco --- js/src/builtin/TypedObject.cpp | 11 ++++-- js/src/jsapi-tests/testGCGrayMarking.cpp | 21 +++++++--- js/src/jsfriendapi.cpp | 2 +- js/src/vm/EnvironmentObject.cpp | 26 ++++++------ js/src/vm/EnvironmentObject.h | 8 ---- js/src/vm/JSCompartment.cpp | 50 ++++++++---------------- js/src/vm/JSCompartment.h | 17 ++++---- 7 files changed, 64 insertions(+), 71 deletions(-) diff --git a/js/src/builtin/TypedObject.cpp b/js/src/builtin/TypedObject.cpp index 6f84b4e88f28..051c1ccb82c7 100644 --- a/js/src/builtin/TypedObject.cpp +++ b/js/src/builtin/TypedObject.cpp @@ -1349,7 +1349,7 @@ bool TypedObject::isAttached() const { if (is()) { - ObjectWeakMap* table = compartment()->lazyArrayBuffers; + ObjectWeakMap* table = compartment()->lazyArrayBuffers.get(); if (table) { JSObject* buffer = table->lookup(this); if (buffer) @@ -2143,13 +2143,16 @@ InlineTypedObject::obj_moved(JSObject* dst, JSObject* src) ArrayBufferObject* InlineTransparentTypedObject::getOrCreateBuffer(JSContext* cx) { - ObjectWeakMap*& table = cx->compartment()->lazyArrayBuffers; - if (!table) { - table = cx->new_(cx); + if (!cx->compartment()->lazyArrayBuffers) { + auto table = cx->make_unique(cx); if (!table || !table->init()) return nullptr; + + cx->compartment()->lazyArrayBuffers = Move(table); } + ObjectWeakMap* table = cx->compartment()->lazyArrayBuffers.get(); + JSObject* obj = table->lookup(this); if (obj) return &obj->as(); diff --git a/js/src/jsapi-tests/testGCGrayMarking.cpp b/js/src/jsapi-tests/testGCGrayMarking.cpp index 4a7f6ed98b54..56e0363f19aa 100644 --- a/js/src/jsapi-tests/testGCGrayMarking.cpp +++ b/js/src/jsapi-tests/testGCGrayMarking.cpp @@ -13,19 +13,30 @@ using namespace js; using namespace js::gc; +namespace js { + +struct GCManagedObjectWeakMap : public ObjectWeakMap +{ + using ObjectWeakMap::ObjectWeakMap; +}; + +} // namespace js + namespace JS { template <> -struct DeletePolicy : public js::GCManagedDeletePolicy +struct DeletePolicy + : public js::GCManagedDeletePolicy {}; template <> -struct MapTypeToRootKind { +struct MapTypeToRootKind { static const JS::RootKind kind = JS::RootKind::Traceable; }; template <> -struct GCPolicy : public NonGCPointerPolicy +struct GCPolicy + : public NonGCPointerPolicy {}; } // namespace JS @@ -331,12 +342,12 @@ bool TestUnassociatedWeakMaps() { // Make a weakmap that's not associated with a JSObject. - auto weakMap = cx->make_unique(cx); + auto weakMap = cx->make_unique(cx); CHECK(weakMap); CHECK(weakMap->init()); // Make sure this gets traced during GC. - Rooted rootMap(cx, weakMap.get()); + Rooted rootMap(cx, weakMap.get()); JSObject* key = AllocWeakmapKeyObject(); CHECK(key); diff --git a/js/src/jsfriendapi.cpp b/js/src/jsfriendapi.cpp index 449249b82714..d4ae6914f313 100644 --- a/js/src/jsfriendapi.cpp +++ b/js/src/jsfriendapi.cpp @@ -1425,7 +1425,7 @@ js::SetAllocationMetadataBuilder(JSContext* cx, const AllocationMetadataBuilder* JS_FRIEND_API(JSObject*) js::GetAllocationMetadata(JSObject* obj) { - ObjectWeakMap* map = obj->compartment()->objectMetadataTable; + ObjectWeakMap* map = obj->compartment()->objectMetadataTable.get(); if (map) return map->lookup(obj); return nullptr; diff --git a/js/src/vm/EnvironmentObject.cpp b/js/src/vm/EnvironmentObject.cpp index c11f72e6bccd..74092767d181 100644 --- a/js/src/vm/EnvironmentObject.cpp +++ b/js/src/vm/EnvironmentObject.cpp @@ -2518,7 +2518,7 @@ DebugEnvironments::ensureCompartmentData(JSContext* cx) { JSCompartment* c = cx->compartment(); if (c->debugEnvs) - return c->debugEnvs; + return c->debugEnvs.get(); auto debugEnvs = cx->make_unique(cx, cx->zone()); if (!debugEnvs || !debugEnvs->init()) { @@ -2526,14 +2526,14 @@ DebugEnvironments::ensureCompartmentData(JSContext* cx) return nullptr; } - c->debugEnvs = debugEnvs.release(); - return c->debugEnvs; + c->debugEnvs = Move(debugEnvs); + return c->debugEnvs.get(); } /* static */ DebugEnvironmentProxy* DebugEnvironments::hasDebugEnvironment(JSContext* cx, EnvironmentObject& env) { - DebugEnvironments* envs = env.compartment()->debugEnvs; + DebugEnvironments* envs = env.compartment()->debugEnvs.get(); if (!envs) return nullptr; @@ -2567,7 +2567,7 @@ DebugEnvironments::hasDebugEnvironment(JSContext* cx, const EnvironmentIter& ei) { MOZ_ASSERT(!ei.hasSyntacticEnvironment()); - DebugEnvironments* envs = cx->compartment()->debugEnvs; + DebugEnvironments* envs = cx->compartment()->debugEnvs.get(); if (!envs) return nullptr; @@ -2717,7 +2717,7 @@ DebugEnvironments::onPopCall(JSContext* cx, AbstractFramePtr frame) { assertSameCompartment(cx, frame); - DebugEnvironments* envs = cx->compartment()->debugEnvs; + DebugEnvironments* envs = cx->compartment()->debugEnvs.get(); if (!envs) return; @@ -2759,7 +2759,7 @@ DebugEnvironments::onPopLexical(JSContext* cx, AbstractFramePtr frame, jsbytecod { assertSameCompartment(cx, frame); - DebugEnvironments* envs = cx->compartment()->debugEnvs; + DebugEnvironments* envs = cx->compartment()->debugEnvs.get(); if (!envs) return; @@ -2771,7 +2771,7 @@ template void DebugEnvironments::onPopGeneric(JSContext* cx, const EnvironmentIter& ei) { - DebugEnvironments* envs = cx->compartment()->debugEnvs; + DebugEnvironments* envs = cx->compartment()->debugEnvs.get(); if (!envs) return; @@ -2807,7 +2807,7 @@ DebugEnvironments::onPopVar(JSContext* cx, AbstractFramePtr frame, jsbytecode* p { assertSameCompartment(cx, frame); - DebugEnvironments* envs = cx->compartment()->debugEnvs; + DebugEnvironments* envs = cx->compartment()->debugEnvs.get(); if (!envs) return; @@ -2827,14 +2827,14 @@ DebugEnvironments::onPopVar(JSContext* cx, const EnvironmentIter& ei) void DebugEnvironments::onPopWith(AbstractFramePtr frame) { - if (DebugEnvironments* envs = frame.compartment()->debugEnvs) + if (DebugEnvironments* envs = frame.compartment()->debugEnvs.get()) envs->liveEnvs.remove(&frame.environmentChain()->as()); } void DebugEnvironments::onCompartmentUnsetIsDebuggee(JSCompartment* c) { - if (DebugEnvironments* envs = c->debugEnvs) { + if (DebugEnvironments* envs = c->debugEnvs.get()) { envs->proxiedEnvs.clear(); envs->missingEnvs.clear(); envs->liveEnvs.clear(); @@ -2903,7 +2903,7 @@ DebugEnvironments::updateLiveEnvironments(JSContext* cx) LiveEnvironmentVal* DebugEnvironments::hasLiveEnvironment(EnvironmentObject& env) { - DebugEnvironments* envs = env.compartment()->debugEnvs; + DebugEnvironments* envs = env.compartment()->debugEnvs.get(); if (!envs) return nullptr; @@ -2942,7 +2942,7 @@ DebugEnvironments::unsetPrevUpToDateUntil(JSContext* cx, AbstractFramePtr until) /* static */ void DebugEnvironments::forwardLiveFrame(JSContext* cx, AbstractFramePtr from, AbstractFramePtr to) { - DebugEnvironments* envs = cx->compartment()->debugEnvs; + DebugEnvironments* envs = cx->compartment()->debugEnvs.get(); if (!envs) return; diff --git a/js/src/vm/EnvironmentObject.h b/js/src/vm/EnvironmentObject.h index 4982262174c6..5059523b0ac9 100644 --- a/js/src/vm/EnvironmentObject.h +++ b/js/src/vm/EnvironmentObject.h @@ -1211,12 +1211,4 @@ AnalyzeEntrainedVariables(JSContext* cx, HandleScript script); } // namespace js -namespace JS { - -template <> -struct DeletePolicy : public js::GCManagedDeletePolicy -{}; - -} // namespace JS - #endif /* vm_EnvironmentObject_h */ diff --git a/js/src/vm/JSCompartment.cpp b/js/src/vm/JSCompartment.cpp index 3d57022aec64..840ffe9f62ec 100644 --- a/js/src/vm/JSCompartment.cpp +++ b/js/src/vm/JSCompartment.cpp @@ -48,15 +48,10 @@ JSCompartment::JSCompartment(Zone* zone) regExps(), globalWriteBarriered(0), detachedTypedObjects(0), - objectMetadataTable(nullptr), innerViews(zone), - lazyArrayBuffers(nullptr), - nonSyntacticLexicalEnvironments_(nullptr), gcIncomingGrayPointers(nullptr), validAccessPtr(nullptr), - debugEnvs(nullptr), enumerators(nullptr), - jitCompartment_(nullptr), lcovOutput() { runtime_->numCompartments++; @@ -88,12 +83,7 @@ JSCompartment::~JSCompartment() if (rt->lcovOutput().isEnabled()) rt->lcovOutput().writeLCovResult(lcovOutput); - js_delete(jitCompartment_); - js_delete(debugEnvs); - js_delete(objectMetadataTable); - js_delete(lazyArrayBuffers); - js_delete(nonSyntacticLexicalEnvironments_); - js_free(enumerators); + MOZ_ASSERT(enumerators == iteratorSentinel_.get()); #ifdef DEBUG // Avoid assertion destroying the unboxed layouts list if the embedding @@ -114,6 +104,13 @@ JSCompartment::init(JSContext* maybecx) return false; } + NativeIteratorSentinel sentinel(NativeIterator::allocateSentinel(maybecx)); + if (!sentinel) + return false; + + iteratorSentinel_ = Move(sentinel); + enumerators = iteratorSentinel_.get(); + return true; } @@ -136,10 +133,6 @@ Realm::init(JSContext* maybecx) */ JS::ResetTimeZone(); - enumerators = NativeIterator::allocateSentinel(maybecx); - if (!enumerators) - return false; - if (!savedStacks_.init() || !varNames_.init() || !iteratorCache.init()) @@ -196,18 +189,14 @@ JSCompartment::ensureJitCompartmentExists(JSContext* cx) if (!zone()->getJitZone(cx)) return false; - /* Set the compartment early, so linking works. */ - jitCompartment_ = cx->new_(); - - if (!jitCompartment_) + UniquePtr jitComp = cx->make_unique(); + if (!jitComp) return false; - if (!jitCompartment_->initialize(cx)) { - js_delete(jitCompartment_); - jitCompartment_ = nullptr; + if (!jitComp->initialize(cx)) return false; - } + jitCompartment_ = Move(jitComp); return true; } @@ -537,9 +526,11 @@ LexicalEnvironmentObject* JSCompartment::getOrCreateNonSyntacticLexicalEnvironment(JSContext* cx, HandleObject enclosing) { if (!nonSyntacticLexicalEnvironments_) { - nonSyntacticLexicalEnvironments_ = cx->new_(cx); - if (!nonSyntacticLexicalEnvironments_ || !nonSyntacticLexicalEnvironments_->init()) + auto map = cx->make_unique(cx); + if (!map || !map->init()) return nullptr; + + nonSyntacticLexicalEnvironments_ = Move(map); } // If a wrapped WithEnvironmentObject was passed in, unwrap it, as we may @@ -1028,13 +1019,6 @@ Realm::forgetAllocationMetadataBuilder() allocationMetadataBuilder_ = nullptr; } -void -Realm::clearObjectMetadata() -{ - js_delete(objectMetadataTable); - objectMetadataTable = nullptr; -} - void Realm::setNewObjectMetadata(JSContext* cx, HandleObject obj) { @@ -1044,7 +1028,7 @@ Realm::setNewObjectMetadata(JSContext* cx, HandleObject obj) if (JSObject* metadata = allocationMetadataBuilder_->build(cx, obj, oomUnsafe)) { assertSameCompartment(cx, metadata); if (!objectMetadataTable) { - objectMetadataTable = cx->new_(cx); + objectMetadataTable = cx->make_unique(cx); if (!objectMetadataTable || !objectMetadataTable->init()) oomUnsafe.crash("setNewObjectMetadata"); } diff --git a/js/src/vm/JSCompartment.h b/js/src/vm/JSCompartment.h index 97b6475d8304..3bb150079430 100644 --- a/js/src/vm/JSCompartment.h +++ b/js/src/vm/JSCompartment.h @@ -629,7 +629,7 @@ struct JSCompartment #endif // Keep track of the metadata objects which can be associated with each JS // object. Both keys and values are in this compartment. - js::ObjectWeakMap* objectMetadataTable; + js::UniquePtr objectMetadataTable; // Map from array buffers to views sharing that storage. JS::WeakCache innerViews; @@ -637,7 +637,7 @@ struct JSCompartment // Inline transparent typed objects do not initially have an array buffer, // but can have that buffer created lazily if it is accessed later. This // table manages references from such typed objects to their buffers. - js::ObjectWeakMap* lazyArrayBuffers; + js::UniquePtr lazyArrayBuffers; // All unboxed layouts in the compartment. mozilla::LinkedList unboxedLayouts; @@ -646,7 +646,7 @@ struct JSCompartment // All non-syntactic lexical environments in the compartment. These are kept in // a map because when loading scripts into a non-syntactic environment, we need // to use the same lexical environment to persist lexical bindings. - js::ObjectWeakMap* nonSyntacticLexicalEnvironments_; + js::UniquePtr nonSyntacticLexicalEnvironments_; public: /* @@ -754,12 +754,16 @@ struct JSCompartment } /* Bookkeeping information for debug scope objects. */ - js::DebugEnvironments* debugEnvs; + js::UniquePtr debugEnvs; /* * List of potentially active iterators that may need deleted property * suppression. */ + private: + using NativeIteratorSentinel = js::UniquePtr; + NativeIteratorSentinel iteratorSentinel_; + public: js::NativeIterator* enumerators; MOZ_ALWAYS_INLINE bool objectMaybeInIteration(JSObject* obj); @@ -772,12 +776,12 @@ struct JSCompartment bool maybeAlive = true; protected: - js::jit::JitCompartment* jitCompartment_; + js::UniquePtr jitCompartment_; public: bool ensureJitCompartmentExists(JSContext* cx); js::jit::JitCompartment* jitCompartment() { - return jitCompartment_; + return jitCompartment_.get(); } // Aggregated output used to collect JSScript hit counts when code coverage @@ -1014,7 +1018,6 @@ class JS::Realm : public JSCompartment void setAllocationMetadataBuilder(const js::AllocationMetadataBuilder* builder); void forgetAllocationMetadataBuilder(); void setNewObjectMetadata(JSContext* cx, JS::HandleObject obj); - void clearObjectMetadata(); bool hasObjectPendingMetadata() const { return objectMetadataState_.is();