[ubsan] Port FieldType to the new design

Bug: v8:3770
Change-Id: I6e2782a7f8589c466b54987c850d41d4ff5f6489
Reviewed-on: https://chromium-review.googlesource.com/c/1316618Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57292}

src/field-type.cc

@@ -13,16 +13,10 @@ namespace v8 {
 namespace internal {
 
 // static
-FieldType* FieldType::None() {
-  // Do not Smi::kZero here or for Any(), as that may translate
-  // as `nullptr` which is not a valid value for `this`.
-  return reinterpret_cast<FieldType*>(Smi::FromInt(2).ptr());
-}
+FieldType FieldType::None() { return FieldType(Smi::FromInt(2).ptr()); }
 
 // static
-FieldType* FieldType::Any() {
-  return reinterpret_cast<FieldType*>(Smi::FromInt(1).ptr());
-}
+FieldType FieldType::Any() { return FieldType(Smi::FromInt(1).ptr()); }
 
 // static
 Handle<FieldType> FieldType::None(Isolate* isolate) {
@@ -35,43 +29,43 @@ Handle<FieldType> FieldType::Any(Isolate* isolate) {
 }
 
 // static
-FieldType* FieldType::Class(i::Map* map) { return FieldType::cast(map); }
+FieldType FieldType::Class(Map* map) { return FieldType::cast(map); }
 
 // static
-Handle<FieldType> FieldType::Class(i::Handle<i::Map> map, Isolate* isolate) {
+Handle<FieldType> FieldType::Class(Handle<Map> map, Isolate* isolate) {
   return handle(Class(*map), isolate);
 }
 
 // static
-FieldType* FieldType::cast(Object* object) {
+FieldType FieldType::cast(Object* object) {
   DCHECK(object == None() || object == Any() || object->IsMap());
-  return reinterpret_cast<FieldType*>(object);
+  return FieldType(object->ptr());
 }
 
-bool FieldType::IsClass() { return this->IsMap(); }
+bool FieldType::IsClass() const { return this->IsMap(); }
 
-Map* FieldType::AsClass() {
+Map* FieldType::AsClass() const {
   DCHECK(IsClass());
-  return Map::cast(this);
+  return Map::cast(*this);
 }
 
-bool FieldType::NowStable() {
+bool FieldType::NowStable() const {
   return !this->IsClass() || AsClass()->is_stable();
 }
 
-bool FieldType::NowIs(FieldType* other) {
+bool FieldType::NowIs(FieldType other) const {
   if (other->IsAny()) return true;
   if (IsNone()) return true;
   if (other->IsNone()) return false;
   if (IsAny()) return false;
   DCHECK(IsClass());
   DCHECK(other->IsClass());
-  return this == other;
+  return *this == other;
 }
 
-bool FieldType::NowIs(Handle<FieldType> other) { return NowIs(*other); }
+bool FieldType::NowIs(Handle<FieldType> other) const { return NowIs(*other); }
 
-void FieldType::PrintTo(std::ostream& os) {
+void FieldType::PrintTo(std::ostream& os) const {
   if (IsAny()) {
     os << "Any";
   } else if (IsNone()) {
@@ -82,11 +76,11 @@ void FieldType::PrintTo(std::ostream& os) {
   }
 }
 
-bool FieldType::NowContains(Object* value) {
-  if (this == Any()) return true;
-  if (this == None()) return false;
+bool FieldType::NowContains(Object* value) const {
+  if (*this == Any()) return true;
+  if (*this == None()) return false;
   if (!value->IsHeapObject()) return false;
-  return HeapObject::cast(value)->map() == Map::cast(this);
+  return HeapObject::cast(value)->map() == Map::cast(*this);
 }
 
 }  // namespace internal

src/field-type.h

@@ -6,6 +6,7 @@
 #define V8_FIELD_TYPE_H_
 
 #include "src/objects.h"
+#include "src/objects/heap-object.h"
 #include "src/objects/map.h"
 
 namespace v8 {
@@ -14,29 +15,35 @@ namespace internal {
 template <typename T>
 class Handle;
 
-class FieldType : public Object {
+class FieldType : public ObjectPtr {
  public:
-  static FieldType* None();
-  static FieldType* Any();
+  static FieldType None();
+  static FieldType Any();
   static Handle<FieldType> None(Isolate* isolate);
   static Handle<FieldType> Any(Isolate* isolate);
-  static FieldType* Class(i::Map* map);
-  static Handle<FieldType> Class(i::Handle<i::Map> map, Isolate* isolate);
-  static FieldType* cast(Object* object);
+  static FieldType Class(Map* map);
+  static Handle<FieldType> Class(Handle<Map> map, Isolate* isolate);
+  static FieldType cast(Object* object);
 
-  bool NowContains(Object* value);
+  bool NowContains(Object* value) const;
 
-  bool NowContains(Handle<Object> value) { return NowContains(*value); }
+  bool NowContains(Handle<Object> value) const { return NowContains(*value); }
 
-  bool IsClass();
-  Map* AsClass();
-  bool IsNone() { return this == None(); }
-  bool IsAny() { return this == Any(); }
-  bool NowStable();
-  bool NowIs(FieldType* other);
-  bool NowIs(Handle<FieldType> other);
+  bool IsClass() const;
+  Map* AsClass() const;
+  bool IsNone() const { return *this == None(); }
+  bool IsAny() const { return *this == Any(); }
+  bool NowStable() const;
+  bool NowIs(FieldType other) const;
+  bool NowIs(Handle<FieldType> other) const;
 
-  void PrintTo(std::ostream& os);
+  void PrintTo(std::ostream& os) const;
+
+  FieldType* operator->() { return this; }
+  const FieldType* operator->() const { return this; }
+
+ private:
+  explicit constexpr FieldType(Address ptr) : ObjectPtr(ptr) {}
 };
 
 }  // namespace internal

src/ic/accessor-assembler.cc

@@ -1063,8 +1063,8 @@ void AccessorAssembler::CheckFieldType(TNode<DescriptorArray> descriptors,
     GotoIf(TaggedIsSmi(value), bailout);
     TNode<MaybeObject> field_type = LoadFieldTypeByKeyIndex(
         descriptors, UncheckedCast<IntPtrT>(name_index));
-    intptr_t kNoneType = reinterpret_cast<intptr_t>(FieldType::None());
-    intptr_t kAnyType = reinterpret_cast<intptr_t>(FieldType::Any());
+    const Address kNoneType = FieldType::None().ptr();
+    const Address kAnyType = FieldType::Any().ptr();
     DCHECK_NE(kNoneType, kClearedWeakHeapObject);
     DCHECK_NE(kAnyType, kClearedWeakHeapObject);
     // FieldType::None can't hold any value.

src/map-updater.cc

@@ -61,7 +61,7 @@ Object* MapUpdater::GetValue(int descriptor) const {
   return old_descriptors_->GetStrongValue(descriptor);
 }
 
-FieldType* MapUpdater::GetFieldType(int descriptor) const {
+FieldType MapUpdater::GetFieldType(int descriptor) const {
   DCHECK_LE(0, descriptor);
   if (descriptor == modified_descriptor_) {
     DCHECK_EQ(kField, new_location_);
@@ -284,7 +284,7 @@ MapUpdater::State MapUpdater::FindRootMap() {
     DCHECK_EQ(kData, old_details.kind());
     DCHECK_EQ(kData, new_kind_);
     DCHECK_EQ(kField, new_location_);
-    FieldType* old_field_type =
+    FieldType old_field_type =
         old_descriptors_->GetFieldType(modified_descriptor_);
     if (!new_field_type_->NowIs(old_field_type)) {
       return CopyGeneralizeAllFields("GenAll_RootModification5");
@@ -628,7 +628,7 @@ Handle<Map> MapUpdater::FindSplitMap(Handle<DescriptorArray> descriptors) {
     if (!details.representation().Equals(next_details.representation())) break;
 
     if (next_details.location() == kField) {
-      FieldType* next_field_type = next_descriptors->GetFieldType(i);
+      FieldType next_field_type = next_descriptors->GetFieldType(i);
       if (!descriptors->GetFieldType(i)->NowIs(next_field_type)) {
         break;
       }

src/map-updater.h

@@ -120,7 +120,7 @@ class MapUpdater {
 
   // Returns field type for a |descriptor| with kField location in "updated"
   // |old_descrtiptors_| array.
-  inline FieldType* GetFieldType(int descriptor) const;
+  inline FieldType GetFieldType(int descriptor) const;
 
   // If a |descriptor| property in "updated" |old_descriptors_| has kField
   // location then returns it's field type otherwise computes optimal field

src/maybe-handles.h

@@ -31,15 +31,16 @@ class MaybeHandle final {
   template <typename S, typename = typename std::enable_if<
                             std::is_convertible<S*, T*>::value ||
                             std::is_same<T, Object>::value>::type>
-  V8_INLINE MaybeHandle(Handle<S> handle)
-      : location_(reinterpret_cast<T**>(handle.location_)) {}
+  V8_INLINE MaybeHandle(Handle<S> handle) : location_(handle.location_) {}
 
   // Constructor for handling automatic up casting.
   // Ex. MaybeHandle<JSArray> can be passed when Handle<Object> is expected.
+  // TODO(3770): Remove T==Object special case after the migration.
   template <typename S, typename = typename std::enable_if<
-                            std::is_convertible<S*, T*>::value>::type>
+                            std::is_convertible<S*, T*>::value ||
+                            std::is_same<T, Object>::value>::type>
   V8_INLINE MaybeHandle(MaybeHandle<S> maybe_handle)
-      : location_(reinterpret_cast<T**>(maybe_handle.location_)) {}
+      : location_(maybe_handle.location_) {}
 
   V8_INLINE MaybeHandle(T* object, Isolate* isolate);
 
@@ -65,12 +66,14 @@ class MaybeHandle final {
 
   // Returns the raw address where this handle is stored. This should only be
   // used for hashing handles; do not ever try to dereference it.
-  V8_INLINE Address address() const { return bit_cast<Address>(location_); }
+  V8_INLINE Address address() const {
+    return reinterpret_cast<Address>(location_);
+  }
 
   bool is_null() const { return location_ == nullptr; }
 
  protected:
-  T** location_ = nullptr;
+  Address* location_ = nullptr;
 
   // MaybeHandles of different classes are allowed to access each
   // other's location_.

src/objects-debug.cc

@@ -574,7 +574,7 @@ void JSObject::JSObjectVerify(Isolate* isolate) {
         if (value->IsUninitialized(isolate)) continue;
         if (r.IsSmi()) DCHECK(value->IsSmi());
         if (r.IsHeapObject()) DCHECK(value->IsHeapObject());
-        FieldType* field_type = descriptors->GetFieldType(i);
+        FieldType field_type = descriptors->GetFieldType(i);
         bool type_is_none = field_type->IsNone();
         bool type_is_any = field_type->IsAny();
         if (r.IsNone()) {

src/objects-inl.h

@@ -1328,7 +1328,7 @@ int DescriptorArray::GetFieldIndex(int descriptor_number) {
   return GetDetails(descriptor_number).field_index();
 }
 
-FieldType* DescriptorArray::GetFieldType(int descriptor_number) {
+FieldType DescriptorArray::GetFieldType(int descriptor_number) {
   DCHECK_EQ(GetDetails(descriptor_number).location(), kField);
   MaybeObject wrapped_type = GetValue(descriptor_number);
   return Map::UnwrapFieldType(wrapped_type);

src/objects-printer.cc

@@ -2378,7 +2378,7 @@ void DescriptorArray::PrintDescriptorDetails(std::ostream& os, int descriptor,
   os << " @ ";
   switch (details.location()) {
     case kField: {
-      FieldType* field_type = GetFieldType(descriptor);
+      FieldType field_type = GetFieldType(descriptor);
       field_type->PrintTo(os);
       break;
     }

src/objects.cc

@@ -3993,7 +3993,7 @@ MaybeObjectHandle Map::WrapFieldType(Isolate* isolate, Handle<FieldType> type) {
 }
 
 // static
-FieldType* Map::UnwrapFieldType(MaybeObject wrapped_type) {
+FieldType Map::UnwrapFieldType(MaybeObject wrapped_type) {
   if (wrapped_type->IsCleared()) {
     return FieldType::None();
   }
@@ -4810,7 +4810,7 @@ void Map::UpdateFieldType(Isolate* isolate, int descriptor, Handle<Name> name,
   }
 }
 
-bool FieldTypeIsCleared(Representation rep, FieldType* type) {
+bool FieldTypeIsCleared(Representation rep, FieldType type) {
   return type->IsNone() && rep.IsHeapObject();
 }
 
@@ -4980,7 +4980,7 @@ Map* Map::TryReplayPropertyTransitions(Isolate* isolate, Map* old_map) {
     }
     if (new_details.location() == kField) {
       if (new_details.kind() == kData) {
-        FieldType* new_type = new_descriptors->GetFieldType(i);
+        FieldType new_type = new_descriptors->GetFieldType(i);
         // Cleared field types need special treatment. They represent lost
         // knowledge, so we must first generalize the new_type to "Any".
         if (FieldTypeIsCleared(new_details.representation(), new_type)) {
@@ -4988,7 +4988,7 @@ Map* Map::TryReplayPropertyTransitions(Isolate* isolate, Map* old_map) {
         }
         DCHECK_EQ(kData, old_details.kind());
         if (old_details.location() == kField) {
-          FieldType* old_type = old_descriptors->GetFieldType(i);
+          FieldType old_type = old_descriptors->GetFieldType(i);
           if (FieldTypeIsCleared(old_details.representation(), old_type) ||
               !old_type->NowIs(new_type)) {
             return nullptr;
@@ -5005,7 +5005,7 @@ Map* Map::TryReplayPropertyTransitions(Isolate* isolate, Map* old_map) {
       } else {
         DCHECK_EQ(kAccessor, new_details.kind());
 #ifdef DEBUG
-        FieldType* new_type = new_descriptors->GetFieldType(i);
+        FieldType new_type = new_descriptors->GetFieldType(i);
         DCHECK(new_type->IsAny());
 #endif
         UNREACHABLE();

src/objects/descriptor-array.h

@@ -79,7 +79,7 @@ class DescriptorArray : public WeakFixedArray {
   inline MaybeObjectSlot GetDescriptorEndSlot(int descriptor_number);
   inline PropertyDetails GetDetails(int descriptor_number);
   inline int GetFieldIndex(int descriptor_number);
-  inline FieldType* GetFieldType(int descriptor_number);
+  inline FieldType GetFieldType(int descriptor_number);
 
   inline Name* GetSortedKey(int descriptor_number);
   inline int GetSortedKeyIndex(int descriptor_number);

src/objects/map-inl.h

@@ -89,7 +89,7 @@ InterceptorInfo* Map::GetIndexedInterceptor() {
 
 bool Map::IsInplaceGeneralizableField(PropertyConstness constness,
                                       Representation representation,
-                                      FieldType* field_type) {
+                                      FieldType field_type) {
   if (FLAG_track_constant_fields && FLAG_modify_map_inplace &&
       (constness == PropertyConstness::kConst)) {
     // VariableMode::kConst -> PropertyConstness::kMutable field generalization

src/objects/map.h

@@ -481,7 +481,7 @@ class Map : public HeapObject {
   // by just updating current map.
   static inline bool IsInplaceGeneralizableField(PropertyConstness constness,
                                                  Representation representation,
-                                                 FieldType* field_type);
+                                                 FieldType field_type);
 
   // Generalizes constness, representation and field_type if objects with given
   // instance type can have fast elements that can be transitioned by stubs or
@@ -653,7 +653,7 @@ class Map : public HeapObject {
 
   static MaybeObjectHandle WrapFieldType(Isolate* isolate,
                                          Handle<FieldType> type);
-  static FieldType* UnwrapFieldType(MaybeObject wrapped_type);
+  static FieldType UnwrapFieldType(MaybeObject wrapped_type);
 
   V8_WARN_UNUSED_RESULT static MaybeHandle<Map> CopyWithField(
       Isolate* isolate, Handle<Map> map, Handle<Name> name,

src/objects/maybe-object.h

@@ -78,11 +78,20 @@ class MaybeObject {
   inline Object* GetHeapObjectOrSmi() const;
 
   inline bool IsObject() const;
-  template <typename T>
+  template <typename T, typename = typename std::enable_if<
+                            std::is_base_of<Object, T>::value>::type>
   T* cast() const {
     DCHECK(!HasWeakHeapObjectTag(ptr_));
     return T::cast(reinterpret_cast<Object*>(ptr_));
   }
+  // Replacement for the above, temporarily separate for incremental transition.
+  // TODO(3770): Get rid of the duplication.
+  template <typename T, typename = typename std::enable_if<
+                            std::is_base_of<ObjectPtr, T>::value>::type>
+  T cast() const {
+    DCHECK(!HasWeakHeapObjectTag(ptr_));
+    return T::cast(ObjectPtr(ptr_));
+  }
 
   static MaybeObject FromSmi(Smi smi) {
     DCHECK(HAS_SMI_TAG(smi->ptr()));
@@ -94,6 +103,11 @@ class MaybeObject {
     return MaybeObject(object->ptr());
   }
 
+  static MaybeObject FromObject(ObjectPtr object) {
+    DCHECK(!HasWeakHeapObjectTag(object.ptr()));
+    return MaybeObject(object.ptr());
+  }
+
   static inline MaybeObject MakeWeak(MaybeObject object);
 
 #ifdef VERIFY_HEAP

test/cctest/test-field-type-tracking.cc

@@ -123,7 +123,8 @@ class Expectations {
         constness = PropertyConstness::kMutable;
       }
       if (representation.IsHeapObject() && !FieldType::cast(*value)->IsAny()) {
-        value = FieldType::Any(isolate_);
+        // TODO(3770): Drop extra Handle constructor call after migration.
+        value = Handle<Object>(FieldType::Any(isolate_));
       }
     }
     constnesses_[index] = constness;
@@ -252,7 +253,8 @@ class Expectations {
     CHECK(index < number_of_properties_);
     representations_[index] = Representation::Tagged();
     if (locations_[index] == kField) {
-      values_[index] = FieldType::Any(isolate_);
+      // TODO(3770): Drop extra Handle constructor call after migration.
+      values_[index] = Handle<Object>(FieldType::Any(isolate_));
     }
   }
 
@@ -272,7 +274,7 @@ class Expectations {
     Object* expected_value = *values_[descriptor];
     if (details.location() == kField) {
       if (details.kind() == kData) {
-        FieldType* type = descriptors->GetFieldType(descriptor);
+        FieldType type = descriptors->GetFieldType(descriptor);
         return FieldType::cast(expected_value) == type;
       } else {
         // kAccessor

test/cctest/test-transitions.cc

@@ -32,16 +32,14 @@ TEST(TransitionArray_SimpleFieldTransitions) {
 
   Handle<Map> map0 = Map::Create(isolate, 0);
   Handle<Map> map1 =
-      Map::CopyWithField(isolate, map0, name1,
-                         handle(FieldType::Any(), isolate), attributes,
-                         PropertyConstness::kMutable, Representation::Tagged(),
-                         OMIT_TRANSITION)
+      Map::CopyWithField(isolate, map0, name1, FieldType::Any(isolate),
+                         attributes, PropertyConstness::kMutable,
+                         Representation::Tagged(), OMIT_TRANSITION)
           .ToHandleChecked();
   Handle<Map> map2 =
-      Map::CopyWithField(isolate, map0, name2,
-                         handle(FieldType::Any(), isolate), attributes,
-                         PropertyConstness::kMutable, Representation::Tagged(),
-                         OMIT_TRANSITION)
+      Map::CopyWithField(isolate, map0, name2, FieldType::Any(isolate),
+                         attributes, PropertyConstness::kMutable,
+                         Representation::Tagged(), OMIT_TRANSITION)
           .ToHandleChecked();
 
   CHECK(map0->raw_transitions()->IsSmi());
@@ -91,16 +89,14 @@ TEST(TransitionArray_FullFieldTransitions) {
 
   Handle<Map> map0 = Map::Create(isolate, 0);
   Handle<Map> map1 =
-      Map::CopyWithField(isolate, map0, name1,
-                         handle(FieldType::Any(), isolate), attributes,
-                         PropertyConstness::kMutable, Representation::Tagged(),
-                         OMIT_TRANSITION)
+      Map::CopyWithField(isolate, map0, name1, FieldType::Any(isolate),
+                         attributes, PropertyConstness::kMutable,
+                         Representation::Tagged(), OMIT_TRANSITION)
           .ToHandleChecked();
   Handle<Map> map2 =
-      Map::CopyWithField(isolate, map0, name2,
-                         handle(FieldType::Any(), isolate), attributes,
-                         PropertyConstness::kMutable, Representation::Tagged(),
-                         OMIT_TRANSITION)
+      Map::CopyWithField(isolate, map0, name2, FieldType::Any(isolate),
+                         attributes, PropertyConstness::kMutable,
+                         Representation::Tagged(), OMIT_TRANSITION)
           .ToHandleChecked();
 
   CHECK(map0->raw_transitions()->IsSmi());
@@ -157,9 +153,8 @@ TEST(TransitionArray_DifferentFieldNames) {
     SNPrintF(buffer, "prop%d", i);
     Handle<String> name = factory->InternalizeUtf8String(buffer.start());
     Handle<Map> map =
-        Map::CopyWithField(isolate, map0, name,
-                           handle(FieldType::Any(), isolate), attributes,
-                           PropertyConstness::kMutable,
+        Map::CopyWithField(isolate, map0, name, FieldType::Any(isolate),
+                           attributes, PropertyConstness::kMutable,
                            Representation::Tagged(), OMIT_TRANSITION)
             .ToHandleChecked();
     names[i] = name;
@@ -250,8 +245,7 @@ TEST(TransitionArray_SameFieldNamesDifferentAttributes) {
     SNPrintF(buffer, "prop%d", i);
     Handle<String> name = factory->InternalizeUtf8String(buffer.start());
     Handle<Map> map =
-        Map::CopyWithField(isolate, map0, name,
-                           handle(FieldType::Any(), isolate), NONE,
+        Map::CopyWithField(isolate, map0, name, FieldType::Any(isolate), NONE,
                            PropertyConstness::kMutable,
                            Representation::Tagged(), OMIT_TRANSITION)
             .ToHandleChecked();
@@ -271,9 +265,8 @@ TEST(TransitionArray_SameFieldNamesDifferentAttributes) {
     PropertyAttributes attributes = static_cast<PropertyAttributes>(i);
 
     Handle<Map> map =
-        Map::CopyWithField(isolate, map0, name,
-                           handle(FieldType::Any(), isolate), attributes,
-                           PropertyConstness::kMutable,
+        Map::CopyWithField(isolate, map0, name, FieldType::Any(isolate),
+                           attributes, PropertyConstness::kMutable,
                            Representation::Tagged(), OMIT_TRANSITION)
             .ToHandleChecked();
     attr_maps[i] = map;