Bug 1590694 - Make it possible to use const GC wrappers r=jandem

The main change is to make BarrieredBase::unsafeUnbarrieredForTracing() const and use const_cast to get return the non-const pointer. This makes sense because the GC is allowed to update the pointer used while preseving the logical value of the cell, e.g. when compacting.

Differential Revision: https://phabricator.services.mozilla.com/D50618

--HG--
extra : moz-landing-system : lando

js/src/gc/Barrier.h

@@ -418,7 +418,7 @@ class MOZ_NON_MEMMOVABLE BarrieredBase {
   // instantiation. Friending to the generic template leads to a number of
   // unintended consequences, including template resolution ambiguity and a
   // circular dependency with Tracing.h.
-  T* unsafeUnbarrieredForTracing() { return &value; }
+  T* unsafeUnbarrieredForTracing() const { return const_cast<T*>(&value); }
 };
 
 // Base class for barriered pointer types that intercept only writes.

js/src/gc/Marking.cpp

@@ -497,17 +497,18 @@ template void js::TraceManuallyBarrieredCrossCompartmentEdge<LazyScript*>(
 
 template <typename T>
 void js::TraceCrossCompartmentEdge(JSTracer* trc, JSObject* src,
-                                   WriteBarriered<T>* dst, const char* name) {
+                                   const WriteBarriered<T>* dst,
+                                   const char* name) {
   if (ShouldTraceCrossCompartment(trc, src, dst->get())) {
     TraceEdgeInternal(trc, dst->unsafeUnbarrieredForTracing(), name);
   }
 }
 
-template void js::TraceCrossCompartmentEdge<Value>(JSTracer* trc, JSObject* src,
-                                                   WriteBarriered<Value>* dst,
-                                                   const char* name);
+template void js::TraceCrossCompartmentEdge<Value>(
+    JSTracer* trc, JSObject* src, const WriteBarriered<Value>* dst,
+    const char* name);
 template void js::TraceCrossCompartmentEdge<JSScript*>(
-    JSTracer* trc, JSObject* src, WriteBarriered<JSScript*>* dst,
+    JSTracer* trc, JSObject* src, const WriteBarriered<JSScript*>* dst,
     const char* name);
 
 template <typename T>

js/src/gc/Marking.h

@@ -109,7 +109,7 @@ inline bool IsAboutToBeFinalizedUnbarriered(T* thingp) {
 }
 
 template <typename T>
-inline bool IsAboutToBeFinalized(WriteBarriered<T>* thingp) {
+inline bool IsAboutToBeFinalized(const WriteBarriered<T>* thingp) {
   return IsAboutToBeFinalizedInternal(
       ConvertToBase(thingp->unsafeUnbarrieredForTracing()));
 }

js/src/gc/Tracer.h

@@ -119,7 +119,7 @@ inline void AssertRootMarkingPhase(JSTracer* trc) {}
 // wrapped in the WeakCache<> template to perform the appropriate sweeping.
 
 template <typename T>
-inline void TraceEdge(JSTracer* trc, WriteBarriered<T>* thingp,
+inline void TraceEdge(JSTracer* trc, const WriteBarriered<T>* thingp,
                       const char* name) {
   gc::TraceEdgeInternal(
       trc, gc::ConvertToBase(thingp->unsafeUnbarrieredForTracing()), name);
@@ -134,7 +134,7 @@ inline void TraceEdge(JSTracer* trc, WeakHeapPtr<T>* thingp, const char* name) {
 // tracing.
 
 template <typename T>
-inline void TraceNullableEdge(JSTracer* trc, WriteBarriered<T>* thingp,
+inline void TraceNullableEdge(JSTracer* trc, const WriteBarriered<T>* thingp,
                               const char* name) {
   if (InternalBarrierMethods<T>::isMarkable(thingp->get())) {
     TraceEdge(trc, thingp, name);
@@ -228,7 +228,7 @@ void TraceRootRange(JSTracer* trc, size_t len, T* vec, const char* name) {
 // destination thing is not being GC'd, then the edge will not be traced.
 template <typename T>
 void TraceCrossCompartmentEdge(JSTracer* trc, JSObject* src,
-                               WriteBarriered<T>* dst, const char* name);
+                               const WriteBarriered<T>* dst, const char* name);
 
 // As above but with manual barriers.
 template <typename T>

js/src/jsapi-tests/testGCHeapPostBarriers.cpp

@@ -17,26 +17,29 @@
 
 // A heap-allocated structure containing one of our barriered pointer wrappers
 // to test.
-template <typename W>
+template <typename W, typename T>
 struct TestStruct {
   W wrapper;
-};
-
-// A specialized version for GCPtr that adds a zone() method.
-template <typename T>
-struct TestStruct<js::GCPtr<T>> {
-  js::GCPtr<T> wrapper;
 
   void trace(JSTracer* trc) {
     TraceNullableEdge(trc, &wrapper, "TestStruct::wrapper");
   }
+
+  TestStruct() {}
+  explicit TestStruct(T init) : wrapper(init) {}
 };
 
 // Give the GCPtr version GCManagedDeletePolicy as required.
 namespace JS {
+
 template <typename T>
-struct DeletePolicy<TestStruct<js::GCPtr<T>>>
-    : public js::GCManagedDeletePolicy<TestStruct<js::GCPtr<T>>> {};
+struct DeletePolicy<TestStruct<js::GCPtr<T>, T>>
+    : public js::GCManagedDeletePolicy<TestStruct<js::GCPtr<T>, T>> {};
+
+template <typename T>
+struct DeletePolicy<TestStruct<const js::GCPtr<T>, T>>
+    : public js::GCManagedDeletePolicy<TestStruct<const js::GCPtr<T>, T>> {};
+
 }  // namespace JS
 
 template <typename T>
@@ -77,7 +80,7 @@ BEGIN_TEST(testGCHeapPostBarriers) {
   CHECK(!js::gc::IsInsideNursery(obj.get()));
   JS::RootedObject tenuredObject(cx, obj);
 
-  /* Currently JSObject and JSFunction objects are nursery allocated. */
+  /* JSObject and JSFunction objects are nursery allocated. */
   CHECK(TestHeapPostBarriersForType<JSObject>());
   CHECK(TestHeapPostBarriersForType<JSFunction>());
 
@@ -95,20 +98,49 @@ bool CanAccessObject(JSObject* obj) {
 
 template <typename T>
 bool TestHeapPostBarriersForType() {
-  CHECK((TestHeapPostBarriersForWrapper<T, JS::Heap<T*>>()));
-  CHECK((TestHeapPostBarriersForWrapper<T, js::GCPtr<T*>>()));
-  CHECK((TestHeapPostBarriersForWrapper<T, js::HeapPtr<T*>>()));
+  CHECK((TestHeapPostBarriersForWrapper<JS::Heap, T>()));
+  CHECK((TestHeapPostBarriersForWrapper<js::GCPtr, T>()));
+  CHECK((TestHeapPostBarriersForWrapper<js::HeapPtr, T>()));
   return true;
 }
 
-template <typename T, typename W>
+template <template <typename> class W, typename T>
 bool TestHeapPostBarriersForWrapper() {
-  CHECK((TestHeapPostBarrierUpdate<T, W>()));
-  CHECK((TestHeapPostBarrierInitFailure<T, W>()));
+  CHECK((TestHeapPostBarrierConstruction<W<T*>, T>()));
+  CHECK((TestHeapPostBarrierConstruction<const W<T*>, T>()));
+  CHECK((TestHeapPostBarrierUpdate<W<T*>, T>()));
+  CHECK((TestHeapPostBarrierInitFailure<W<T*>, T>()));
+  CHECK((TestHeapPostBarrierInitFailure<const W<T*>, T>()));
   return true;
 }
 
-template <typename T, typename W>
+template <typename W, typename T>
+bool TestHeapPostBarrierConstruction() {
+  T* initialObj = CreateGCThing<T>(cx);
+  CHECK(initialObj != nullptr);
+  CHECK(js::gc::IsInsideNursery(initialObj));
+  uintptr_t initialObjAsInt = uintptr_t(initialObj);
+
+  {
+    auto testStruct = cx->make_unique<TestStruct<W, T*>>(initialObj);
+    CHECK(testStruct);
+
+    auto& wrapper = testStruct->wrapper;
+    CHECK(wrapper == initialObj);
+
+    cx->minorGC(JS::GCReason::API);
+
+    CHECK(uintptr_t(wrapper.get()) != initialObjAsInt);
+    CHECK(!js::gc::IsInsideNursery(wrapper.get()));
+    CHECK(CanAccessObject(wrapper.get()));
+  }
+
+  cx->minorGC(JS::GCReason::API);
+
+  return true;
+}
+
+template <typename W, typename T>
 bool TestHeapPostBarrierUpdate() {
   // Normal case - allocate a heap object, write a nursery pointer into it and
   // check that it gets updated on minor GC.
@@ -118,35 +150,29 @@ bool TestHeapPostBarrierUpdate() {
   CHECK(js::gc::IsInsideNursery(initialObj));
   uintptr_t initialObjAsInt = uintptr_t(initialObj);
 
-  TestStruct<W>* ptr = nullptr;
-
   {
-    auto testStruct = cx->make_unique<TestStruct<W>>();
+    auto testStruct = cx->make_unique<TestStruct<W, T*>>();
     CHECK(testStruct);
 
-    W& wrapper = testStruct->wrapper;
+    auto& wrapper = testStruct->wrapper;
     CHECK(wrapper.get() == nullptr);
+
     wrapper = initialObj;
     CHECK(wrapper == initialObj);
 
-    ptr = testStruct.release();
+    cx->minorGC(JS::GCReason::API);
+
+    CHECK(uintptr_t(wrapper.get()) != initialObjAsInt);
+    CHECK(!js::gc::IsInsideNursery(wrapper.get()));
+    CHECK(CanAccessObject(wrapper.get()));
   }
 
   cx->minorGC(JS::GCReason::API);
 
-  W& wrapper = ptr->wrapper;
-  CHECK(uintptr_t(wrapper.get()) != initialObjAsInt);
-  CHECK(!js::gc::IsInsideNursery(wrapper.get()));
-  CHECK(CanAccessObject(wrapper.get()));
-
-  JS::DeletePolicy<TestStruct<W>>()(ptr);
-
-  cx->minorGC(JS::GCReason::API);
-
   return true;
 }
 
-template <typename T, typename W>
+template <typename W, typename T>
 bool TestHeapPostBarrierInitFailure() {
   // Failure case - allocate a heap object, write a nursery pointer into it
   // and fail to complete initialization.
@@ -156,12 +182,10 @@ bool TestHeapPostBarrierInitFailure() {
   CHECK(js::gc::IsInsideNursery(initialObj));
 
   {
-    auto testStruct = cx->make_unique<TestStruct<W>>();
+    auto testStruct = cx->make_unique<TestStruct<W, T*>>(initialObj);
     CHECK(testStruct);
 
-    W& wrapper = testStruct->wrapper;
-    CHECK(wrapper.get() == nullptr);
-    wrapper = initialObj;
+    auto& wrapper = testStruct->wrapper;
     CHECK(wrapper == initialObj);
 
     // testStruct deleted here, as if we left this block due to an error.