Bug 1148214 - Replace manual AllocKind range checks with a few centralized functions. r=terrence
authorEmanuel Hoogeveen <emanuel.hoogeveen@gmail.com>
Thu, 26 Mar 2015 17:07:00 -0400
changeset 266690 db80ac5a48aaecc13b1b1e869b8d048b51a2af85
parent 266689 8fff872add3102e966afd90517715447fb1afd6d
child 266691 d2f789b621773ae9fa3aa27044d68c67a0993adc
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs1148214
milestone39.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 1148214 - Replace manual AllocKind range checks with a few centralized functions. r=terrence
js/src/gc/Allocator.cpp
js/src/gc/Heap.h
js/src/jit/MacroAssembler.cpp
js/src/jit/VMFunctions.cpp
js/src/jsgc.cpp
js/src/jsgc.h
js/src/jsobj.cpp
js/src/jspropertytree.cpp
--- a/js/src/gc/Allocator.cpp
+++ b/js/src/gc/Allocator.cpp
@@ -94,17 +94,17 @@ GCRuntime::checkIncrementalZoneState(Exc
 }
 
 template <typename T, AllowGC allowGC /* = CanGC */>
 JSObject *
 js::Allocate(ExclusiveContext *cx, AllocKind kind, size_t nDynamicSlots, InitialHeap heap,
              const Class *clasp)
 {
     static_assert(mozilla::IsConvertible<T *, JSObject *>::value, "must be JSObject derived");
-    MOZ_ASSERT(kind <= AllocKind::OBJECT_LAST);
+    MOZ_ASSERT(IsObjectAllocKind(kind));
     size_t thingSize = Arena::thingSize(kind);
 
     MOZ_ASSERT(thingSize == Arena::thingSize(kind));
     MOZ_ASSERT(thingSize >= sizeof(JSObject_Slots0));
     static_assert(sizeof(JSObject_Slots0) >= CellSize,
                   "All allocations must be at least the allocator-imposed minimum size.");
 
     // Off-main-thread alloc cannot trigger GC or make runtime assertions.
--- a/js/src/gc/Heap.h
+++ b/js/src/gc/Heap.h
@@ -105,18 +105,37 @@ enum class AllocKind {
     STRING,
     EXTERNAL_STRING,
     SYMBOL,
     JITCODE,
     LIMIT,
     LAST = LIMIT - 1
 };
 
-static_assert(uint8_t(AllocKind::OBJECT0) == 0, "Please check AllocKind iterations and comparisons"
-    " of the form |kind <= AllocKind::OBJECT_LAST| to ensure their range is still valid!");
+static_assert(int(AllocKind::FIRST) == 0, "Various places depend on AllocKind starting at 0, "
+                                          "please audit them carefully!");
+static_assert(int(AllocKind::OBJECT0) == 0, "Various places depend on AllocKind::OBJECT0 being 0, "
+                                            "please audit them carefully!");
+
+inline bool
+IsObjectAllocKind(AllocKind kind)
+{
+    return kind >= AllocKind::OBJECT0 && kind <= AllocKind::OBJECT_LAST;
+}
+
+inline bool
+IsValidAllocKind(AllocKind kind)
+{
+    return kind >= AllocKind::FIRST && kind <= AllocKind::LAST;
+}
+
+inline bool IsAllocKind(AllocKind kind)
+{
+    return kind >= AllocKind::FIRST && kind <= AllocKind::LIMIT;
+}
 
 // Returns a sequence for use in a range-based for loop,
 // to iterate over all alloc kinds.
 inline decltype(mozilla::MakeEnumeratedRange<int>(AllocKind::FIRST, AllocKind::LIMIT))
 AllAllocKinds()
 {
     return mozilla::MakeEnumeratedRange<int>(AllocKind::FIRST, AllocKind::LIMIT);
 }
@@ -129,18 +148,18 @@ ObjectAllocKinds()
     return mozilla::MakeEnumeratedRange<int>(AllocKind::OBJECT0, AllocKind::OBJECT_LIMIT);
 }
 
 // Returns a sequence for use in a range-based for loop,
 // to iterate over alloc kinds from |first| to |limit|, exclusive.
 inline decltype(mozilla::MakeEnumeratedRange<int>(AllocKind::FIRST, AllocKind::LIMIT))
 SomeAllocKinds(AllocKind first = AllocKind::FIRST, AllocKind limit = AllocKind::LIMIT)
 {
-    MOZ_ASSERT(limit <= AllocKind::LIMIT);
-    MOZ_ASSERT(first <= limit);
+    MOZ_ASSERT(IsAllocKind(first), "|first| is not a valid AllocKind!");
+    MOZ_ASSERT(IsAllocKind(limit), "|limit| is not a valid AllocKind!");
     return mozilla::MakeEnumeratedRange<int>(first, limit);
 }
 
 // AllAllocKindArray<ValueType> gives an enumerated array of ValueTypes,
 // with each index corresponding to a particular alloc kind.
 template<typename ValueType> using AllAllocKindArray =
     mozilla::EnumeratedArray<AllocKind, AllocKind::LIMIT, ValueType>;
 
