[ubsan] Drop old NeverReadOnlySpaceObject class

Two uses in the API needed adaptation; all other uses have already
been subsumed by the new implementation (previously known as
NeverReadOnlySpaceObjectPtr, here renamed to NeverReadOnlySpaceObject).

Bug: v8:3770
Change-Id: Idf0e4a98a407b9afea22e8790da34cf017b892a5
Reviewed-on: https://chromium-review.googlesource.com/c/1397671
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58620}

include/v8-internal.h

@@ -20,6 +20,8 @@ class Isolate;
 
 namespace internal {
 
+class Isolate;
+
 typedef uintptr_t Address;
 static const Address kNullAddress = 0;
 
@@ -361,6 +363,10 @@ V8_INLINE void PerformCastCheck(T* data) {
   CastCheck<std::is_base_of<Data, T>::value>::Perform(data);
 }
 
+// {obj} must be the raw tagged pointer representation of a HeapObject
+// that's guaranteed to never be in ReadOnlySpace.
+V8_EXPORT internal::Isolate* IsolateFromNeverReadOnlySpaceObject(Address obj);
+
 }  // namespace internal
 }  // namespace v8
 

include/v8.h

@@ -869,10 +869,6 @@ class V8_EXPORT HandleScope {
   void operator delete(void*, size_t);
   void operator delete[](void*, size_t);
 
-  // Uses heap_object to obtain the current Isolate.
-  static internal::Address* CreateHandle(
-      internal::NeverReadOnlySpaceObject* heap_object, internal::Address value);
-
   internal::Isolate* isolate_;
   internal::Address* prev_next_;
   internal::Address* prev_limit_;
@@ -9935,8 +9931,9 @@ Local<Value> Object::GetInternalField(int index) {
     int offset = I::kJSObjectHeaderSizeForEmbedderFields +
                  (I::kEmbedderDataSlotSize * index);
     A value = I::ReadTaggedAnyField(obj, offset);
-    A* result = HandleScope::CreateHandle(
-        reinterpret_cast<internal::NeverReadOnlySpaceObject*>(obj), value);
+    internal::Isolate* isolate =
+        internal::IsolateFromNeverReadOnlySpaceObject(obj);
+    A* result = HandleScope::CreateHandle(isolate, value);
     return Local<Value>(reinterpret_cast<Value*>(result));
   }
 #endif
@@ -10556,9 +10553,10 @@ Local<Value> Context::GetEmbedderData(int index) {
 #if !defined(V8_ENABLE_CHECKS) && !defined(V8_COMPRESS_POINTERS)
   typedef internal::Address A;
   typedef internal::Internals I;
-  auto* context = *reinterpret_cast<internal::NeverReadOnlySpaceObject**>(this);
+  internal::Isolate* isolate = internal::IsolateFromNeverReadOnlySpaceObject(
+      *reinterpret_cast<A*>(this));
   A* result =
-      HandleScope::CreateHandle(context, I::ReadEmbedderData<A>(this, index));
+      HandleScope::CreateHandle(isolate, I::ReadEmbedderData<A>(this, index));
   return Local<Value>(reinterpret_cast<Value*>(result));
 #else
   return SlowGetEmbedderData(index);

src/api.cc

@@ -1109,14 +1109,6 @@ i::Address* HandleScope::CreateHandle(i::Isolate* isolate, i::Address value) {
   return i::HandleScope::CreateHandle(isolate, value);
 }
 
-i::Address* HandleScope::CreateHandle(
-    i::NeverReadOnlySpaceObject* writable_object, i::Address value) {
-  DCHECK(
-      i::Object(reinterpret_cast<i::Address>(writable_object)).IsHeapObject());
-  return i::HandleScope::CreateHandle(writable_object->GetIsolate(), value);
-}
-
-
 EscapableHandleScope::EscapableHandleScope(Isolate* v8_isolate) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
   escape_slot_ =
@@ -3618,6 +3610,10 @@ MaybeLocal<Uint32> Value::ToUint32(Local<Context> context) const {
   RETURN_ESCAPED(result);
 }
 
+i::Isolate* i::IsolateFromNeverReadOnlySpaceObject(i::Address obj) {
+  return i::NeverReadOnlySpaceObject::GetIsolate(
+      i::HeapObject::cast(i::Object(obj)));
+}
 
 void i::Internals::CheckInitializedImpl(v8::Isolate* external_isolate) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(external_isolate);

src/compiler/node-matchers.h

@@ -13,6 +13,7 @@
 #include "src/double.h"
 #include "src/external-reference.h"
 #include "src/globals.h"
+#include "src/objects/heap-object.h"
 
 namespace v8 {
 namespace internal {

src/global-handles.h

@@ -12,6 +12,7 @@
 #include "include/v8-profiler.h"
 
 #include "src/handles.h"
+#include "src/objects.h"
 #include "src/utils.h"
 
 namespace v8 {

src/handles.h

@@ -12,12 +12,6 @@
 #include "src/base/macros.h"
 #include "src/checks.h"
 #include "src/globals.h"
-// TODO(3770): The objects.h and heap-object.h includes are required to make
-// the std::enable_if<std::is_base_of<...>> conditions below work. Once the
-// migration is complete, we should be able to get by with just forward
-// declarations.
-#include "src/objects.h"
-#include "src/objects/heap-object.h"
 #include "src/zone/zone.h"
 
 namespace v8 {
@@ -128,29 +122,11 @@ class Handle final : public HandleBase {
                             std::is_convertible<S*, T*>::value>::type>
   V8_INLINE Handle(Handle<S> handle) : HandleBase(handle) {}
 
-  // The NeverReadOnlySpaceObject special-case is needed for the
-  // ContextFromNeverReadOnlySpaceObject helper function in api.cc.
-  template <typename T1 = T,
-            typename = typename std::enable_if<
-                std::is_base_of<NeverReadOnlySpaceObject, T1>::value>::type>
-  V8_INLINE T* operator->() const {
-    return operator*();
-  }
-  template <typename T1 = T, typename = typename std::enable_if<
-                                 std::is_base_of<Object, T1>::value>::type>
   V8_INLINE T operator->() const {
     return operator*();
   }
 
   // Provides the C++ dereference operator.
-  template <typename T1 = T,
-            typename = typename std::enable_if<
-                std::is_base_of<NeverReadOnlySpaceObject, T1>::value>::type>
-  V8_INLINE T* operator*() const {
-    return reinterpret_cast<T*>(HandleBase::operator*());
-  }
-  template <typename T1 = T, typename = typename std::enable_if<
-                                 std::is_base_of<Object, T1>::value>::type>
   V8_INLINE T operator*() const {
     // unchecked_cast because we rather trust Handle<T> to contain a T than
     // include all the respective -inl.h headers for SLOW_DCHECKs.

src/objects-inl.h

@@ -754,20 +754,6 @@ ReadOnlyRoots HeapObject::GetReadOnlyRoots() const {
   return ReadOnlyRoots(MemoryChunk::FromHeapObject(*this)->heap());
 }
 
-Heap* NeverReadOnlySpaceObject::GetHeap() const {
-  MemoryChunk* chunk =
-      MemoryChunk::FromAddress(reinterpret_cast<Address>(this));
-  // Make sure we are not accessing an object in RO space.
-  SLOW_DCHECK(chunk->owner()->identity() != RO_SPACE);
-  Heap* heap = chunk->heap();
-  SLOW_DCHECK(heap != nullptr);
-  return heap;
-}
-
-Isolate* NeverReadOnlySpaceObject::GetIsolate() const {
-  return GetHeap()->isolate();
-}
-
 Map HeapObject::map() const { return map_word().ToMap(); }
 
 void HeapObject::set_map(Map value) {

src/objects.h

@@ -1102,18 +1102,6 @@ class MapWord {
   Address value_;
 };
 
-// Mixin class for objects that can never be in RO space.
-// TODO(leszeks): Add checks in the factory that we never allocate these objects
-// in RO space.
-class NeverReadOnlySpaceObject {
- public:
-  // The Heap the object was allocated in. Used also to access Isolate.
-  inline Heap* GetHeap() const;
-
-  // Convenience method to get current isolate.
-  inline Isolate* GetIsolate() const;
-};
-
 template <int start_offset, int end_offset, int size>
 class FixedBodyDescriptor;
 

src/objects/embedder-data-array.h

@@ -7,7 +7,7 @@
 
 #include "src/globals.h"
 #include "src/maybe-handles.h"
-#include "src/objects.h"
+#include "src/objects/heap-object.h"
 
 // Has to be the last include (doesn't have include guards):
 #include "src/objects/object-macros.h"

src/objects/heap-object-inl.h

@@ -27,11 +27,11 @@ HeapObject HeapObject::FromAddress(Address address) {
   return HeapObject(address + kHeapObjectTag);
 }
 
-Heap* NeverReadOnlySpaceObjectPtr::GetHeap(const HeapObject object) {
+Heap* NeverReadOnlySpaceObject::GetHeap(const HeapObject object) {
   return GetHeapFromWritableObject(object);
 }
 
-Isolate* NeverReadOnlySpaceObjectPtr::GetIsolate(const HeapObject object) {
+Isolate* NeverReadOnlySpaceObject::GetIsolate(const HeapObject object) {
   return GetHeap(object)->isolate();
 }
 

src/objects/heap-object.h

@@ -192,13 +192,8 @@ class HeapObject : public Object {
   OBJECT_CONSTRUCTORS(HeapObject, Object);
 };
 
-// Replacement for NeverReadOnlySpaceObject, temporarily separate for
-// incremental transition.
 // Helper class for objects that can never be in RO space.
-// TODO(leszeks): Add checks in the factory that we never allocate these objects
-// in RO space.
-// TODO(3770): Get rid of the duplication.
-class NeverReadOnlySpaceObjectPtr {
+class NeverReadOnlySpaceObject {
  public:
   // The Heap the object was allocated in. Used also to access Isolate.
   static inline Heap* GetHeap(const HeapObject object);

src/objects/object-macros.h

@@ -34,12 +34,14 @@
   inline Heap* GetHeap() const; \
   inline Isolate* GetIsolate() const;
 
+// TODO(leszeks): Add checks in the factory that we never allocate these
+// objects in RO space.
 #define NEVER_READ_ONLY_SPACE_IMPL(Type)                \
   Heap* Type::GetHeap() const {                         \
-    return NeverReadOnlySpaceObjectPtr::GetHeap(*this);    \
+    return NeverReadOnlySpaceObject::GetHeap(*this);    \
   }                                                     \
   Isolate* Type::GetIsolate() const {                   \
-    return NeverReadOnlySpaceObjectPtr::GetIsolate(*this); \
+    return NeverReadOnlySpaceObject::GetIsolate(*this); \
   }
 
 #define DECL_PRIMITIVE_ACCESSORS(name, type) \