[snapshot] Refactor the FixedRawData bytecode

From recent discussions on https://crrev.com/c/2057355 it became clear
that the FixedRawData serializer bytecodes need to be refactored. This
CL

- updates documentation to correctly specify that data length is given
in kTaggedSize unit, and
- changes the way smi roots are serialized s.t. deserialization no
longer needs hacks to be correct on big-endian architectures and when
(theoretically) deserializing sequences of smi roots. We do this by
serializing the full object slot for smi roots instead of just the
smi contents.

Bug: v8:10374
Change-Id: I190172d8030888e1950413e436e43b3d6e3b8fd5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2130276
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Junliang Yan <jyan@ca.ibm.com>
Cr-Commit-Position: refs/heads/master@{#66966}

src/snapshot/deserializer.cc

@@ -751,41 +751,17 @@ bool Deserializer::ReadData(TSlot current, TSlot limit,
         break;
       }
 
-      // Deserialize raw data of fixed length from 1 to 32 words.
+      // Deserialize raw data of fixed length from 1 to 32 times kTaggedSize.
       STATIC_ASSERT(kNumberOfFixedRawData == 32);
       SIXTEEN_CASES(kFixedRawData)
       SIXTEEN_CASES(kFixedRawData + 16) {
-        // This bytecode has become very confusing with recent changes due to
-        // pointer compression. From comments and variable names it implies that
-        // the length unit is words/kPointerSize, but the unit is actually
-        // kTaggedSize since https://chromium-review.googlesource.com/c/1388529.
-        //
-        // Also, contents can be (tagged) Smis or just a raw byte sequence. In
-        // the case of Smis we must be careful when deserializing into full
-        // object slots. It is not valid to deserialize a sequence of >1 Smis
-        // into full object slots in compressed pointer builds.
-        //
-        // Likewise one must pay attention to endianness when deserializing a
-        // smi into a full object slot. That is what the code below is trying to
-        // address.
-        //
-        // The solution below works because we currently never deserialize >1
-        // Smi into full object slots, or raw byte sequences into full object
-        // slots. But those assumptions are fragile.
-        //
-        const int size_in_tagged = data - kFixedRawDataStart;
-        const int size_in_bytes = size_in_tagged * kTaggedSize;
-        Address addr = current.address();
-        DCHECK_IMPLIES(kTaggedSize != TSlot::kSlotDataSize,
-                       size_in_tagged == 1);
-#ifdef V8_TARGET_BIG_ENDIAN
-        if (kTaggedSize != TSlot::kSlotDataSize) {
-          // Should only be reached when deserializing a Smi root.
-          addr += kTaggedSize;
-        }
-#endif
-        source_.CopyRaw(reinterpret_cast<void*>(addr), size_in_bytes);
-        current += size_in_tagged;
+        int size_in_tagged = data - kFixedRawDataStart;
+        source_.CopyRaw(current.ToVoidPtr(), size_in_tagged * kTaggedSize);
+
+        int size_in_bytes = size_in_tagged * kTaggedSize;
+        int size_in_slots = size_in_bytes / TSlot::kSlotDataSize;
+        DCHECK(IsAligned(size_in_bytes, TSlot::kSlotDataSize));
+        current += size_in_slots;
         break;
       }
 

src/snapshot/roots-serializer.cc

@@ -47,7 +47,7 @@ void RootsSerializer::VisitRootPointers(Root root, const char* description,
     // - Only root list elements that have been fully serialized can be
     //   referenced using kRootArray bytecodes.
     for (FullObjectSlot current = start; current < end; ++current) {
-      SerializeRootObject(*current);
+      SerializeRootObject(current);
       size_t root_index = current - roots_table.begin();
       root_has_been_serialized_.set(root_index);
     }

src/snapshot/serializer-common.h

@@ -235,7 +235,6 @@ class SerializerDeserializer : public RootVisitor {
 
     // 0x60..0x7f
     kFixedRawData = 0x60,
-    kOnePointerRawData = kFixedRawData,
     kFixedRawDataStart = kFixedRawData - 1,
 
     //

src/snapshot/serializer.cc

@@ -102,15 +102,16 @@ bool Serializer::MustBeDeferred(HeapObject object) { return false; }
 void Serializer::VisitRootPointers(Root root, const char* description,
                                    FullObjectSlot start, FullObjectSlot end) {
   for (FullObjectSlot current = start; current < end; ++current) {
-    SerializeRootObject(*current);
+    SerializeRootObject(current);
   }
 }
 
-void Serializer::SerializeRootObject(Object object) {
-  if (object.IsSmi()) {
-    PutSmi(Smi::cast(object));
+void Serializer::SerializeRootObject(FullObjectSlot slot) {
+  Object o = *slot;
+  if (o.IsSmi()) {
+    PutSmiRoot(slot);
   } else {
-    SerializeObject(HeapObject::cast(object));
+    SerializeObject(HeapObject::cast(o));
   }
 }
 
@@ -209,12 +210,21 @@ void Serializer::PutRoot(RootIndex root, HeapObject object) {
   }
 }
 
-void Serializer::PutSmi(Smi smi) {
-  sink_.Put(kOnePointerRawData, "Smi");
-  Tagged_t raw_value = static_cast<Tagged_t>(smi.ptr());
-  byte bytes[kTaggedSize];
-  memcpy(bytes, &raw_value, kTaggedSize);
-  for (int i = 0; i < kTaggedSize; i++) sink_.Put(bytes[i], "Byte");
+void Serializer::PutSmiRoot(FullObjectSlot slot) {
+  // Serializing a smi root in compressed pointer builds will serialize the
+  // full object slot (of kSystemPointerSize) to avoid complications during
+  // deserialization (endianness or smi sequences).
+  STATIC_ASSERT(decltype(slot)::kSlotDataSize == sizeof(Address));
+  STATIC_ASSERT(decltype(slot)::kSlotDataSize == kSystemPointerSize);
+  static constexpr int bytes_to_output = decltype(slot)::kSlotDataSize;
+  static constexpr int size_in_tagged = bytes_to_output >> kTaggedSizeLog2;
+  sink_.PutSection(kFixedRawDataStart + size_in_tagged, "Smi");
+
+  Address raw_value = Smi::cast(*slot).ptr();
+  const byte* raw_value_as_bytes = reinterpret_cast<const byte*>(&raw_value);
+  for (size_t i = 0; i < bytes_to_output; i++) {
+    sink_.Put(raw_value_as_bytes[i], "Byte");
+  }
 }
 
 void Serializer::PutBackReference(HeapObject object,
@@ -834,8 +844,8 @@ void Serializer::ObjectSerializer::OutputRawData(Address up_to) {
     DCHECK(to_skip == bytes_to_output);
     if (IsAligned(bytes_to_output, kObjectAlignment) &&
         bytes_to_output <= kNumberOfFixedRawData * kTaggedSize) {
-      int size_in_words = bytes_to_output >> kTaggedSizeLog2;
-      sink_->PutSection(kFixedRawDataStart + size_in_words, "FixedRawData");
+      int size_in_tagged = bytes_to_output >> kTaggedSizeLog2;
+      sink_->PutSection(kFixedRawDataStart + size_in_tagged, "FixedRawData");
     } else {
       sink_->Put(kVariableRawData, "VariableRawData");
       sink_->PutInt(bytes_to_output, "length");

src/snapshot/serializer.h

@@ -198,10 +198,10 @@ class Serializer : public SerializerDeserializer {
 
   void VisitRootPointers(Root root, const char* description,
                          FullObjectSlot start, FullObjectSlot end) override;
-  void SerializeRootObject(Object object);
+  void SerializeRootObject(FullObjectSlot slot);
 
   void PutRoot(RootIndex root_index, HeapObject object);
-  void PutSmi(Smi smi);
+  void PutSmiRoot(FullObjectSlot slot);
   void PutBackReference(HeapObject object, SerializerReference reference);
   void PutAttachedReference(SerializerReference reference);
   // Emit alignment prefix if necessary, return required padding space in bytes.