Bug 1284388 - Check shape table consistency in debug builds r=terrence
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 08 Jul 2016 10:17:08 +0100
changeset 304217 78c59d3a3795fc1e981eb9f6ee9f34561549dd26
parent 304216 1450ae4e9e4f953f8317b82fae3eb375c3ca0c3a
child 304218 86d2baff7ec89962f3da863cc2a82e0b078a815d
push id30417
push userkwierso@gmail.com
push dateFri, 08 Jul 2016 21:56:02 +0000
treeherdermozilla-central@fd8ff97bc294 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs1284388
milestone50.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 1284388 - Check shape table consistency in debug builds r=terrence
js/src/gc/Marking.cpp
js/src/vm/Shape.cpp
js/src/vm/Shape.h
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -1037,18 +1037,20 @@ js::GCMarker::eagerlyMarkChildren(Shape*
 {
     MOZ_ASSERT(shape->isMarked(this->markColor()));
     do {
         // Special case: if a base shape has a shape table then all its pointers
         // must point to this shape or an anscestor.  Since these pointers will
         // be traced by this loop they do not need to be traced here as well.
         BaseShape* base = shape->base();
         CheckTraversedEdge(shape, base);
-        if (mark(base))
+        if (mark(base)) {
+            MOZ_ASSERT(base->canSkipMarkingShapeTable(shape));
             base->traceChildrenSkipShapeTable(this);
+        }
 
         traverseEdge(shape, shape->propidRef().get());
 
         // When triggered between slices on belhalf of a barrier, these
         // objects may reside in the nursery, so require an extra check.
         // FIXME: Bug 1157967 - remove the isTenured checks.
         if (shape->hasGetterObject() && shape->getterObject()->isTenured())
             traverseEdge(shape, shape->getterObject());
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -1353,16 +1353,38 @@ BaseShape::traceChildrenSkipShapeTable(J
 
 void
 BaseShape::traceShapeTable(JSTracer* trc)
 {
     if (hasTable())
         table().trace(trc);
 }
 
+#ifdef DEBUG
+bool
+BaseShape::canSkipMarkingShapeTable(Shape* lastShape)
+{
+    // Check that every shape in the shape table will be marked by marking
+    // |lastShape|.
+
+    if (!hasTable())
+        return true;
+
+    uint32_t count = 0;
+    for (Shape::Range<NoGC> r(lastShape); !r.empty(); r.popFront()) {
+        Shape* shape = &r.front();
+        ShapeTable::Entry& entry = table().search<MaybeAdding::NotAdding>(shape->propid());
+        if (entry.isLive())
+            count++;
+    }
+
+    return count == table().entryCount();
+}
+#endif
+
 #ifdef JSGC_HASH_TABLE_CHECKS
 
 void
 JSCompartment::checkBaseShapeTableAfterMovingGC()
 {
     if (!baseShapes.initialized())
         return;
 
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -124,32 +124,34 @@ enum class MaybeAdding { Adding = true, 
 
 /*
  * Shapes use multiplicative hashing, but specialized to
  * minimize footprint.
  */
 class ShapeTable {
   public:
     friend class NativeObject;
+    friend class BaseShape;
     static const uint32_t MIN_ENTRIES   = 11;
 
     class Entry {
         // js::Shape pointer tag bit indicating a collision.
         static const uintptr_t SHAPE_COLLISION = 1;
         static Shape* const SHAPE_REMOVED; // = SHAPE_COLLISION
 
         Shape* shape_;
 
         Entry() = delete;
         Entry(const Entry&) = delete;
         Entry& operator=(const Entry&) = delete;
 
       public:
         bool isFree() const { return shape_ == nullptr; }
         bool isRemoved() const { return shape_ == SHAPE_REMOVED; }
+        bool isLive() const { return !isFree() && !isRemoved(); }
         bool hadCollision() const { return uintptr_t(shape_) & SHAPE_COLLISION; }
 
         void setFree() { shape_ = nullptr; }
         void setRemoved() { shape_ = SHAPE_REMOVED; }
 
         Shape* shape() const {
             return reinterpret_cast<Shape*>(uintptr_t(shape_) & ~SHAPE_COLLISION);
         }
@@ -449,16 +451,20 @@ class BaseShape : public gc::TenuredCell
     /* For JIT usage */
     static inline size_t offsetOfFlags() { return offsetof(BaseShape, flags); }
 
     static const JS::TraceKind TraceKind = JS::TraceKind::BaseShape;
 
     void traceChildren(JSTracer* trc);
     void traceChildrenSkipShapeTable(JSTracer* trc);
 
+#ifdef DEBUG
+    bool canSkipMarkingShapeTable(Shape* lastShape);
+#endif
+
   private:
     static void staticAsserts() {
         JS_STATIC_ASSERT(offsetof(BaseShape, clasp_) == offsetof(js::shadow::BaseShape, clasp_));
         static_assert(sizeof(BaseShape) % gc::CellSize == 0,
                       "Things inheriting from gc::Cell must have a size that's "
                       "a multiple of gc::CellSize");
     }