Bug 1399944 - Check for valid GC cell pointers in various places r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Tue, 19 Sep 2017 12:31:31 +0100
changeset 667069 74faaba5ecd2fae75df27f3186f9f6eb8d1fa4bc
parent 667068 69536041f010d547faafcc0e9a96468ed68d42c5
child 667070 a23a810d95855a42f009ccd72604935db0680bd3
push id80609
push userbmo:mstriemer@mozilla.com
push dateTue, 19 Sep 2017 17:59:49 +0000
reviewerssfink
bugs1399944
milestone57.0a1
Bug 1399944 - Check for valid GC cell pointers in various places r=sfink
js/public/GCPolicyAPI.h
js/public/GCVariant.h
js/public/HeapAPI.h
js/public/Id.h
js/public/Realm.h
js/public/RootingAPI.h
js/public/Value.h
js/src/gc/Heap.h
js/src/gc/Policy.h
js/src/jit/RematerializedFrame.h
js/src/jsapi-tests/testGCGrayMarking.cpp
js/src/jsgc.cpp
js/src/vm/Scope.h
--- a/js/public/GCPolicyAPI.h
+++ b/js/public/GCPolicyAPI.h
@@ -89,30 +89,35 @@ struct StructGCPolicy
 
     static void sweep(T* tp) {
         return tp->sweep();
     }
 
     static bool needsSweep(T* tp) {
         return tp->needsSweep();
     }
+
+    static bool isValid(const T& tp) {
+        return true;
+    }
 };
 
 // The default GC policy attempts to defer to methods on the underlying type.
 // Most C++ structures that contain a default constructor, a trace function and
 // a sweep function will work out of the box with Rooted, Handle, GCVector,
 // and GCHash{Set,Map}.
 template <typename T> struct GCPolicy : public StructGCPolicy<T> {};
 
 // This policy ignores any GC interaction, e.g. for non-GC types.
 template <typename T>
 struct IgnoreGCPolicy {
     static T initial() { return T(); }
     static void trace(JSTracer* trc, T* t, const char* name) {}
     static bool needsSweep(T* v) { return false; }
+    static bool isValid(const T& v) { return true; }
 };
 template <> struct GCPolicy<uint32_t> : public IgnoreGCPolicy<uint32_t> {};
 template <> struct GCPolicy<uint64_t> : public IgnoreGCPolicy<uint64_t> {};
 
 template <typename T>
 struct GCPointerPolicy
 {
     static T initial() { return nullptr; }
@@ -120,25 +125,46 @@ struct GCPointerPolicy
         if (*vp)
             js::UnsafeTraceManuallyBarrieredEdge(trc, vp, name);
     }
     static bool needsSweep(T* vp) {
         if (*vp)
             return js::gc::IsAboutToBeFinalizedUnbarriered(vp);
         return false;
     }
+    static bool isValid(T v) {
+        return js::gc::IsCellPointerValid(v);
+    }
 };
 template <> struct GCPolicy<JS::Symbol*> : public GCPointerPolicy<JS::Symbol*> {};
 template <> struct GCPolicy<JSAtom*> : public GCPointerPolicy<JSAtom*> {};
 template <> struct GCPolicy<JSFunction*> : public GCPointerPolicy<JSFunction*> {};
 template <> struct GCPolicy<JSObject*> : public GCPointerPolicy<JSObject*> {};
 template <> struct GCPolicy<JSScript*> : public GCPointerPolicy<JSScript*> {};
 template <> struct GCPolicy<JSString*> : public GCPointerPolicy<JSString*> {};
 
 template <typename T>
