diff --git a/js/public/RootingAPI.h b/js/public/RootingAPI.h index b29e90f1c6c..53a090ae0b3 100644 --- a/js/public/RootingAPI.h +++ b/js/public/RootingAPI.h @@ -934,7 +934,7 @@ MutableHandle::MutableHandle(PersistentRooted* root) * These roots can be used in heap-allocated data structures, so they are not * associated with any particular JSContext or stack. They are registered with * the JSRuntime itself, without locking, so they require a full JSContext to be - * initialized, not one of its more restricted superclasses. Initialization may + * initialized, not one of its more restricted superclasses. Initialization may * take place on construction, or in two phases if the no-argument constructor * is called followed by init(). * diff --git a/js/src/gc/RootMarking.cpp b/js/src/gc/RootMarking.cpp index 67033a2495a..54476acfb0f 100644 --- a/js/src/gc/RootMarking.cpp +++ b/js/src/gc/RootMarking.cpp @@ -136,7 +136,10 @@ JS_FOR_EACH_TRACEKIND(FINISH_ROOT_LIST) #undef FINISH_ROOT_LIST FinishPersistentRootedChain(heapRoots_[JS::RootKind::Id]); FinishPersistentRootedChain(heapRoots_[JS::RootKind::Value]); - FinishPersistentRootedChain(heapRoots_[JS::RootKind::Traceable]); + + // Note that we do not finalize the Traceable list as we do not know how to + // safely clear memebers. We instead assert that none escape the RootLists. + // See the comment on RootLists::~RootLists for details. } inline void diff --git a/js/src/jsapi-tests/testGCExactRooting.cpp b/js/src/jsapi-tests/testGCExactRooting.cpp index 01bb1fdf08a..f50fb466b6a 100644 --- a/js/src/jsapi-tests/testGCExactRooting.cpp +++ b/js/src/jsapi-tests/testGCExactRooting.cpp @@ -95,7 +95,7 @@ BEGIN_TEST(testGCRootedStaticStructInternalStackStorageAugmented) bool same; // Automatic move from stack to heap. - JS::PersistentRooted heap(cx, container); + JS::PersistentRooted heap(rt, container); // clear prior rooting. container.obj() = nullptr; @@ -125,6 +125,42 @@ BEGIN_TEST(testGCRootedStaticStructInternalStackStorageAugmented) } END_TEST(testGCRootedStaticStructInternalStackStorageAugmented) +static JS::PersistentRooted sLongLived; +BEGIN_TEST(testGCPersistentRootedOutlivesRuntime) +{ + JSContext* cx2 = JS_NewContext(rt, 8192); + CHECK(cx2); + + sLongLived.init(cx2, JS_NewObject(cx, nullptr)); + CHECK(sLongLived); + + JS_DestroyContext(cx2); + CHECK(!sLongLived); + + return true; +} +END_TEST(testGCPersistentRootedOutlivesRuntime) + +// Unlike the above, the following test is an example of an invalid usage: for +// performance and simplicity reasons, PersistentRooted is not +// allowed to outlive the container it belongs to. The following commented out +// test can be used to verify that the relevant assertion fires as expected. +static JS::PersistentRooted sContainer; +BEGIN_TEST(testGCPersistentRootedTraceableCannotOutliveRuntime) +{ + JS::Rooted container(cx); + container.obj() = JS_NewObject(cx, nullptr); + container.str() = JS_NewStringCopyZ(cx, "Hello"); + sContainer.init(rt, container); + + // Commenting the following line will trigger an assertion that the + // PersistentRooted outlives the runtime it is attached to. + sContainer.reset(); + + return true; +} +END_TEST(testGCPersistentRootedTraceableCannotOutliveRuntime) + using MyHashMap = js::GCHashMap; BEGIN_TEST(testGCRootedHashMap) diff --git a/js/src/jspubtd.h b/js/src/jspubtd.h index 5410252a101..b7ded95473b 100644 --- a/js/src/jspubtd.h +++ b/js/src/jspubtd.h @@ -125,9 +125,6 @@ typedef void (* JSTraceDataOp)(JSTracer* trc, void* data); namespace js { - -void FinishGC(JSRuntime* rt); - namespace gc { class AutoTraceSession; class StoreBuffer; @@ -292,6 +289,29 @@ class RootLists } } + ~RootLists() { + // The semantics of PersistentRooted containing pointers and tagged + // pointers are somewhat different from those of PersistentRooted + // containing a structure with a trace method. PersistentRooted + // containing pointers are allowed to outlive the owning RootLists, + // whereas those containing a traceable structure are not. + // + // The purpose of this feature is to support lazy initialization of + // global references for the several places in Gecko that do not have + // access to a tighter context, but that still need to refer to GC + // pointers. For such pointers, FinishPersistentRootedChains ensures + // that the contained references are nulled out when the owning + // RootLists dies to prevent UAF errors. + // + // However, for RootKind::Traceable, we do not know the concrete type + // of the held thing, so we simply cannot do this without accruing + // extra overhead and complexity for all users for a case that is + // unlikely to ever be used in practice. For this reason, the following + // assertion disallows usage of PersistentRooted that + // outlives the RootLists. + MOZ_ASSERT(heapRoots_[JS::RootKind::Traceable].isEmpty()); + } + void traceStackRoots(JSTracer* trc); void checkNoGCRooters();