@@ -610,18 +629,18 @@ struct ArenaHeader
     static_assert(ArenaShift >= 8 + 1 + 1 + 1,
                   "ArenaHeader::auxNextLink packing assumes that ArenaShift has enough bits to "
                   "cover allocKind and hasDelayedMarking.");
 
     inline uintptr_t address() const;
     inline Chunk *chunk() const;
 
     bool allocated() const {
-        MOZ_ASSERT(allocKind <= size_t(AllocKind::LIMIT));
-        return allocKind < size_t(AllocKind::LIMIT);
+        MOZ_ASSERT(IsAllocKind(AllocKind(allocKind)));
+        return IsValidAllocKind(AllocKind(allocKind));
     }
 
     void init(JS::Zone *zoneArg, AllocKind kind) {
         MOZ_ASSERT(!allocated());
         MOZ_ASSERT(!markOverflow);
         MOZ_ASSERT(!allocatedDuringIncremental);
         MOZ_ASSERT(!hasDelayedMarking);
         zone = zoneArg;
--- a/js/src/jit/MacroAssembler.cpp
+++ b/js/src/jit/MacroAssembler.cpp
@@ -1139,17 +1139,17 @@ MacroAssembler::callFreeStub(Register sl
     pop(regSlots);
 }
 
 // Inlined equivalent of gc::AllocateObject, without failure case handling.
 void
 MacroAssembler::allocateObject(Register result, Register temp, gc::AllocKind allocKind,
                                uint32_t nDynamicSlots, gc::InitialHeap initialHeap, Label *fail)
 {
-    MOZ_ASSERT(allocKind <= gc::AllocKind::OBJECT_LAST);
+    MOZ_ASSERT(gc::IsObjectAllocKind(allocKind));
 
     checkAllocatorState(fail);
 
     if (shouldNurseryAllocate(allocKind, initialHeap))
         return nurseryAllocate(result, temp, allocKind, nDynamicSlots, initialHeap, fail);
 
     if (!nDynamicSlots)
         return freeListAllocate(result, temp, allocKind, fail);
@@ -1175,31 +1175,31 @@ MacroAssembler::allocateObject(Register 
     bind(&success);
 }
 
 void
 MacroAssembler::newGCThing(Register result, Register temp, JSObject *templateObj,
                            gc::InitialHeap initialHeap, Label *fail)
 {
     gc::AllocKind allocKind = templateObj->asTenured().getAllocKind();
-    MOZ_ASSERT(allocKind <= gc::AllocKind::OBJECT_LAST);
+    MOZ_ASSERT(gc::IsObjectAllocKind(allocKind));
 
     size_t ndynamic = 0;
     if (templateObj->isNative())
         ndynamic = templateObj->as<NativeObject>().numDynamicSlots();
     allocateObject(result, temp, allocKind, ndynamic, initialHeap, fail);
 }
 
 void
 MacroAssembler::createGCObject(Register obj, Register temp, JSObject *templateObj,
                                gc::InitialHeap initialHeap, Label *fail, bool initContents,
                                bool convertDoubleElements)
 {
     gc::AllocKind allocKind = templateObj->asTenured().getAllocKind();
-    MOZ_ASSERT(allocKind <= gc::AllocKind::OBJECT_LAST);
+    MOZ_ASSERT(gc::IsObjectAllocKind(allocKind));
 
     uint32_t nDynamicSlots = 0;
     if (templateObj->isNative()) {
         nDynamicSlots = templateObj->as<NativeObject>().numDynamicSlots();
 
         // Arrays with copy on write elements do not need fixed space for an
         // elements header. The template object, which owns the original
         // elements, might have another allocation kind.
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -1155,17 +1155,17 @@ AssertValidObjectPtr(JSContext *cx, JSOb
     MOZ_ASSERT(obj->runtimeFromMainThread() == cx->runtime());
 
     MOZ_ASSERT_IF(!obj->hasLazyGroup() && obj->maybeShape(),
                   obj->group()->clasp() == obj->maybeShape()->getObjectClass());
 
     if (obj->isTenured()) {
         MOZ_ASSERT(obj->isAligned());
         gc::AllocKind kind = obj->asTenured().getAllocKind();
-        MOZ_ASSERT(kind <= js::gc::AllocKind::OBJECT_LAST);
+        MOZ_ASSERT(gc::IsObjectAllocKind(kind));
         MOZ_ASSERT(obj->asTenured().zone() == cx->zone());
     }
 #endif
 }
 
 void
 AssertValidObjectOrNullPtr(JSContext *cx, JSObject *obj)
 {
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -1828,17 +1828,17 @@ static bool
 CanRelocateZone(JSRuntime *rt, Zone *zone)
 {
     return !rt->isAtomsZone(zone) && !rt->isSelfHostingZone(zone);
 }
 
 static bool
 CanRelocateAllocKind(AllocKind kind)
 {
-    return kind <= AllocKind::OBJECT_LAST;
+    return IsObjectAllocKind(kind);
 }
 
 size_t ArenaHeader::countFreeCells()
 {
     size_t count = 0;
     size_t thingSize = getThingSize();
     FreeSpan firstSpan(getFirstFreeSpan());
     for (const FreeSpan *span = &firstSpan; !span->isEmpty(); span = span->nextSpan())
@@ -1954,17 +1954,17 @@ RelocateCell(Zone *zone, TenuredCell *sr
         dstAlloc = GCRuntime::refillFreeListInGC(zone, thingKind);
     if (!dstAlloc)
         return false;
     TenuredCell *dst = TenuredCell::fromPointer(dstAlloc);
 
     // Copy source cell contents to destination.
     memcpy(dst, src, thingSize);
 
-    if (thingKind <= AllocKind::OBJECT_LAST) {
+    if (IsObjectAllocKind(thingKind)) {
         JSObject *srcObj = static_cast<JSObject *>(static_cast<Cell *>(src));
         JSObject *dstObj = static_cast<JSObject *>(static_cast<Cell *>(dst));
 
         if (srcObj->isNative()) {
             NativeObject *srcNative = &srcObj->as<NativeObject>();
             NativeObject *dstNative = &dstObj->as<NativeObject>();
 
             // Fixup the pointer to inline object elements if necessary.
@@ -2306,17 +2306,17 @@ struct ArenasToUpdate
     AllocKind kind;      // Current alloc kind to process
     ArenaHeader *arena;  // Next arena to process
 
     bool shouldProcessKind(AllocKind kind);
 };
 
 bool ArenasToUpdate::shouldProcessKind(AllocKind kind)
 {
-    MOZ_ASSERT(kind < AllocKind::LIMIT);
+    MOZ_ASSERT(IsValidAllocKind(kind));
 
     // GC things that do not contain JSObject pointers don't need updating.
     if (kind == AllocKind::FAT_INLINE_STRING ||
         kind == AllocKind::STRING ||
         kind == AllocKind::EXTERNAL_STRING ||
         kind == AllocKind::SYMBOL)
     {
         return false;
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -76,17 +76,17 @@ template <> struct MapTypeToFinalizeKind
 template <> struct MapTypeToFinalizeKind<JSString>          { static const AllocKind kind = AllocKind::STRING; };
 template <> struct MapTypeToFinalizeKind<JSExternalString>  { static const AllocKind kind = AllocKind::EXTERNAL_STRING; };
 template <> struct MapTypeToFinalizeKind<JS::Symbol>        { static const AllocKind kind = AllocKind::SYMBOL; };
 template <> struct MapTypeToFinalizeKind<jit::JitCode>      { static const AllocKind kind = AllocKind::JITCODE; };
 
 static inline bool
 IsNurseryAllocable(AllocKind kind)
 {
-    MOZ_ASSERT(kind < AllocKind::LIMIT);
+    MOZ_ASSERT(IsValidAllocKind(kind));
     static const bool map[] = {
         false,     /* AllocKind::OBJECT0 */
         true,      /* AllocKind::OBJECT0_BACKGROUND */
         false,     /* AllocKind::OBJECT2 */
         true,      /* AllocKind::OBJECT2_BACKGROUND */
         false,     /* AllocKind::OBJECT4 */
         true,      /* AllocKind::OBJECT4_BACKGROUND */
         false,     /* AllocKind::OBJECT8 */
@@ -109,17 +109,17 @@ IsNurseryAllocable(AllocKind kind)
     };
     JS_STATIC_ASSERT(JS_ARRAY_LENGTH(map) == size_t(AllocKind::LIMIT));
     return map[size_t(kind)];
 }
 
 static inline bool
 IsBackgroundFinalized(AllocKind kind)
 {
-    MOZ_ASSERT(kind < AllocKind::LIMIT);
+    MOZ_ASSERT(IsValidAllocKind(kind));
     static const bool map[] = {
         false,     /* AllocKind::OBJECT0 */
         true,      /* AllocKind::OBJECT0_BACKGROUND */
         false,     /* AllocKind::OBJECT2 */
         true,      /* AllocKind::OBJECT2_BACKGROUND */
         false,     /* AllocKind::OBJECT4 */
         true,      /* AllocKind::OBJECT4_BACKGROUND */
         false,     /* AllocKind::OBJECT8 */
@@ -142,17 +142,17 @@ IsBackgroundFinalized(AllocKind kind)
     };
     JS_STATIC_ASSERT(JS_ARRAY_LENGTH(map) == size_t(AllocKind::LIMIT));
     return map[size_t(kind)];
 }
 
 static inline bool
 CanBeFinalizedInBackground(AllocKind kind, const Class *clasp)
 {
-    MOZ_ASSERT(kind <= AllocKind::OBJECT_LAST);
+    MOZ_ASSERT(IsObjectAllocKind(kind));
     /* If the class has no finalizer or a finalizer that is safe to call on
      * a different thread, we change the alloc kind. For example,
      * AllocKind::OBJECT0 calls the finalizer on the main thread,
      * AllocKind::OBJECT0_BACKGROUND calls the finalizer on the gcHelperThread.
      * IsBackgroundFinalized is called to prevent recursively incrementing
      * the alloc kind; kind may already be a background finalize kind.
      */
     return (!IsBackgroundFinalized(kind) &&
@@ -214,17 +214,17 @@ GetGCObjectKindForBytes(size_t nbytes)
     MOZ_ASSERT(nbytes <= dataSlots * sizeof(Value));
     return GetGCObjectKind(dataSlots);
 }
 
 static inline AllocKind
 GetBackgroundAllocKind(AllocKind kind)
 {
     MOZ_ASSERT(!IsBackgroundFinalized(kind));
-    MOZ_ASSERT(kind < AllocKind::OBJECT_LAST);
+    MOZ_ASSERT(IsObjectAllocKind(kind));
     return AllocKind(size_t(kind) + 1);
 }
 
 /* Get the number of fixed slots and initial capacity associated with a kind. */
 static inline size_t
 GetGCKindSlots(AllocKind thingKind)
 {
     /* Using a switch in hopes that thingKind will usually be a compile-time constant. */
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -1380,17 +1380,17 @@ NewObjectWithGroupIsCachable(ExclusiveCo
 /*
  * Create a plain object with the specified group. This bypasses getNewGroup to
  * avoid losing creation site information for objects made by scripted 'new'.
  */
 JSObject *
 js::NewObjectWithGroupCommon(ExclusiveContext *cx, HandleObjectGroup group,
                              gc::AllocKind allocKind, NewObjectKind newKind)
 {
-    MOZ_ASSERT(allocKind <= gc::AllocKind::OBJECT_LAST);
+    MOZ_ASSERT(gc::IsObjectAllocKind(allocKind));
     if (CanBeFinalizedInBackground(allocKind, group->clasp()))
         allocKind = GetBackgroundAllocKind(allocKind);
 
     bool isCachable = NewObjectWithGroupIsCachable(cx, group, newKind);
     if (isCachable) {
         NewObjectCache &cache = cx->asJSContext()->runtime()->newObjectCache;
         NewObjectCache::EntryIndex entry = -1;
         if (cache.lookupGroup(group, allocKind, &entry)) {
--- a/js/src/jspropertytree.cpp
+++ b/js/src/jspropertytree.cpp
@@ -237,17 +237,17 @@ Shape::fixupDictionaryShapeAfterMovingGC
     if (IsInsideNursery(cell)) {
         listp = nullptr;
         return;
     }
 
     AllocKind kind = TenuredCell::fromPointer(cell)->getAllocKind();
     MOZ_ASSERT(kind == AllocKind::SHAPE ||
                kind == AllocKind::ACCESSOR_SHAPE ||
-               kind <= AllocKind::OBJECT_LAST);
+               IsObjectAllocKind(kind));
     if (kind == AllocKind::SHAPE || kind == AllocKind::ACCESSOR_SHAPE) {
         // listp points to the parent field of the next shape.
         Shape *next = reinterpret_cast<Shape *>(uintptr_t(listp) -
                                                 offsetof(Shape, parent));
         listp = &gc::MaybeForwarded(next)->parent;
     } else {
         // listp points to the shape_ field of an object.
         JSObject *last = reinterpret_cast<JSObject *>(uintptr_t(listp) -