+struct NonGCPointerPolicy
+{
+    static T initial() { return nullptr; }
+    static void trace(JSTracer* trc, T* vp, const char* name) {
+        if (*vp)
+            (*vp)->trace(trc);
+    }
+    static bool needsSweep(T* vp) {
+        if (*vp)
+            return (*vp)->needsSweep();
+        return false;
+    }
+    static bool isValid(T v) {
+        return true;
+    }
+};
+
+template <typename T>
 struct GCPolicy<JS::Heap<T>>
 {
     static void trace(JSTracer* trc, JS::Heap<T>* thingp, const char* name) {
         TraceEdge(trc, thingp, name);
     }
     static bool needsSweep(JS::Heap<T>* thingp) {
         return *thingp && js::gc::EdgeNeedsSweep(thingp);
     }
@@ -153,16 +179,21 @@ struct GCPolicy<mozilla::UniquePtr<T, D>
         if (tp->get())
             GCPolicy<T>::trace(trc, tp->get(), name);
     }
     static bool needsSweep(mozilla::UniquePtr<T,D>* tp) {
         if (tp->get())
             return GCPolicy<T>::needsSweep(tp->get());
         return false;
     }
+    static bool isValid(const mozilla::UniquePtr<T,D>& t) {
+        if (t.get())
+            return GCPolicy<T>::isValid(*t.get());
+        return true;
+    }
 };
 
 // GCPolicy<Maybe<T>> forwards tracing/sweeping to GCPolicy<T*> if
 // when the Maybe<T> is full.
 template <typename T>
 struct GCPolicy<mozilla::Maybe<T>>
 {
     static mozilla::Maybe<T> initial() { return mozilla::Maybe<T>(); }
@@ -170,15 +201,20 @@ struct GCPolicy<mozilla::Maybe<T>>
         if (tp->isSome())
             GCPolicy<T>::trace(trc, tp->ptr(), name);
     }
     static bool needsSweep(mozilla::Maybe<T>* tp) {
         if (tp->isSome())
             return GCPolicy<T>::needsSweep(tp->ptr());
         return false;
     }
+    static bool isValid(const mozilla::Maybe<T>& t) {
+        if (t.isSome())
+            return GCPolicy<T>::isValid(t.ref());
+        return true;
+    }
 };
 
 template <> struct GCPolicy<JS::Realm*>;  // see Realm.h
 
 } // namespace JS
 
 #endif // GCPolicyAPI_h
--- a/js/public/GCVariant.h
+++ b/js/public/GCVariant.h
@@ -113,16 +113,29 @@ struct GCPolicy<mozilla::Variant<Ts...>>
     using Impl = detail::GCVariantImplementation<Ts...>;
 
     // Variants do not provide initial(). They do not have a default initial
     // value and one must be provided.
 
     static void trace(JSTracer* trc, mozilla::Variant<Ts...>* v, const char* name) {
         Impl::trace(trc, v, name);
     }
