[cleanup] Use RootIndex instead of int in serializer code

Bug: v8:8015
Change-Id: I2f407c5ffaed96b90b9ead452a98a19ef1700b75
Reviewed-on: https://chromium-review.googlesource.com/1240336
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56233}

src/address-map.cc

@@ -14,9 +14,8 @@ RootIndexMap::RootIndexMap(Isolate* isolate) {
   map_ = isolate->root_index_map();
   if (map_ != nullptr) return;
   map_ = new HeapObjectToIndexHashMap();
-  for (uint32_t i = 0; i < static_cast<int>(RootIndex::kStrongRootListLength);
-       i++) {
-    RootIndex root_index = static_cast<RootIndex>(i);
+  for (RootIndex root_index = RootIndex::kFirstStrongRoot;
+       root_index <= RootIndex::kLastStrongRoot; ++root_index) {
     Object* root = isolate->heap()->root(root_index);
     if (!root->IsHeapObject()) continue;
     // Omit root entries that can be written after initialization. They must
@@ -26,11 +25,12 @@ RootIndexMap::RootIndexMap(Isolate* isolate) {
     if (isolate->heap()->RootCanBeTreatedAsConstant(root_index)) {
       HeapObject* heap_object = HeapObject::cast(root);
       Maybe<uint32_t> maybe_index = map_->Get(heap_object);
+      uint32_t index = static_cast<uint32_t>(root_index);
       if (maybe_index.IsJust()) {
         // Some are initialized to a previous value in the root list.
-        DCHECK_LT(maybe_index.FromJust(), i);
+        DCHECK_LT(maybe_index.FromJust(), index);
       } else {
-        map_->Set(heap_object, i);
+        map_->Set(heap_object, index);
       }
     } else {
       // Immortal immovable root objects are constant and allocated on the first

src/address-map.h

@@ -56,11 +56,14 @@ class RootIndexMap {
  public:
   explicit RootIndexMap(Isolate* isolate);
 
-  static const int kInvalidRootIndex = -1;
-
-  int Lookup(HeapObject* obj) {
+  // Returns true on successful lookup and sets *|out_root_list|.
+  bool Lookup(HeapObject* obj, RootIndex* out_root_list) {
     Maybe<uint32_t> maybe_index = map_->Get(obj);
-    return maybe_index.IsJust() ? maybe_index.FromJust() : kInvalidRootIndex;
+    if (maybe_index.IsJust()) {
+      *out_root_list = static_cast<RootIndex>(maybe_index.FromJust());
+      return true;
+    }
+    return false;
   }
 
  private:

src/handles.cc

@@ -30,12 +30,9 @@ bool HandleBase::IsDereferenceAllowed(DereferenceCheckMode mode) const {
   Isolate* isolate;
   if (!Isolate::FromWritableHeapObject(heap_object, &isolate)) return true;
   Heap* heap = isolate->heap();
-  Object** roots_array_start = heap->roots_array_start();
-  if (roots_array_start <= location_ &&
-      location_ < roots_array_start +
-                      static_cast<int>(RootIndex::kStrongRootListLength) &&
-      heap->RootCanBeTreatedAsConstant(
-          static_cast<RootIndex>(location_ - roots_array_start))) {
+  RootIndex root_index;
+  if (heap->IsRootHandleLocation(location_, &root_index) &&
+      heap->RootCanBeTreatedAsConstant(root_index)) {
     return true;
   }
   if (!AllowHandleDereference::IsAllowed()) return false;
@@ -158,11 +155,9 @@ Object** CanonicalHandleScope::Lookup(Object* object) {
     return HandleScope::CreateHandle(isolate_, object);
   }
   if (object->IsHeapObject()) {
-    int index = root_index_map_->Lookup(HeapObject::cast(object));
-    if (index != RootIndexMap::kInvalidRootIndex) {
-      return isolate_->heap()
-          ->root_handle(static_cast<RootIndex>(index))
-          .location();
+    RootIndex root_index;
+    if (root_index_map_->Lookup(HeapObject::cast(object), &root_index)) {
+      return isolate_->heap()->root_handle(root_index).location();
     }
   }
   Object*** entry = identity_map_->Get(object);

src/heap/heap.cc

@@ -3770,9 +3770,8 @@ void Heap::IterateWeakRoots(RootVisitor* v, VisitMode mode) {
   const bool isMinorGC = mode == VISIT_ALL_IN_SCAVENGE ||
                          mode == VISIT_ALL_IN_MINOR_MC_MARK ||
                          mode == VISIT_ALL_IN_MINOR_MC_UPDATE;
-  v->VisitRootPointer(
-      Root::kStringTable, nullptr,
-      reinterpret_cast<Object**>(&roots_[RootIndex::kStringTable]));
+  v->VisitRootPointer(Root::kStringTable, nullptr,
+                      &roots_[RootIndex::kStringTable]);
   v->Synchronize(VisitorSynchronization::kStringTable);
   if (!isMinorGC && mode != VISIT_ALL_IN_SWEEP_NEWSPACE &&
       mode != VISIT_FOR_SERIALIZATION) {
@@ -3846,8 +3845,7 @@ void Heap::IterateStrongRoots(RootVisitor* v, VisitMode mode) {
                          mode == VISIT_ALL_IN_MINOR_MC_MARK ||
                          mode == VISIT_ALL_IN_MINOR_MC_UPDATE;
   v->VisitRootPointers(Root::kStrongRootList, nullptr,
-                       &roots_[RootIndex::kRootsStart],
-                       &roots_[RootIndex::kStrongRootListLength]);
+                       roots_.strong_roots_begin(), roots_.strong_roots_end());
   v->Synchronize(VisitorSynchronization::kStrongRootList);
 
   isolate_->bootstrapper()->Iterate(v);

src/heap/heap.h

@@ -781,6 +781,10 @@ class Heap {
     return Handle<Object>(&roots_[index]);
   }
 
+  bool IsRootHandleLocation(Object** handle_location, RootIndex* index) const {
+    return roots_.IsRootHandleLocation(handle_location, index);
+  }
+
   template <typename T>
   bool IsRootHandle(Handle<T> handle, RootIndex* index) const {
     return roots_.IsRootHandle(handle, index);

src/objects-debug.cc

@@ -2160,9 +2160,8 @@ bool CanLeak(Object* obj, Heap* heap) {
   if (obj->IsContext()) return true;
   if (obj->IsMap()) {
     Map* map = Map::cast(obj);
-    for (int i = 0; i < static_cast<int>(RootIndex::kStrongRootListLength);
-         i++) {
-      RootIndex root_index = static_cast<RootIndex>(i);
+    for (RootIndex root_index = RootIndex::kFirstStrongRoot;
+         root_index <= RootIndex::kLastStrongRoot; ++root_index) {
       if (map == heap->root(root_index)) return false;
     }
     return true;

src/roots-inl.h

@@ -11,9 +11,19 @@
 #include "src/objects/api-callbacks.h"
 
 namespace v8 {
-
 namespace internal {
 
+V8_INLINE bool operator<(RootIndex lhs, RootIndex rhs) {
+  typedef typename std::underlying_type<RootIndex>::type type;
+  return static_cast<type>(lhs) < static_cast<type>(rhs);
+}
+
+V8_INLINE RootIndex operator++(RootIndex& index) {
+  typedef typename std::underlying_type<RootIndex>::type type;
+  index = static_cast<RootIndex>(static_cast<type>(index) + 1);
+  return index;
+}
+
 ReadOnlyRoots::ReadOnlyRoots(Isolate* isolate) : heap_(isolate->heap()) {}
 
 #define ROOT_ACCESSOR(type, name, CamelName)                        \
@@ -35,7 +45,6 @@ FixedTypedArrayBase* ReadOnlyRoots::EmptyFixedTypedArrayForMap(const Map* map) {
 }
 
 }  // namespace internal
-
 }  // namespace v8
 
 #endif  // V8_ROOTS_INL_H_

src/roots.h

@@ -331,10 +331,16 @@ enum class RootIndex : uint16_t {
 
   kRootListLength,
 
-  // Helper aliases.
-  kRootsStart = 0,
-  kStrongRootListLength = kStringTable,
-  kSmiRootsStart = kStringTable + 1
+  // Helper aliases for inclusive regions of root indices.
+  kFirstRoot = 0,
+  kLastRoot = kRootListLength - 1,
+
+  // kStringTable is not a strong root.
+  kFirstStrongRoot = kFirstRoot,
+  kLastStrongRoot = kStringTable - 1,
+
+  kFirstSmiRoot = kStringTable + 1,
+  kLastSmiRoot = kLastRoot
 };
 // clang-format on
 
@@ -344,20 +350,21 @@ class RootsTable {
   static constexpr size_t kEntriesCount =
       static_cast<size_t>(RootIndex::kRootListLength);
 
-  static constexpr size_t kSmiRootsStart =
-      static_cast<size_t>(RootIndex::kSmiRootsStart);
-
   RootsTable() : roots_{} {}
 
-  template <typename T>
-  bool IsRootHandle(Handle<T> handle, RootIndex* index) const {
-    Object** const handle_location = bit_cast<Object**>(handle.address());
+  bool IsRootHandleLocation(Object** handle_location, RootIndex* index) const {
     if (handle_location >= &roots_[kEntriesCount]) return false;
     if (handle_location < &roots_[0]) return false;
     *index = static_cast<RootIndex>(handle_location - &roots_[0]);
     return true;
   }
 
+  template <typename T>
+  bool IsRootHandle(Handle<T> handle, RootIndex* index) const {
+    Object** handle_location = bit_cast<Object**>(handle.address());
+    return IsRootHandleLocation(handle_location, index);
+  }
+
   Object* const& operator[](RootIndex root_index) const {
     size_t index = static_cast<size_t>(root_index);
     DCHECK_LT(index, kEntriesCount);
@@ -365,8 +372,19 @@ class RootsTable {
   }
 
  private:
-  Object** smi_roots_begin() { return &roots_[kSmiRootsStart]; }
-  Object** smi_roots_end() { return &roots_[kEntriesCount]; }
+  Object** strong_roots_begin() {
+    return &roots_[static_cast<size_t>(RootIndex::kFirstStrongRoot)];
+  }
+  Object** strong_roots_end() {
+    return &roots_[static_cast<size_t>(RootIndex::kLastStrongRoot) + 1];
+  }
+
+  Object** smi_roots_begin() {
+    return &roots_[static_cast<size_t>(RootIndex::kFirstSmiRoot)];
+  }
+  Object** smi_roots_end() {
+    return &roots_[static_cast<size_t>(RootIndex::kLastSmiRoot) + 1];
+  }
 
   Object*& operator[](RootIndex root_index) {
     size_t index = static_cast<size_t>(root_index);

src/snapshot/builtin-serializer.cc

@@ -70,8 +70,8 @@ void BuiltinSerializer::SerializeObject(HeapObject* o, HowToCode how_to_code,
   DCHECK(!o->IsSmi());
 
   // Roots can simply be serialized as root references.
-  int root_index = root_index_map()->Lookup(o);
-  if (root_index != RootIndexMap::kInvalidRootIndex) {
+  RootIndex root_index;
+  if (root_index_map()->Lookup(o, &root_index)) {
     DCHECK(startup_serializer_->root_has_been_serialized(root_index));
     PutRoot(root_index, o, how_to_code, where_to_point, skip);
     return;

src/snapshot/code-serializer.cc

@@ -124,8 +124,8 @@ void CodeSerializer::SerializeObject(HeapObject* obj, HowToCode how_to_code,
                                      WhereToPoint where_to_point, int skip) {
   if (SerializeHotObject(obj, how_to_code, where_to_point, skip)) return;
 
-  int root_index = root_index_map()->Lookup(obj);
-  if (root_index != RootIndexMap::kInvalidRootIndex) {
+  RootIndex root_index;
+  if (root_index_map()->Lookup(obj, &root_index)) {
     PutRoot(root_index, obj, how_to_code, where_to_point, skip);
     return;
   }

src/snapshot/partial-serializer.cc

@@ -59,8 +59,8 @@ void PartialSerializer::SerializeObject(HeapObject* obj, HowToCode how_to_code,
   }
   if (SerializeHotObject(obj, how_to_code, where_to_point, skip)) return;
 
-  int root_index = root_index_map()->Lookup(obj);
-  if (root_index != RootIndexMap::kInvalidRootIndex) {
+  RootIndex root_index;
+  if (root_index_map()->Lookup(obj, &root_index)) {
     PutRoot(root_index, obj, how_to_code, where_to_point, skip);
     return;
   }

src/snapshot/serializer.cc

@@ -237,9 +237,10 @@ bool Serializer<AllocatorT>::ObjectIsBytecodeHandler(HeapObject* obj) const {
 
 template <class AllocatorT>
 void Serializer<AllocatorT>::PutRoot(
-    int root_index, HeapObject* object,
+    RootIndex root, HeapObject* object,
     SerializerDeserializer::HowToCode how_to_code,
     SerializerDeserializer::WhereToPoint where_to_point, int skip) {
+  int root_index = static_cast<int>(root);
   if (FLAG_trace_serializer) {
     PrintF(" Encoding root %d:", root_index);
     object->ShortPrint();
@@ -736,11 +737,13 @@ void Serializer<AllocatorT>::ObjectSerializer::VisitPointers(
     HeapObjectReferenceType reference_type;
     while (current < end &&
            (*current)->GetHeapObject(&current_contents, &reference_type)) {
-      int root_index = serializer_->root_index_map()->Lookup(current_contents);
+      RootIndex root_index;
       // Repeats are not subject to the write barrier so we can only use
       // immortal immovable root members. They are never in new space.
-      if (current != start && root_index != RootIndexMap::kInvalidRootIndex &&
-          Heap::RootIsImmortalImmovable(static_cast<RootIndex>(root_index)) &&
+      if (current != start &&
+          serializer_->root_index_map()->Lookup(current_contents,
+                                                &root_index) &&
+          Heap::RootIsImmortalImmovable(root_index) &&
           *current == current[-1]) {
         DCHECK_EQ(reference_type, HeapObjectReferenceType::STRONG);
         DCHECK(!Heap::InNewSpace(current_contents));

src/snapshot/serializer.h

@@ -172,8 +172,8 @@ class Serializer : public SerializerDeserializer {
                          Object** end) override;
   void SerializeRootObject(Object* object);
 
-  void PutRoot(int index, HeapObject* object, HowToCode how, WhereToPoint where,
-               int skip);
+  void PutRoot(RootIndex root_index, HeapObject* object, HowToCode how,
+               WhereToPoint where, int skip);
   void PutSmi(Smi* smi);
   void PutBackReference(HeapObject* object, SerializerReference reference);
   void PutAttachedReference(SerializerReference reference,

src/snapshot/startup-serializer.cc

@@ -34,10 +34,10 @@ void StartupSerializer::SerializeObject(HeapObject* obj, HowToCode how_to_code,
   }
   if (SerializeHotObject(obj, how_to_code, where_to_point, skip)) return;
 
-  int root_index = root_index_map()->Lookup(obj);
+  RootIndex root_index;
   // We can only encode roots as such if it has already been serialized.
   // That applies to root indices below the wave front.
-  if (root_index != RootIndexMap::kInvalidRootIndex) {
+  if (root_index_map()->Lookup(obj, &root_index)) {
     if (root_has_been_serialized(root_index)) {
       PutRoot(root_index, obj, how_to_code, where_to_point, skip);
       return;
@@ -136,7 +136,7 @@ void StartupSerializer::VisitRootPointers(Root root, const char* description,
     //   referenced using kRootArray bytecodes.
     for (Object** current = start; current < end; current++) {
       SerializeRootObject(*current);
-      int root_index = static_cast<int>(current - start);
+      size_t root_index = static_cast<size_t>(current - start);
       root_has_been_serialized_.set(root_index);
     }
   } else {
@@ -152,12 +152,9 @@ void StartupSerializer::CheckRehashability(HeapObject* obj) {
 }
 
 bool StartupSerializer::MustBeDeferred(HeapObject* object) {
-  if (root_has_been_serialized_.test(
-          static_cast<size_t>(RootIndex::kFreeSpaceMap)) &&
-      root_has_been_serialized_.test(
-          static_cast<size_t>(RootIndex::kOnePointerFillerMap)) &&
-      root_has_been_serialized_.test(
-          static_cast<size_t>(RootIndex::kTwoPointerFillerMap))) {
+  if (root_has_been_serialized(RootIndex::kFreeSpaceMap) &&
+      root_has_been_serialized(RootIndex::kOnePointerFillerMap) &&
+      root_has_been_serialized(RootIndex::kTwoPointerFillerMap)) {
     // All required root objects are serialized, so any aligned objects can
     // be saved without problems.
     return false;

src/snapshot/startup-serializer.h

@@ -28,8 +28,8 @@ class StartupSerializer : public Serializer<> {
   int PartialSnapshotCacheIndex(HeapObject* o);
 
   bool can_be_rehashed() const { return can_be_rehashed_; }
-  bool root_has_been_serialized(int root_index) const {
-    return root_has_been_serialized_.test(root_index);
+  bool root_has_been_serialized(RootIndex root_index) const {
+    return root_has_been_serialized_.test(static_cast<size_t>(root_index));
   }
 
  private:
@@ -69,8 +69,7 @@ class StartupSerializer : public Serializer<> {
 
   void CheckRehashability(HeapObject* obj);
 
-  std::bitset<static_cast<size_t>(RootIndex::kStrongRootListLength)>
-      root_has_been_serialized_;
+  std::bitset<RootsTable::kEntriesCount> root_has_been_serialized_;
   PartialCacheIndexMap partial_cache_index_map_;
   std::vector<AccessorInfo*> accessor_infos_;
   std::vector<CallHandlerInfo*> call_handler_infos_;

test/cctest/heap/test-heap.cc

@@ -4765,8 +4765,8 @@ TEST(MapRetaining) {
 }
 
 TEST(WritableVsImmortalRoots) {
-  for (int i = 0; i < static_cast<int>(RootIndex::kStrongRootListLength); ++i) {
-    RootIndex root_index = static_cast<RootIndex>(i);
+  for (RootIndex root_index = RootIndex::kFirstRoot;
+       root_index <= RootIndex::kLastRoot; ++root_index) {
     bool writable = Heap::RootCanBeWrittenAfterInitialization(root_index);
     bool immortal = Heap::RootIsImmortalImmovable(root_index);
     // A root value can be writable, immortal, or neither, but not both.

test/mkgrokdump/mkgrokdump.cc

@@ -117,7 +117,7 @@ static int DumpHeapConstants(const char* argv0) {
         // Skip maps in RO_SPACE since they will be reported elsewhere.
         if (o->IsMap()) continue;
         const char* n = nullptr;
-        i::RootIndex i = i::RootIndex::kStrongRootListLength;
+        i::RootIndex i = i::RootIndex::kFirstSmiRoot;
         intptr_t p = reinterpret_cast<intptr_t>(o) & 0x7FFFF;
         STRONG_READ_ONLY_ROOT_LIST(RO_ROOT_LIST_CASE)
         MUTABLE_ROOT_LIST(ROOT_LIST_CASE)