Bug 1585921 - Use root marking functions to trace unbarriered pointers in GCPolicy traits since this is only safe when we're marking roots r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 09 Oct 2019 10:30:02 +0000
changeset 496956 bfbd9f2b907ccddc9303223e90ff0bd0d7a533aa
parent 496955 71e0aaf161bf0b61cd3d3eee9ea988956f4629fd
child 496957 754da41ac0f741d3a701b1866325f97ae21a5ef0
push id97554
push userjcoppeard@mozilla.com
push dateWed, 09 Oct 2019 10:56:10 +0000
treeherderautoland@754da41ac0f7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1585921
milestone71.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 1585921 - Use root marking functions to trace unbarriered pointers in GCPolicy traits since this is only safe when we're marking roots r=sfink The root marking functions have assertions that will catch this being used outside of heap marking. Differential Revision: https://phabricator.services.mozilla.com/D48534
js/public/GCPolicyAPI.h
js/public/Id.h
js/public/Value.h
js/src/gc/Policy.h
js/src/vm/HelperThreads.cpp
js/src/vm/Realm.h
js/src/vm/TaggedProto.h
js/src/wasm/WasmJS.h
--- a/js/public/GCPolicyAPI.h
+++ b/js/public/GCPolicyAPI.h
@@ -104,19 +104,19 @@ template <>
 struct GCPolicy<uint64_t> : public IgnoreGCPolicy<uint64_t> {};
 
 template <typename T>
 struct GCPointerPolicy {
   static_assert(mozilla::IsPointer<T>::value,
                 "Non-pointer type not allowed for GCPointerPolicy");
 
   static void trace(JSTracer* trc, T* vp, const char* name) {
-    if (*vp) {
-      js::UnsafeTraceManuallyBarrieredEdge(trc, vp, name);
-    }
+    // It's not safe to trace unbarriered pointers except as part of root
+    // marking.
+    UnsafeTraceRoot(trc, vp, name);
   }
   static bool needsSweep(T* vp) {
     if (*vp) {
       return js::gc::IsAboutToBeFinalizedUnbarriered(vp);
     }
     return false;
   }
   static bool isTenured(T v) { return !js::gc::IsInsideNursery(v); }
--- a/js/public/Id.h
+++ b/js/public/Id.h
@@ -165,17 +165,19 @@ constexpr const jsid JSID_EMPTY = jsid::
 extern JS_PUBLIC_DATA const JS::HandleId JSID_VOIDHANDLE;
 extern JS_PUBLIC_DATA const JS::HandleId JSID_EMPTYHANDLE;
 
 namespace JS {
 
 template <>
 struct GCPolicy<jsid> {
   static void trace(JSTracer* trc, jsid* idp, const char* name) {
-    js::UnsafeTraceManuallyBarrieredEdge(trc, idp, name);
+    // It's not safe to trace unbarriered pointers except as part of root
+    // marking.
+    UnsafeTraceRoot(trc, idp, name);
   }
   static bool isValid(jsid id) {
     return !JSID_IS_GCTHING(id) ||
            js::gc::IsCellPointerValid(JSID_TO_GCTHING(id).asCell());
   }
 };
 
 #ifdef DEBUG
--- a/js/public/Value.h
+++ b/js/public/Value.h
@@ -1072,17 +1072,19 @@ namespace JS {
 JS_PUBLIC_API void HeapValuePostWriteBarrier(Value* valuep, const Value& prev,
                                              const Value& next);
 JS_PUBLIC_API void HeapValueWriteBarriers(Value* valuep, const Value& prev,
                                           const Value& next);
 
 template <>
 struct GCPolicy<JS::Value> {
   static void trace(JSTracer* trc, Value* v, const char* name) {
-    js::UnsafeTraceManuallyBarrieredEdge(trc, v, name);
+    // It's not safe to trace unbarriered pointers except as part of root
+    // marking.
+    UnsafeTraceRoot(trc, v, name);
   }
   static bool isTenured(const Value& thing) {
     return !thing.isGCThing() || !IsInsideNursery(thing.toGCThing());
   }
   static bool isValid(const Value& value) {
     return !value.isGCThing() || js::gc::IsCellPointerValid(value.toGCThing());
   }
 };
--- a/js/src/gc/Policy.h
+++ b/js/src/gc/Policy.h
@@ -39,19 +39,20 @@ struct InternalGCPointerPolicy : public 
     }
   }
   static void readBarrier(T v) {
     if (v) {
       Type::readBarrier(v);
     }
   }
   static void trace(JSTracer* trc, T* vp, const char* name) {
-    if (*vp) {
-      TraceManuallyBarrieredEdge(trc, vp, name);
-    }
+    // It's not safe to trace unbarriered pointers except as part of root
+    // marking. If you get an assertion here you probably need to add a barrier,
+    // e.g. HeapPtr<T>.
+    TraceNullableRoot(trc, vp, name);
   }
 };
 
 }  // namespace js
 
 namespace JS {
 
 // Internally, all pointer types are treated as pointers to GC things by
--- a/js/src/vm/HelperThreads.cpp
+++ b/js/src/vm/HelperThreads.cpp
@@ -527,17 +527,17 @@ void ParseTask::trace(JSTracer* trc) {
   }
 
   Zone* zone = MaybeForwarded(parseGlobal)->zoneFromAnyThread();
   if (zone->usedByHelperThread()) {
     MOZ_ASSERT(!zone->isCollecting());
     return;
   }
 
-  TraceManuallyBarrieredEdge(trc, &parseGlobal, "ParseTask::parseGlobal");
+  TraceRoot(trc, &parseGlobal, "ParseTask::parseGlobal");
   scripts.trace(trc);
   sourceObjects.trace(trc);
 }
 
 size_t ParseTask::sizeOfExcludingThis(
     mozilla::MallocSizeOf mallocSizeOf) const {
   return options.sizeOfExcludingThis(mallocSizeOf) +
          errors.sizeOfExcludingThis(mallocSizeOf);
--- a/js/src/vm/Realm.h
+++ b/js/src/vm/Realm.h
@@ -329,17 +329,18 @@ class JS::Realm : public JS::shadow::Rea
       JSContext* cx);
 
   // The global environment record's [[VarNames]] list that contains all
   // names declared using FunctionDeclaration, GeneratorDeclaration, and
   // VariableDeclaration declarations in global code in this realm.
   // Names are only removed from this list by a |delete IdentifierReference|
   // that successfully removes that global property.
   using VarNamesSet =
