[ubsan] Port OrderedHashTable and subclasses to the new design

Bug: v8:3770
Change-Id: I0bac3cc2a0a2c7e6c1cf18e7db00bdffd2d9f7a1
Reviewed-on: https://chromium-review.googlesource.com/c/1345328
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57799}

src/handles.h

@@ -30,6 +30,9 @@ class Isolate;
 template <typename T>
 class MaybeHandle;
 class ObjectPtr;
+class OrderedHashMap;
+class OrderedHashSet;
+class OrderedNameDictionary;
 
 // ----------------------------------------------------------------------------
 // Base class for Handle instantiations.  Don't use directly.
@@ -138,14 +141,18 @@ class Handle final : public HandleBase {
   // Constructor for handling automatic up casting.
   // Ex. Handle<JSFunction> can be passed when Handle<Object> is expected.
   // TODO(3770): Remove special cases after the migration.
-  template <typename S, typename = typename std::enable_if<
-                            std::is_convertible<S*, T*>::value ||
-                            std::is_same<T, Object>::value ||
-                            (std::is_same<T, HeapObject>::value &&
-                             (std::is_same<S, Code>::value ||
-                              std::is_same<S, Context>::value ||
-                              std::is_same<S, Map>::value ||
-                              std::is_same<S, NumberDictionary>::value))>::type>
+  template <
+      typename S,
+      typename = typename std::enable_if<
+          std::is_convertible<S*, T*>::value ||
+          std::is_same<T, Object>::value ||
+          (std::is_same<T, HeapObject>::value &&
+           (std::is_same<S, Code>::value || std::is_same<S, Context>::value ||
+            std::is_same<S, Map>::value ||
+            std::is_same<S, NumberDictionary>::value ||
+            std::is_same<S, OrderedHashMap>::value ||
+            std::is_same<S, OrderedHashSet>::value ||
+            std::is_same<S, OrderedNameDictionary>::value))>::type>
   V8_INLINE Handle(Handle<S> handle) : HandleBase(handle) {}
 
   // The NeverReadOnlySpaceObject special-case is needed for the

src/keys.cc

@@ -75,7 +75,9 @@ void KeyAccumulator::AddKey(Handle<Object> key, AddKeyConversion convert) {
   }
   if (IsShadowed(key)) return;
   if (keys_.is_null()) {
-    keys_ = OrderedHashSet::Allocate(isolate_, 16);
+    // TODO(3770): Drop explicit conversion.
+    keys_ =
+        Handle<FixedArray>(OrderedHashSet::Allocate(isolate_, 16).location());
   }
   uint32_t index;
   if (convert == CONVERT_TO_ARRAY_INDEX && key->IsString() &&
@@ -88,7 +90,8 @@ void KeyAccumulator::AddKey(Handle<Object> key, AddKeyConversion convert) {
     // be left-trimmer. Hence the previous Set should not keep a pointer to the
     // new one.
     keys_->set(OrderedHashSet::kNextTableIndex, Smi::kZero);
-    keys_ = new_set;
+    // TODO(3770): Drop explicit conversion.
+    keys_ = Handle<FixedArray>(new_set.location());
   }
 }
 

src/objects-inl.h

@@ -528,6 +528,25 @@ StringSet::StringSet(Address ptr) : HashTable<StringSet, StringSetShape>(ptr) {
   SLOW_DCHECK(IsStringSet());
 }
 
+template <class Derived, int entrysize>
+OrderedHashTable<Derived, entrysize>::OrderedHashTable(Address ptr)
+    : FixedArrayPtr(ptr) {}
+
+OrderedHashSet::OrderedHashSet(Address ptr)
+    : OrderedHashTable<OrderedHashSet, 1>(ptr) {
+  SLOW_DCHECK(IsOrderedHashSet());
+}
+
+OrderedHashMap::OrderedHashMap(Address ptr)
+    : OrderedHashTable<OrderedHashMap, 2>(ptr) {
+  SLOW_DCHECK(IsOrderedHashMap());
+}
+
+OrderedNameDictionary::OrderedNameDictionary(Address ptr)
+    : OrderedHashTable<OrderedNameDictionary, 3>(ptr) {
+  SLOW_DCHECK(IsOrderedNameDictionary());
+}
+
 // ------------------------------------
 // Cast operations
 
@@ -547,7 +566,7 @@ CAST_ACCESSOR(HeapObject)
 CAST_ACCESSOR(HeapNumber)
 CAST_ACCESSOR(LayoutDescriptor)
 CAST_ACCESSOR(MutableHeapNumber)
-CAST_ACCESSOR(OrderedNameDictionary)
+CAST_ACCESSOR2(OrderedNameDictionary)
 CAST_ACCESSOR2(NameDictionary)
 CAST_ACCESSOR(NormalizedMapCache)
 CAST_ACCESSOR2(NumberDictionary)
@@ -555,8 +574,8 @@ CAST_ACCESSOR(Object)
 CAST_ACCESSOR2(ObjectHashSet)
 CAST_ACCESSOR2(ObjectHashTable)
 CAST_ACCESSOR(Oddball)
-CAST_ACCESSOR(OrderedHashMap)
-CAST_ACCESSOR(OrderedHashSet)
+CAST_ACCESSOR2(OrderedHashMap)
+CAST_ACCESSOR2(OrderedHashSet)
 CAST_ACCESSOR(PropertyCell)
 CAST_ACCESSOR(RegExpMatchInfo)
 CAST_ACCESSOR(ScopeInfo)
@@ -2028,7 +2047,7 @@ Relocatable::~Relocatable() {
 
 template<class Derived, class TableType>
 Object* OrderedHashTableIterator<Derived, TableType>::CurrentKey() {
-  TableType* table(TableType::cast(this->table()));
+  TableType table = TableType::cast(this->table());
   int index = Smi::ToInt(this->index());
   Object* key = table->KeyAt(index);
   DCHECK(!key->IsTheHole());

src/objects/js-collection-inl.h

@@ -30,7 +30,7 @@ CAST_ACCESSOR(JSWeakMap)
 CAST_ACCESSOR(JSWeakSet)
 
 Object* JSMapIterator::CurrentValue() {
-  OrderedHashMap* table(OrderedHashMap::cast(this->table()));
+  OrderedHashMap table = OrderedHashMap::cast(this->table());
   int index = Smi::ToInt(this->index());
   Object* value = table->ValueAt(index);
   DCHECK(!value->IsTheHole());

src/objects/ordered-hash-table-inl.h

@@ -10,6 +10,9 @@
 #include "src/heap/heap.h"
 #include "src/objects/fixed-array-inl.h"
 
+// Has to be the last include (doesn't have include guards):
+#include "src/objects/object-macros.h"
+
 namespace v8 {
 namespace internal {
 
@@ -114,4 +117,6 @@ inline bool SmallOrderedHashMap::Is(Handle<HeapObject> table) {
 }  // namespace internal
 }  // namespace v8
 
+#include "src/objects/object-macros-undef.h"
+
 #endif  // V8_OBJECTS_ORDERED_HASH_TABLE_INL_H_

src/objects/ordered-hash-table.cc

@@ -81,7 +81,7 @@ Handle<Derived> OrderedHashTable<Derived, entrysize>::Clear(
 
 template <class Derived, int entrysize>
 bool OrderedHashTable<Derived, entrysize>::HasKey(Isolate* isolate,
-                                                  Derived* table, Object* key) {
+                                                  Derived table, Object* key) {
   DCHECK((entrysize == 1 && table->IsOrderedHashSet()) ||
          (entrysize == 2 && table->IsOrderedHashMap()) ||
          (entrysize == 3 && table->IsOrderedNameDictionary()));
@@ -227,7 +227,7 @@ Handle<Derived> OrderedHashTable<Derived, entrysize>::Rehash(
 
 template <class Derived, int entrysize>
 bool OrderedHashTable<Derived, entrysize>::Delete(Isolate* isolate,
-                                                  Derived* table, Object* key) {
+                                                  Derived table, Object* key) {
   DisallowHeapAllocation no_gc;
   int entry = table->FindEntry(isolate, key);
   if (entry == kNotFound) return false;
@@ -376,11 +376,11 @@ template Handle<OrderedHashSet> OrderedHashTable<OrderedHashSet, 1>::Clear(
     Isolate* isolate, Handle<OrderedHashSet> table);
 
 template bool OrderedHashTable<OrderedHashSet, 1>::HasKey(Isolate* isolate,
-                                                          OrderedHashSet* table,
+                                                          OrderedHashSet table,
                                                           Object* key);
 
 template bool OrderedHashTable<OrderedHashSet, 1>::Delete(Isolate* isolate,
-                                                          OrderedHashSet* table,
+                                                          OrderedHashSet table,
                                                           Object* key);
 
 template int OrderedHashTable<OrderedHashSet, 1>::FindEntry(Isolate* isolate,
@@ -400,11 +400,11 @@ template Handle<OrderedHashMap> OrderedHashTable<OrderedHashMap, 2>::Clear(
     Isolate* isolate, Handle<OrderedHashMap> table);
 
 template bool OrderedHashTable<OrderedHashMap, 2>::HasKey(Isolate* isolate,
-                                                          OrderedHashMap* table,
+                                                          OrderedHashMap table,
                                                           Object* key);
 
 template bool OrderedHashTable<OrderedHashMap, 2>::Delete(Isolate* isolate,
-                                                          OrderedHashMap* table,
+                                                          OrderedHashMap table,
                                                           Object* key);
 
 template int OrderedHashTable<OrderedHashMap, 2>::FindEntry(Isolate* isolate,
@@ -416,7 +416,7 @@ OrderedHashTable<OrderedNameDictionary, 3>::Allocate(Isolate* isolate,
                                                      PretenureFlag pretenure);
 
 template bool OrderedHashTable<OrderedNameDictionary, 3>::HasKey(
-    Isolate* isolate, OrderedNameDictionary* table, Object* key);
+    Isolate* isolate, OrderedNameDictionary table, Object* key);
 
 template Handle<OrderedNameDictionary>
 OrderedHashTable<OrderedNameDictionary, 3>::EnsureGrowable(
@@ -888,12 +888,12 @@ Handle<HeapObject> OrderedHashSetHandler::Add(Isolate* isolate,
 template <class Derived, class TableType>
 void OrderedHashTableIterator<Derived, TableType>::Transition() {
   DisallowHeapAllocation no_allocation;
-  TableType* table = TableType::cast(this->table());
+  TableType table = TableType::cast(this->table());
   if (!table->IsObsolete()) return;
 
   int index = Smi::ToInt(this->index());
   while (table->IsObsolete()) {
-    TableType* next_table = table->NextTable();
+    TableType next_table = table->NextTable();
 
     if (index > 0) {
       int nod = table->NumberOfDeletedElements();
@@ -924,7 +924,7 @@ bool OrderedHashTableIterator<Derived, TableType>::HasMore() {
 
   Transition();
 
-  TableType* table = TableType::cast(this->table());
+  TableType table = TableType::cast(this->table());
   int index = Smi::ToInt(this->index());
   int used_capacity = table->UsedCapacity();
 

src/objects/ordered-hash-table.h

@@ -53,7 +53,7 @@ namespace internal {
 //   [3 + NumberOfRemovedHoles()..length]: Not used
 //
 template <class Derived, int entrysize>
-class OrderedHashTable : public FixedArray {
+class OrderedHashTable : public FixedArrayPtr {
  public:
   // Returns an OrderedHashTable with a capacity of at least |capacity|.
   static Handle<Derived> Allocate(Isolate* isolate, int capacity,
@@ -73,11 +73,11 @@ class OrderedHashTable : public FixedArray {
   static Handle<Derived> Clear(Isolate* isolate, Handle<Derived> table);
 
   // Returns true if the OrderedHashTable contains the key
-  static bool HasKey(Isolate* isolate, Derived* table, Object* key);
+  static bool HasKey(Isolate* isolate, Derived table, Object* key);
 
   // Returns a true value if the OrderedHashTable contains the key and
   // the key has been deleted. This does not shrink the table.
-  static bool Delete(Isolate* isolate, Derived* table, Object* key);
+  static bool Delete(Isolate* isolate, Derived table, Object* key);
 
   int FindEntry(Isolate* isolate, Object* key);
 
@@ -124,7 +124,7 @@ class OrderedHashTable : public FixedArray {
   bool IsObsolete() { return !get(kNextTableIndex)->IsSmi(); }
 
   // The next newer table. This is only valid if the table is obsolete.
-  Derived* NextTable() { return Derived::cast(get(kNextTableIndex)); }
+  Derived NextTable() { return Derived::cast(get(kNextTableIndex)); }
 
   // When the table is obsolete we store the indexes of the removed holes.
   int RemovedIndexAt(int index) {
@@ -185,16 +185,18 @@ class OrderedHashTable : public FixedArray {
   // Returns the number elements that can fit into the allocated buffer.
   int Capacity() { return NumberOfBuckets() * kLoadFactor; }
 
-  void SetNextTable(Derived* next_table) { set(kNextTableIndex, next_table); }
+  void SetNextTable(Derived next_table) { set(kNextTableIndex, next_table); }
 
   void SetRemovedIndexAt(int index, int removed_index) {
     return set(kRemovedHolesIndex + index, Smi::FromInt(removed_index));
   }
+
+  OBJECT_CONSTRUCTORS(OrderedHashTable, FixedArrayPtr)
 };
 
 class OrderedHashSet : public OrderedHashTable<OrderedHashSet, 1> {
  public:
-  DECL_CAST(OrderedHashSet)
+  DECL_CAST2(OrderedHashSet)
 
   static Handle<OrderedHashSet> Add(Isolate* isolate,
                                     Handle<OrderedHashSet> table,
@@ -205,11 +207,13 @@ class OrderedHashSet : public OrderedHashTable<OrderedHashSet, 1> {
   static HeapObject* GetEmpty(ReadOnlyRoots ro_roots);
   static inline RootIndex GetMapRootIndex();
   static inline bool Is(Handle<HeapObject> table);
+
+  OBJECT_CONSTRUCTORS(OrderedHashSet, OrderedHashTable<OrderedHashSet, 1>)
 };
 
 class OrderedHashMap : public OrderedHashTable<OrderedHashMap, 2> {
  public:
-  DECL_CAST(OrderedHashMap)
+  DECL_CAST2(OrderedHashMap)
 
   // Returns a value if the OrderedHashMap contains the key, otherwise
   // returns undefined.
@@ -225,6 +229,8 @@ class OrderedHashMap : public OrderedHashTable<OrderedHashMap, 2> {
   static inline bool Is(Handle<HeapObject> table);
 
   static const int kValueOffset = 1;
+
+  OBJECT_CONSTRUCTORS(OrderedHashMap, OrderedHashTable<OrderedHashMap, 2>)
 };
 
 // This is similar to the OrderedHashTable, except for the memory
@@ -573,7 +579,7 @@ class OrderedHashSetHandler
 class OrderedNameDictionary
     : public OrderedHashTable<OrderedNameDictionary, 3> {
  public:
-  DECL_CAST(OrderedNameDictionary)
+  DECL_CAST2(OrderedNameDictionary)
 
   static Handle<OrderedNameDictionary> Add(Isolate* isolate,
                                            Handle<OrderedNameDictionary> table,
@@ -598,6 +604,9 @@ class OrderedNameDictionary
 
   static const int kValueOffset = 1;
   static const int kPropertyDetailsOffset = 2;
+
+  OBJECT_CONSTRUCTORS(OrderedNameDictionary,
+                      OrderedHashTable<OrderedNameDictionary, 3>)
 };
 
 class SmallOrderedNameDictionary