+
+    static bool isValid(const mozilla::Variant<Ts...>& v) {
+        return v.match(IsValidMatcher());
+    }
+
+  private:
+    struct IsValidMatcher
+    {
+        template<typename T>
+        bool match(T& v) {
+            return GCPolicy<T>::isValid(v);
+        };
+    };
 };
 
 } // namespace JS
 
 namespace js {
 
 template <typename Wrapper, typename... Ts>
 class WrappedPtrOperations<mozilla::Variant<Ts...>, Wrapper>
--- a/js/public/HeapAPI.h
+++ b/js/public/HeapAPI.h
@@ -48,16 +48,17 @@ const size_t ChunkMarkBitmapOffset = 258
 const size_t ChunkMarkBitmapBits = 31744;
 #else
 const size_t ChunkMarkBitmapOffset = 1032352;
 const size_t ChunkMarkBitmapBits = 129024;
 #endif
 const size_t ChunkRuntimeOffset = ChunkSize - sizeof(void*);
 const size_t ChunkTrailerSize = 2 * sizeof(uintptr_t) + sizeof(uint64_t);
 const size_t ChunkLocationOffset = ChunkSize - ChunkTrailerSize;
+const size_t ChunkStoreBufferOffset = ChunkLocationOffset + sizeof(uint64_t);
 const size_t ArenaZoneOffset = sizeof(size_t);
 const size_t ArenaHeaderSize = sizeof(size_t) + 2 * sizeof(uintptr_t) +
                                sizeof(size_t) + sizeof(uintptr_t);
 
 /*
  * Live objects are marked black or gray. Everything reachable from a JS root is
  * marked black. Objects marked gray are eligible for cycle collection.
  *
@@ -367,31 +368,62 @@ CellIsMarkedGray(const Cell* cell)
 extern JS_PUBLIC_API(bool)
 CellIsMarkedGrayIfKnown(const Cell* cell);
 
 #ifdef DEBUG
 extern JS_PUBLIC_API(bool)
 CellIsNotGray(const Cell* cell);
 #endif
 
+MOZ_ALWAYS_INLINE ChunkLocation
+GetCellLocation(const void* cell)
+{
+    uintptr_t addr = uintptr_t(cell);
+    addr &= ~js::gc::ChunkMask;
+    addr |= js::gc::ChunkLocationOffset;
+    return *reinterpret_cast<ChunkLocation*>(addr);
+}
+
+MOZ_ALWAYS_INLINE bool
+NurseryCellHasStoreBuffer(const void* cell)
+{
+    uintptr_t addr = uintptr_t(cell);
+    addr &= ~js::gc::ChunkMask;
+    addr |= js::gc::ChunkStoreBufferOffset;
+    return *reinterpret_cast<void**>(addr) != nullptr;
+}
+
 } /* namespace detail */
 
 MOZ_ALWAYS_INLINE bool
 IsInsideNursery(const js::gc::Cell* cell)
 {
     if (!cell)
         return false;
-    uintptr_t addr = uintptr_t(cell);
-    addr &= ~js::gc::ChunkMask;
-    addr |= js::gc::ChunkLocationOffset;
-    auto location = *reinterpret_cast<ChunkLocation*>(addr);
+    auto location = detail::GetCellLocation(cell);
     MOZ_ASSERT(location == ChunkLocation::Nursery || location == ChunkLocation::TenuredHeap);
     return location == ChunkLocation::Nursery;
 }
 
+MOZ_ALWAYS_INLINE bool
+IsCellPointerValid(const void* cell)
+{
+    if (!cell)
+        return true;
+    auto addr = uintptr_t(cell);
+    if (addr < ChunkSize || addr % CellAlignBytes != 0)
+        return false;
+    auto location = detail::GetCellLocation(cell);
+    if (location == ChunkLocation::TenuredHeap)
+        return !!detail::GetGCThingZone(addr);
+    if (location == ChunkLocation::Nursery)
+        return detail::NurseryCellHasStoreBuffer(cell);
+    return false;
+}
+
 } /* namespace gc */
 } /* namespace js */
 
 namespace JS {
 
 static MOZ_ALWAYS_INLINE Zone*
 GetTenuredGCThingZone(GCCellPtr thing)
 {
--- a/js/public/Id.h
+++ b/js/public/Id.h
@@ -166,16 +166,19 @@ namespace JS {
 
 template <>
 struct GCPolicy<jsid>
 {
     static jsid initial() { return JSID_VOID; }
     static void trace(JSTracer* trc, jsid* idp, const char* name) {
         js::UnsafeTraceManuallyBarrieredEdge(trc, idp, name);
     }
+    static bool isValid(jsid id) {
+        return !JSID_IS_GCTHING(id) || js::gc::IsCellPointerValid(JSID_TO_GCTHING(id).asCell());
+    }
 };
 
 } // namespace JS
 
 namespace js {
 
 template <>
 struct BarrierMethods<jsid>
--- a/js/public/Realm.h
+++ b/js/public/Realm.h
@@ -23,19 +23,18 @@ JS_PUBLIC_API(void) TraceRealm(JSTracer*
 JS_PUBLIC_API(bool) RealmNeedsSweep(JS::Realm* realm);
 }
 }
 
 namespace JS {
 
 // Each Realm holds a strong reference to its GlobalObject, and vice versa.
 template <>
-struct GCPolicy<Realm*>
+struct GCPolicy<Realm*> : public NonGCPointerPolicy<Realm*>
 {
-    static Realm* initial() { return nullptr; }
     static void trace(JSTracer* trc, Realm** vp, const char* name) {
         if (*vp)
             ::js::gc::TraceRealm(trc, *vp, name);
     }
     static bool needsSweep(Realm** vp) {
         return *vp && ::js::gc::RealmNeedsSweep(*vp);
     }
 };
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -391,16 +391,17 @@ class TenuredHeap : public js::HeapBase<
         static_assert(sizeof(T) == sizeof(TenuredHeap<T>),
                       "TenuredHeap<T> must be binary compatible with T.");
     }
     explicit TenuredHeap(T p) : bits(0) { setPtr(p); }
     explicit TenuredHeap(const TenuredHeap<T>& p) : bits(0) { setPtr(p.getPtr()); }
 
     void setPtr(T newPtr) {
         MOZ_ASSERT((reinterpret_cast<uintptr_t>(newPtr) & flagsMask) == 0);
+        MOZ_ASSERT(js::gc::IsCellPointerValid(newPtr));
         if (newPtr)
             AssertGCThingMustBeTenured(newPtr);
         bits = (bits & flagsMask) | reinterpret_cast<uintptr_t>(newPtr);
     }
 
     void setFlags(uintptr_t flagsToSet) {
         MOZ_ASSERT((flagsToSet & ~flagsMask) == 0);
         bits |= flagsToSet;
@@ -566,19 +567,21 @@ class MOZ_STACK_CLASS MutableHandle : pu
 
   private:
     // Disallow nullptr for overloading purposes.
     MutableHandle(decltype(nullptr)) = delete;
 
   public:
     void set(const T& v) {
         *ptr = v;
+        MOZ_ASSERT(GCPolicy<T>::isValid(*ptr));
     }
     void set(T&& v) {
         *ptr = mozilla::Move(v);
+        MOZ_ASSERT(GCPolicy<T>::isValid(*ptr));
     }
 
     /*
      * This may be called only if the location of the T is guaranteed
      * to be marked (for some reason other than being a Rooted),
      * e.g., if it is guaranteed to be reachable from an implicit root.
      *
      * Create a MutableHandle from a raw location of a T.
@@ -813,35 +816,38 @@ class MOZ_RAII Rooted : public js::Roote
     {
         registerWithRootLists(rootLists(cx));
     }
 
     template <typename RootingContext, typename S>
     Rooted(const RootingContext& cx, S&& initial)
       : ptr(mozilla::Forward<S>(initial))
     {
+        MOZ_ASSERT(GCPolicy<T>::isValid(ptr));
         registerWithRootLists(rootLists(cx));
     }
 
     ~Rooted() {
         MOZ_ASSERT(*stack == reinterpret_cast<Rooted<void*>*>(this));
         *stack = prev;
     }
 
     Rooted<T>* previous() { return reinterpret_cast<Rooted<T>*>(prev); }
 
     /*
      * This method is public for Rooted so that Codegen.py can use a Rooted
      * interchangeably with a MutableHandleValue.
      */
     void set(const T& value) {
         ptr = value;
+        MOZ_ASSERT(GCPolicy<T>::isValid(ptr));
     }
     void set(T&& value) {
         ptr = mozilla::Move(value);
+        MOZ_ASSERT(GCPolicy<T>::isValid(ptr));
     }
 
     DECLARE_POINTER_CONSTREF_OPS(T);
     DECLARE_POINTER_ASSIGN_OPS(Rooted, T);
     DECLARE_NONPOINTER_ACCESSOR_METHODS(ptr);
     DECLARE_NONPOINTER_MUTABLE_ACCESSOR_METHODS(ptr);
 
   private:
--- a/js/public/Value.h
+++ b/js/public/Value.h
@@ -349,27 +349,27 @@ class MOZ_NON_PARAM alignas(8) Value
     }
 
     double& getDoubleRef() {
         MOZ_ASSERT(isDouble());
         return data.asDouble;
     }
 
     void setString(JSString* str) {
-        MOZ_ASSERT(uintptr_t(str) > 0x1000);
+        MOZ_ASSERT(js::gc::IsCellPointerValid(str));
         data.asBits = bitsFromTagAndPayload(JSVAL_TAG_STRING, PayloadType(str));
     }
 
     void setSymbol(JS::Symbol* sym) {
-        MOZ_ASSERT(uintptr_t(sym) > 0x1000);
+        MOZ_ASSERT(js::gc::IsCellPointerValid(sym));
         data.asBits = bitsFromTagAndPayload(JSVAL_TAG_SYMBOL, PayloadType(sym));
     }
 
     void setObject(JSObject& obj) {
-        MOZ_ASSERT(uintptr_t(&obj) >= 0x1000);
+        MOZ_ASSERT(js::gc::IsCellPointerValid(&obj));
 #if defined(JS_PUNBOX64)
         // VisualStudio cannot contain parenthesized C++ style cast and shift
         // inside decltype in template parameter:
         //   AssertionConditionType<decltype((uintptr_t(x) >> 1))>
         // It throws syntax error.
         MOZ_ASSERT((((uintptr_t)&obj) >> JSVAL_TAG_SHIFT) == 0);
 #endif
         setObjectNoCheck(&obj);
@@ -751,17 +751,17 @@ class MOZ_NON_PARAM alignas(8) Value
     void setPrivateGCThing(js::gc::Cell* cell) {
         MOZ_ASSERT(JS::GCThingTraceKind(cell) != JS::TraceKind::String,
                    "Private GC thing Values must not be strings. Make a StringValue instead.");
         MOZ_ASSERT(JS::GCThingTraceKind(cell) != JS::TraceKind::Symbol,
                    "Private GC thing Values must not be symbols. Make a SymbolValue instead.");
         MOZ_ASSERT(JS::GCThingTraceKind(cell) != JS::TraceKind::Object,
                    "Private GC thing Values must not be objects. Make an ObjectValue instead.");
 
-        MOZ_ASSERT(uintptr_t(cell) > 0x1000);
+        MOZ_ASSERT(js::gc::IsCellPointerValid(cell));
 #if defined(JS_PUNBOX64)
         // VisualStudio cannot contain parenthesized C++ style cast and shift
         // inside decltype in template parameter:
         //   AssertionConditionType<decltype((uintptr_t(x) >> 1))>
         // It throws syntax error.
         MOZ_ASSERT((((uintptr_t)cell) >> JSVAL_TAG_SHIFT) == 0);
 #endif
         data.asBits = bitsFromTagAndPayload(JSVAL_TAG_PRIVATE_GCTHING, PayloadType(cell));
@@ -1254,16 +1254,19 @@ struct GCPolicy<JS::Value>
 {
     static Value initial() { return UndefinedValue(); }
     static void trace(JSTracer* trc, Value* v, const char* name) {
         js::UnsafeTraceManuallyBarrieredEdge(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());
+    }
 };
 
 } // namespace JS
 
 namespace js {
 
 template <>
 struct BarrierMethods<JS::Value>
--- a/js/src/gc/Heap.h
+++ b/js/src/gc/Heap.h
@@ -1102,16 +1102,19 @@ static_assert(sizeof(Chunk) == ChunkSize
 static_assert(js::gc::ChunkMarkBitmapOffset == offsetof(Chunk, bitmap),
               "The hardcoded API bitmap offset must match the actual offset.");
 static_assert(js::gc::ChunkRuntimeOffset == offsetof(Chunk, trailer) +
                                             offsetof(ChunkTrailer, runtime),
               "The hardcoded API runtime offset must match the actual offset.");
 static_assert(js::gc::ChunkLocationOffset == offsetof(Chunk, trailer) +
                                              offsetof(ChunkTrailer, location),
               "The hardcoded API location offset must match the actual offset.");
+static_assert(js::gc::ChunkStoreBufferOffset == offsetof(Chunk, trailer) +
+                                                offsetof(ChunkTrailer, storeBuffer),
+              "The hardcoded API storeBuffer offset must match the actual offset.");
 
 /*
  * Tracks the used sizes for owned heap data and automatically maintains the
  * memory usage relationship between GCRuntime and Zones.
  */
 class HeapUsage
 {
     /*
--- a/js/src/gc/Policy.h
+++ b/js/src/gc/Policy.h
@@ -121,16 +121,19 @@ struct InternalGCPointerPolicy {
     using Type = typename mozilla::RemovePointer<T>::Type;
     static T initial() { return nullptr; }
     static void preBarrier(T v) { Type::writeBarrierPre(v); }
     static void postBarrier(T* vp, T prev, T next) { Type::writeBarrierPost(vp, prev, next); }
     static void readBarrier(T v) { Type::readBarrier(v); }
     static void trace(JSTracer* trc, T* vp, const char* name) {
         TraceManuallyBarrieredEdge(trc, vp, name);
     }
+    static bool isValid(T v) {
+        return gc::IsCellPointerValid(v);
+    }
 };
 
 } // namespace js
 
 namespace JS {
 
 #define DEFINE_INTERNAL_GC_POLICY(type) \
     template <> struct GCPolicy<type> : public js::InternalGCPointerPolicy<type> {};
--- a/js/src/jit/RematerializedFrame.h
+++ b/js/src/jit/RematerializedFrame.h
@@ -254,22 +254,14 @@ namespace JS {
 template <>
 struct MapTypeToRootKind<js::jit::RematerializedFrame*>
 {
     static const RootKind kind = RootKind::Traceable;
 };
 
 template <>
 struct GCPolicy<js::jit::RematerializedFrame*>
-{
-    static js::jit::RematerializedFrame* initial() {
-        return nullptr;
-    }
-
-    static void trace(JSTracer* trc, js::jit::RematerializedFrame** frame, const char* name) {
-        if (*frame)
-            (*frame)->trace(trc);
-    }
-};
+  : public NonGCPointerPolicy<js::jit::RematerializedFrame*>
+{};
 
 } // namespace JS
 
 #endif // jit_RematerializedFrame_h
--- a/js/src/jsapi-tests/testGCGrayMarking.cpp
+++ b/js/src/jsapi-tests/testGCGrayMarking.cpp
@@ -22,21 +22,18 @@ struct DeletePolicy<js::ObjectWeakMap> :
 {};
 
 template <>
 struct MapTypeToRootKind<js::ObjectWeakMap*> {
     static const JS::RootKind kind = JS::RootKind::Traceable;
 };
 
 template <>
-struct GCPolicy<js::ObjectWeakMap*> {
-    static void trace(JSTracer* trc, js::ObjectWeakMap** tp, const char* name) {
-        (*tp)->trace(trc);
-    }
-};
+struct GCPolicy<js::ObjectWeakMap*> : public NonGCPointerPolicy<js::ObjectWeakMap*>
+{};
 
 } // namespace JS
 
 class AutoNoAnalysisForTest
 {
   public:
     AutoNoAnalysisForTest() {}
 } JS_HAZ_GC_SUPPRESSED;
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -8078,16 +8078,17 @@ JS::AssertGCThingIsNotAnObjectSubclass(C
 {
     MOZ_ASSERT(cell);
     MOZ_ASSERT(cell->getTraceKind() != JS::TraceKind::Object);
 }
 
 JS_FRIEND_API(void)
 js::gc::AssertGCThingHasType(js::gc::Cell* cell, JS::TraceKind kind)
 {
+    MOZ_ASSERT(IsCellPointerValid(cell));
     if (!cell)
         MOZ_ASSERT(kind == JS::TraceKind::Null);
     else if (IsInsideNursery(cell))
         MOZ_ASSERT(kind == JS::TraceKind::Object);
     else
         MOZ_ASSERT(MapAllocToTraceKind(cell->asTenured().getAllocKind()) == kind);
 }
 #endif
--- a/js/src/vm/Scope.h
+++ b/js/src/vm/Scope.h
@@ -1512,27 +1512,17 @@ class MutableWrappedPtrOperations<ScopeI
 
 namespace JS {
 
 template <>
 struct GCPolicy<js::ScopeKind> : public IgnoreGCPolicy<js::ScopeKind>
 { };
 
 template <typename T>
-struct ScopeDataGCPolicy
-{
-    static T initial() {
-        return nullptr;
-    }
-
-    static void trace(JSTracer* trc, T* vp, const char* name) {
-        if (*vp)
-            (*vp)->trace(trc);
-    }
-};
+struct ScopeDataGCPolicy : public NonGCPointerPolicy<T> {};
 
 #define DEFINE_SCOPE_DATA_GCPOLICY(Data)                        \
     template <>                                                 \
     struct MapTypeToRootKind<Data*> {                           \
         static const RootKind kind = RootKind::Traceable;       \
     };                                                          \
     template <>                                                 \
     struct GCPolicy<Data*> : public ScopeDataGCPolicy<Data*>    \