[ubsan] Be more explicit about casting ObjectPtr

Explicitly disallow implicit casting of ObjectPtr to bool to match
clang's and MSVC's behavior.
Introduce a few function overloads using ObjectPtr instead of Object*.
Fix printing of ObjectPtr for objects-printer.cc and GTest.

Bug: v8:3770
Change-Id: I3c3580d363ae6d9fe8f743c6151abc11a915f05c
Reviewed-on: https://chromium-review.googlesource.com/c/1351245
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57928}

src/arm64/macro-assembler-arm64.cc

@@ -402,6 +402,10 @@ void TurboAssembler::Mov(const Register& rd, const Operand& operand,
   }
 }
 
+void TurboAssembler::Mov(const Register& rd, Smi smi) {
+  return Mov(rd, Operand(smi));
+}
+
 void TurboAssembler::Movi16bitHelper(const VRegister& vd, uint64_t imm) {
   DCHECK(is_uint16(imm));
   int byte1 = (imm & 0xFF);

src/arm64/macro-assembler-arm64.h

@@ -233,6 +233,7 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase {
     DCHECK(allow_macro_instructions());
     mov(vd, vd_index, vn, vn_index);
   }
+  void Mov(const Register& rd, Smi smi);
   void Mov(const VRegister& vd, const VRegister& vn, int index) {
     DCHECK(allow_macro_instructions());
     mov(vd, vn, index);

src/global-handles.cc

@@ -74,9 +74,9 @@ class GlobalHandles::Node {
     *first_free = this;
   }
 
-  void Acquire(Object* object) {
+  void Acquire(ObjectPtr object) {
     DCHECK(state() == FREE);
-    object_ = object->ptr();
+    object_ = object.ptr();
     class_id_ = v8::HeapProfiler::kPersistentHandleNoClassId;
     set_independent(false);
     set_active(false);
@@ -526,8 +526,7 @@ GlobalHandles::~GlobalHandles() {
   first_block_ = nullptr;
 }
 
-
-Handle<Object> GlobalHandles::Create(Object* value) {
+Handle<Object> GlobalHandles::Create(ObjectPtr value) {
   if (first_free_ == nullptr) {
     first_block_ = new NodeBlock(this, first_block_);
     first_block_->PutNodesOnFreeList(&first_free_);
@@ -544,8 +543,12 @@ Handle<Object> GlobalHandles::Create(Object* value) {
   return result->handle();
 }
 
+Handle<Object> GlobalHandles::Create(Object* value) {
+  return Create(ObjectPtr(reinterpret_cast<Address>(value)));
+}
+
 Handle<Object> GlobalHandles::Create(Address value) {
-  return Create(reinterpret_cast<Object*>(value));
+  return Create(ObjectPtr(value));
 }
 
 Handle<Object> GlobalHandles::CopyGlobal(Address* location) {

src/global-handles.h

@@ -46,9 +46,9 @@ class GlobalHandles {
   ~GlobalHandles();
 
   // Creates a new global handle that is alive until Destroy is called.
+  // TODO(3770): Drop Object* version.
   Handle<Object> Create(Object* value);
-  // TODO(jkummerow): This and the other Object*/Address overloads below are
-  // temporary. Eventually the respective Object* version should go away.
+  Handle<Object> Create(ObjectPtr value);
   Handle<Object> Create(Address value);
 
   template <typename T>
@@ -58,6 +58,14 @@ class GlobalHandles {
     static_assert(!std::is_same<Object, T>::value, "compiler error");
     return Handle<T>::cast(Create(static_cast<Object*>(value)));
   }
+  template <typename T>
+  Handle<T> Create(T value) {
+    static_assert(std::is_base_of<ObjectPtr, T>::value,
+                  "static type violation");
+    // The compiler should only pick this method if T is not Object.
+    static_assert(!std::is_same<ObjectPtr, T>::value, "compiler error");
+    return Handle<T>::cast(Create(ObjectPtr(value)));
+  }
 
   // Copy a global handle
   static Handle<Object> CopyGlobal(Address* location);

src/heap/spaces-inl.h

@@ -167,6 +167,11 @@ bool PagedSpace::Contains(Object* o) {
   return Page::FromAddress(HeapObject::cast(o)->address())->owner() == this;
 }
 
+bool PagedSpace::Contains(ObjectPtr o) {
+  if (!o.IsHeapObject()) return false;
+  return Page::FromAddress(o.ptr())->owner() == this;
+}
+
 void PagedSpace::UnlinkFreeListCategories(Page* page) {
   DCHECK_EQ(this, page->owner());
   page->ForAllFreeListCategories([this](FreeListCategory* category) {

src/heap/spaces.h

@@ -2077,7 +2077,9 @@ class V8_EXPORT_PRIVATE PagedSpace
 
   // Checks whether an object/address is in this space.
   inline bool Contains(Address a);
+  // TODO(3770): Drop Object* version.
   inline bool Contains(Object* o);
+  inline bool Contains(ObjectPtr o);
   bool ContainsSlow(Address addr);
 
   // Does the space need executable memory?

src/objects-printer.cc

@@ -61,6 +61,11 @@ namespace internal {
 
 #ifdef OBJECT_PRINT
 
+std::ostream& operator<<(std::ostream& os, const ObjectPtr o) {
+  os << reinterpret_cast<void*>(o.ptr());
+  return os;
+}
+
 void Object::Print() {
   StdoutStream os;
   this->Print(os);

src/objects/heap-object.h

@@ -25,6 +25,8 @@ class ObjectPtr {
 
   // Enable incremental transition.
   operator Object*() const { return reinterpret_cast<Object*>(ptr()); }
+  // Make clang on Linux catch what MSVC complains about on Windows:
+  operator bool() const = delete;
 
   bool operator==(const ObjectPtr other) const {
     return this->ptr() == other.ptr();

src/value-serializer.cc

@@ -1004,8 +1004,8 @@ ValueDeserializer::ValueDeserializer(Isolate* isolate,
       position_(data.start()),
       end_(data.start() + data.length()),
       pretenure_(data.length() > kPretenureThreshold ? TENURED : NOT_TENURED),
-      id_map_(Handle<FixedArray>::cast(isolate->global_handles()->Create(
-          ReadOnlyRoots(isolate_).empty_fixed_array()))) {}
+      id_map_(isolate->global_handles()->Create(
+          ReadOnlyRoots(isolate_).empty_fixed_array())) {}
 
 ValueDeserializer::~ValueDeserializer() {
   GlobalHandles::Destroy(id_map_.location());
@@ -1134,9 +1134,8 @@ bool ValueDeserializer::ReadRawBytes(size_t length, const void** data) {
 void ValueDeserializer::TransferArrayBuffer(
     uint32_t transfer_id, Handle<JSArrayBuffer> array_buffer) {
   if (array_buffer_transfer_map_.is_null()) {
-    array_buffer_transfer_map_ =
-        Handle<SimpleNumberDictionary>::cast(isolate_->global_handles()->Create(
-            *SimpleNumberDictionary::New(isolate_, 0)));
+    array_buffer_transfer_map_ = isolate_->global_handles()->Create(
+        *SimpleNumberDictionary::New(isolate_, 0));
   }
   Handle<SimpleNumberDictionary> dictionary =
       array_buffer_transfer_map_.ToHandleChecked();
@@ -1144,8 +1143,8 @@ void ValueDeserializer::TransferArrayBuffer(
       isolate_, dictionary, transfer_id, array_buffer);
   if (!new_dictionary.is_identical_to(dictionary)) {
     GlobalHandles::Destroy(dictionary.location());
-    array_buffer_transfer_map_ = Handle<SimpleNumberDictionary>::cast(
-        isolate_->global_handles()->Create(*new_dictionary));
+    array_buffer_transfer_map_ =
+        isolate_->global_handles()->Create(*new_dictionary);
   }
 }
 
@@ -2058,8 +2057,7 @@ void ValueDeserializer::AddObjectWithID(uint32_t id,
   // If the dictionary was reallocated, update the global handle.
   if (!new_array.is_identical_to(id_map_)) {
     GlobalHandles::Destroy(id_map_.location());
-    id_map_ = Handle<FixedArray>::cast(
-        isolate_->global_handles()->Create(*new_array));
+    id_map_ = isolate_->global_handles()->Create(*new_array);
   }
 }
 

test/unittests/test-utils.h

@@ -312,6 +312,14 @@ class SaveFlags {
   DISALLOW_COPY_AND_ASSIGN(SaveFlags);
 };
 
+// For GTest.
+inline void PrintTo(ObjectPtr o, ::std::ostream* os) {
+  *os << reinterpret_cast<void*>(o.ptr());
+}
+inline void PrintTo(Smi o, ::std::ostream* os) {
+  *os << reinterpret_cast<void*>(o.ptr());
+}
+
 }  // namespace internal
 }  // namespace v8