-      JS::GCHashSet<JSAtom*, js::DefaultHasher<JSAtom*>, js::ZoneAllocPolicy>;
+      GCHashSet<js::HeapPtr<JSAtom*>, js::DefaultHasher<JSAtom*>,
+                js::ZoneAllocPolicy>;
   VarNamesSet varNames_;
 
   friend class js::AutoSetNewObjectMetadata;
   js::NewObjectMetadataState objectMetadataState_{js::ImmediateMetadata()};
 
   // Random number generator for Math.random().
   mozilla::Maybe<mozilla::non_crypto::XorShift128PlusRNG>
       randomNumberGenerator_;
--- a/js/src/vm/TaggedProto.h
+++ b/js/src/vm/TaggedProto.h
@@ -46,18 +46,20 @@ class TaggedProto {
   }
   bool operator!=(const TaggedProto& other) const {
     return proto != other.proto;
   }
 
   HashNumber hashCode() const;
 
   void trace(JSTracer* trc) {
+    // It's not safe to trace unbarriered pointers except as part of root
+    // marking.
     if (isObject()) {
-      TraceManuallyBarrieredEdge(trc, &proto, "TaggedProto");
+      TraceRoot(trc, &proto, "TaggedProto");
     }
   }
 
  private:
   JSObject* proto;
 };
 
 template <>
--- a/js/src/wasm/WasmJS.h
+++ b/js/src/wasm/WasmJS.h
@@ -276,17 +276,18 @@ class WasmInstanceObject : public Native
   const wasm::CodeRange& getExportedFunctionCodeRange(JSFunction* fun,
                                                       wasm::Tier tier);
 
   static WasmInstanceScope* getScope(JSContext* cx,
                                      HandleWasmInstanceObject instanceObj);
   static WasmFunctionScope* getFunctionScope(
       JSContext* cx, HandleWasmInstanceObject instanceObj, uint32_t funcIndex);
 
-  using GlobalObjectVector = GCVector<WasmGlobalObject*, 0, ZoneAllocPolicy>;
+  using GlobalObjectVector =
+      GCVector<HeapPtr<WasmGlobalObject*>, 0, ZoneAllocPolicy>;
   GlobalObjectVector& indirectGlobals() const;
 };
 
 // The class of WebAssembly.Memory. A WasmMemoryObject references an ArrayBuffer
 // or SharedArrayBuffer object which owns the actual memory.
 
 class WasmMemoryObject : public NativeObject {
   static const unsigned BUFFER_SLOT = 0;