[ubsan,snapshot] Replace Object** in src/snapshot/

as part of the ongoing quest to get rid of Object*/Object** entirely.
Turns out the Deserializer was actually using unaligned MaybeObject**
pointers, which is undefined behavior. This patch makes the unaligned
values obvious (as "UnalignedSlot") and safe.

Bug: v8:3770
Change-Id: I20f2cca10cc025fa4867e56d9d740a3653837749
Reviewed-on: https://chromium-review.googlesource.com/c/1295792
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56924}

src/objects/slots.h

@@ -18,24 +18,30 @@ class SlotBase {
     return *static_cast<Subclass*>(this);
   }
 
-  bool operator<(const SlotBase& other) { return ptr_ < other.ptr_; }
-  bool operator<=(const SlotBase& other) { return ptr_ <= other.ptr_; }
-  bool operator==(const SlotBase& other) { return ptr_ == other.ptr_; }
-  size_t operator-(const SlotBase& other) {
+  bool operator<(const SlotBase& other) const { return ptr_ < other.ptr_; }
+  bool operator<=(const SlotBase& other) const { return ptr_ <= other.ptr_; }
+  bool operator==(const SlotBase& other) const { return ptr_ == other.ptr_; }
+  bool operator!=(const SlotBase& other) const { return ptr_ != other.ptr_; }
+  size_t operator-(const SlotBase& other) const {
     DCHECK_GE(ptr_, other.ptr_);
     return static_cast<size_t>((ptr_ - other.ptr_) / kPointerSize);
   }
-  Subclass operator+(int i) { return Subclass(ptr_ + i * kPointerSize); }
+  Subclass operator-(int i) const { return Subclass(ptr_ - i * kPointerSize); }
+  Subclass operator+(int i) const { return Subclass(ptr_ + i * kPointerSize); }
   Subclass& operator+=(int i) {
     ptr_ += i * kPointerSize;
     return *static_cast<Subclass*>(this);
   }
 
-  ValueType operator*() { return *reinterpret_cast<ValueType*>(address()); }
+  ValueType operator*() const {
+    return *reinterpret_cast<ValueType*>(address());
+  }
   void store(ValueType value) {
     *reinterpret_cast<ValueType*>(address()) = value;
   }
 
+  void* ToVoidPtr() const { return reinterpret_cast<void*>(address()); }
+
   Address address() const { return ptr_; }
 
  protected:
@@ -63,8 +69,6 @@ class ObjectSlot : public SlotBase<ObjectSlot, Object*> {
   template <typename T, typename U>
   explicit ObjectSlot(SlotBase<T, U> slot) : SlotBase(slot.address()) {}
 
-  void* ToVoidPtr() const { return reinterpret_cast<void*>(address()); }
-
   inline Object* Relaxed_Load() const;
   inline Object* Relaxed_Load(int offset) const;
   inline void Relaxed_Store(int offset, Object* value) const;

src/snapshot/deserializer.h

@@ -18,6 +18,7 @@ namespace internal {
 class AllocationSite;
 class HeapObject;
 class Object;
+class UnalignedSlot;
 
 // Used for platforms with embedded constant pools to trigger deserialization
 // of objects found in code.
@@ -105,33 +106,30 @@ class Deserializer : public SerializerDeserializer {
 
   void Synchronize(VisitorSynchronization::SyncTag tag) override;
 
-  template <typename T>
-  void UnalignedCopy(T** dest, T** src) {
-    DCHECK(!allocator()->next_reference_is_weak());
-    memcpy(dest, src, sizeof(*src));
-  }
+  void UnalignedCopy(UnalignedSlot dest, MaybeObject* value);
+  void UnalignedCopy(UnalignedSlot dest, Address value);
 
   // Fills in some heap data in an area from start to end (non-inclusive).  The
   // space id is used for the write barrier.  The object_address is the address
   // of the object we are writing into, or nullptr if we are not writing into an
   // object, i.e. if we are writing a series of tagged values that are not on
   // the heap. Return false if the object content has been deferred.
-  bool ReadData(MaybeObject** start, MaybeObject** end, int space,
+  bool ReadData(UnalignedSlot start, UnalignedSlot end, int space,
                 Address object_address);
 
   // A helper function for ReadData, templatized on the bytecode for efficiency.
   // Returns the new value of {current}.
   template <int where, int how, int within, int space_number_if_any>
-  inline MaybeObject** ReadDataCase(Isolate* isolate, MaybeObject** current,
+  inline UnalignedSlot ReadDataCase(Isolate* isolate, UnalignedSlot current,
                                     Address current_object_address, byte data,
                                     bool write_barrier_needed);
 
   // A helper function for ReadData for reading external references.
   // Returns the new value of {current}.
-  inline void** ReadExternalReferenceCase(HowToCode how, void** current,
-                                          Address current_object_address);
+  inline UnalignedSlot ReadExternalReferenceCase(
+      HowToCode how, UnalignedSlot current, Address current_object_address);
 
-  void ReadObject(int space_number, MaybeObject** write_back,
+  void ReadObject(int space_number, UnalignedSlot write_back,
                   HeapObjectReferenceType reference_type);
 
   // Special handling for serialized code like hooking up internalized strings.

src/snapshot/serializer.cc

@@ -740,17 +740,14 @@ void Serializer<AllocatorT>::ObjectSerializer::VisitPointers(HeapObject* host,
 
 template <class AllocatorT>
 void Serializer<AllocatorT>::ObjectSerializer::VisitPointers(
-    HeapObject* host, MaybeObjectSlot start_slot, MaybeObjectSlot end_slot) {
-  // TODO(3770): Migrate away from MaybeObject*/MaybeObject**.
-  MaybeObject** start = reinterpret_cast<MaybeObject**>(start_slot.address());
-  MaybeObject** end = reinterpret_cast<MaybeObject**>(end_slot.address());
-  MaybeObject** current = start;
+    HeapObject* host, MaybeObjectSlot start, MaybeObjectSlot end) {
+  MaybeObjectSlot current = start;
   while (current < end) {
     while (current < end && ((*current)->IsSmi() || (*current)->IsCleared())) {
-      current++;
+      ++current;
     }
     if (current < end) {
-      OutputRawData(reinterpret_cast<Address>(current));
+      OutputRawData(current.address());
     }
     HeapObject* current_contents;
     HeapObjectReferenceType reference_type;
@@ -763,12 +760,12 @@ void Serializer<AllocatorT>::ObjectSerializer::VisitPointers(
           serializer_->root_index_map()->Lookup(current_contents,
                                                 &root_index) &&
           RootsTable::IsImmortalImmovable(root_index) &&
-          *current == current[-1]) {
+          *current == *(current - 1)) {
         DCHECK_EQ(reference_type, HeapObjectReferenceType::STRONG);
         DCHECK(!Heap::InNewSpace(current_contents));
         int repeat_count = 1;
-        while (&current[repeat_count] < end - 1 &&
-               current[repeat_count] == *current) {
+        while (current + repeat_count < end - 1 &&
+               *(current + repeat_count) == *current) {
           repeat_count++;
         }
         current += repeat_count;
@@ -786,7 +783,7 @@ void Serializer<AllocatorT>::ObjectSerializer::VisitPointers(
         serializer_->SerializeObject(current_contents, kPlain, kStartOfObject,
                                      0);
         bytes_processed_so_far_ += kPointerSize;
-        current++;
+        ++current;
       }
     }
   }