[serializer] Clean up value encoding in bytecodes

Unify the encoding/decoding of values into a ranged bytecode with a
single templated class that takes the bytecode, minimum, and maximum,
and provides Encode and Decode methods.

This class also handles range checks on both the input and output,
which (along with a few other byte cases) allows us to get rid of the
PutSection method.

Change-Id: Icb2cd409607ce7b650226eb8dca80c0e363a8acc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2369172
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69642}

src/snapshot/context-deserializer.cc

@@ -90,11 +90,8 @@ void ContextDeserializer::DeserializeEmbedderFields(
   for (int code = source()->Get(); code != kSynchronize;
        code = source()->Get()) {
     HandleScope scope(isolate());
-    int space = code & kSpaceMask;
-    DCHECK_LE(space, kNumberOfSpaces);
-    DCHECK_EQ(code - space, kNewObject);
-    Handle<JSObject> obj(JSObject::cast(GetBackReferencedObject(
-                             static_cast<SnapshotSpace>(space))),
+    SnapshotSpace space = NewObject::Decode(code);
+    Handle<JSObject> obj(JSObject::cast(GetBackReferencedObject(space)),
                          isolate());
     int index = source()->GetInt();
     int size = source()->GetInt();

src/snapshot/deserializer.cc

@@ -126,10 +126,7 @@ void Deserializer::DeserializeDeferredObjects() {
         break;
       }
       default: {
-        const int space_number = code & kSpaceMask;
-        DCHECK_LE(space_number, kNumberOfSpaces);
-        DCHECK_EQ(code - space_number, kNewObject);
-        SnapshotSpace space = static_cast<SnapshotSpace>(space_number);
+        SnapshotSpace space = NewObject::Decode(code);
         HeapObject object = GetBackReferencedObject(space);
         int size = source_.GetInt() << kTaggedSizeLog2;
         Address obj_address = object.address();
@@ -597,13 +594,13 @@ bool Deserializer::ReadData(TSlot current, TSlot limit,
 // This generates a case and a body for the new space (which has to do extra
 // write barrier handling) and handles the other spaces with fall-through cases
 // and one body.
-#define ALL_SPACES(bytecode)                                      \
-  case BytecodeWithSpace<SnapshotSpace::kNew>(bytecode):          \
-  case BytecodeWithSpace<SnapshotSpace::kOld>(bytecode):          \
-  case BytecodeWithSpace<SnapshotSpace::kCode>(bytecode):         \
-  case BytecodeWithSpace<SnapshotSpace::kMap>(bytecode):          \
-  case BytecodeWithSpace<SnapshotSpace::kLargeObject>(bytecode):  \
-  case BytecodeWithSpace<SnapshotSpace::kReadOnlyHeap>(bytecode): \
+#define ALL_SPACES(bytecode)                                         \
+  case SpaceEncoder<bytecode>::Encode(SnapshotSpace::kNew):          \
+  case SpaceEncoder<bytecode>::Encode(SnapshotSpace::kOld):          \
+  case SpaceEncoder<bytecode>::Encode(SnapshotSpace::kCode):         \
+  case SpaceEncoder<bytecode>::Encode(SnapshotSpace::kMap):          \
+  case SpaceEncoder<bytecode>::Encode(SnapshotSpace::kLargeObject):  \
+  case SpaceEncoder<bytecode>::Encode(SnapshotSpace::kReadOnlyHeap): \
     READ_DATA_CASE_BODY(bytecode)
 
       // Deserialize a new object and write a pointer to it to the current
@@ -619,8 +616,8 @@ bool Deserializer::ReadData(TSlot current, TSlot limit,
       // current object.
       case kRootArray:
         READ_DATA_CASE_BODY(kRootArray)
-      // Find an object in the startup object cache and write a pointer to it
-      // to the current object.
+      // Find an object in the startup object cache and write a pointer to it to
+      // the current object.
       case kStartupObjectCache:
         READ_DATA_CASE_BODY(kStartupObjectCache)
       // Find an object in the read-only object cache and write a pointer to it
@@ -756,7 +753,7 @@ bool Deserializer::ReadData(TSlot current, TSlot limit,
       }
 
       case kVariableRepeat: {
-        int repeats = DecodeVariableRepeatCount(source_.GetInt());
+        int repeats = VariableRepeatCount::Decode(source_.GetInt());
         current = ReadRepeatedObject(current, repeats);
         break;
       }
@@ -818,8 +815,7 @@ bool Deserializer::ReadData(TSlot current, TSlot limit,
         STATIC_ASSERT(kRootArrayConstantsCount <=
                       static_cast<int>(RootIndex::kLastImmortalImmovableRoot));
 
-        int id = data & kRootArrayConstantsMask;
-        RootIndex root_index = static_cast<RootIndex>(id);
+        RootIndex root_index = RootArrayConstant::Decode(data);
         MaybeObject object =
             MaybeObject(ReadOnlyRoots(isolate()).at(root_index));
         DCHECK(!Heap::InYoungGeneration(object));
@@ -828,7 +824,7 @@ bool Deserializer::ReadData(TSlot current, TSlot limit,
       }
 
       case CASE_RANGE(kHotObject, 8): {
-        int index = data & kHotObjectMask;
+        int index = HotObject::Decode(data);
         Object hot_object = hot_objects_.Get(index);
         MaybeObject hot_maybe_object = MaybeObject::FromObject(hot_object);
         if (allocator()->GetAndClearNextReferenceIsWeak()) {
@@ -850,7 +846,7 @@ bool Deserializer::ReadData(TSlot current, TSlot limit,
 
       case CASE_RANGE(kFixedRawData, 32): {
         // Deserialize raw data of fixed length from 1 to 32 times kTaggedSize.
-        int size_in_tagged = DecodeFixedRawDataSize(data);
+        int size_in_tagged = FixedRawDataWithSize::Decode(data);
         source_.CopyRaw(current.ToVoidPtr(), size_in_tagged * kTaggedSize);
 
         int size_in_bytes = size_in_tagged * kTaggedSize;
@@ -861,7 +857,7 @@ bool Deserializer::ReadData(TSlot current, TSlot limit,
       }
 
       case CASE_RANGE(kFixedRepeat, 16): {
-        int repeats = DecodeFixedRepeatCount(data);
+        int repeats = FixedRepeatWithCount::Decode(data);
         current = ReadRepeatedObject(current, repeats);
         break;
       }
@@ -904,11 +900,11 @@ TSlot Deserializer::ReadDataCase(TSlot current, Address current_object_address,
           : HeapObjectReferenceType::STRONG;
 
   if (bytecode == kNewObject) {
-    SnapshotSpace space = static_cast<SnapshotSpace>(data & kSpaceMask);
+    SnapshotSpace space = SpaceEncoder<bytecode>::Decode(data);
     heap_object = ReadObject(space);
     emit_write_barrier = (space == SnapshotSpace::kNew);
   } else if (bytecode == kBackref) {
-    SnapshotSpace space = static_cast<SnapshotSpace>(data & kSpaceMask);
+    SnapshotSpace space = SpaceEncoder<bytecode>::Decode(data);
     heap_object = GetBackReferencedObject(space);
     emit_write_barrier = (space == SnapshotSpace::kNew);
   } else if (bytecode == kNewMetaMap) {

src/snapshot/references.h

@@ -15,7 +15,7 @@ namespace internal {
 // TODO(goszczycki): Move this somewhere every file in src/snapshot can use it.
 // The spaces suported by the serializer. Spaces after LO_SPACE (NEW_LO_SPACE
 // and CODE_LO_SPACE) are not supported.
-enum class SnapshotSpace {
+enum class SnapshotSpace : byte {
   kReadOnlyHeap = RO_SPACE,
   kNew = NEW_SPACE,
   kOld = OLD_SPACE,

src/snapshot/serializer-deserializer.h

@@ -5,7 +5,6 @@
 #ifndef V8_SNAPSHOT_SERIALIZER_DESERIALIZER_H_
 #define V8_SNAPSHOT_SERIALIZER_DESERIALIZER_H_
 
-#include "src/base/logging.h"
 #include "src/objects/visitors.h"
 #include "src/snapshot/references.h"
 
@@ -67,9 +66,6 @@ class SerializerDeserializer : public RootVisitor {
   void RestoreExternalReferenceRedirectors(
       Isolate* isolate, const std::vector<CallHandlerInfo>& call_handler_infos);
 
-  static const int kNumberOfPreallocatedSpaces =
-      static_cast<int>(SnapshotSpace::kNumberOfPreallocatedSpaces);
-
   static const int kNumberOfSpaces =
       static_cast<int>(SnapshotSpace::kNumberOfSpaces);
 
@@ -107,12 +103,9 @@ class SerializerDeserializer : public RootVisitor {
   // changed. If that happens, update the kNewObject and kBackref bytecode
   // ranges in the comments below.
   STATIC_ASSERT(6 == kNumberOfSpaces);
-  static const int kSpaceMask = 7;
-  STATIC_ASSERT(kNumberOfSpaces <= kSpaceMask + 1);
 
   // First 32 root array items.
   static const int kRootArrayConstantsCount = 0x20;
-  static const int kRootArrayConstantsMask = 0x1f;
 
   // 32 common raw data lengths.
   static const int kFixedRawDataCount = 0x20;
@@ -122,7 +115,6 @@ class SerializerDeserializer : public RootVisitor {
   // 8 hot (recently seen or back-referenced) objects with optional skip.
   static const int kHotObjectCount = 8;
   STATIC_ASSERT(kHotObjectCount == HotObjectsList::kSize);
-  static const int kHotObjectMask = 0x07;
 
   // 3 alignment prefixes
   static const int kAlignmentPrefixCount = 3;
@@ -224,13 +216,35 @@ class SerializerDeserializer : public RootVisitor {
     kHotObject = 0x90,
   };
 
-  template <SnapshotSpace space>
-  static constexpr byte BytecodeWithSpace(Bytecode bytecode) {
-    STATIC_ASSERT(
-        (static_cast<int>(space) & ~SerializerDeserializer::kSpaceMask) == 0);
-    CONSTEXPR_DCHECK((bytecode & kSpaceMask) == 0);
-    return bytecode + static_cast<int>(space);
-  }
+  // Helper class for encoding and decoding a value into and from a bytecode.
+  template <Bytecode kBytecode, int kMinValue, int kMaxValue,
+            typename TValue = int>
+  struct BytecodeValueEncoder {
+    STATIC_ASSERT((kBytecode + kMaxValue - kMinValue) <= kMaxUInt8);
+
+    static constexpr bool IsEncodable(TValue value) {
+      return base::IsInRange(static_cast<int>(value), kMinValue, kMaxValue);
+    }
+
+    static constexpr byte Encode(TValue value) {
+      CONSTEXPR_DCHECK(IsEncodable(value));
+      return static_cast<byte>(kBytecode + static_cast<int>(value) - kMinValue);
+    }
+
+    static constexpr TValue Decode(byte bytecode) {
+      CONSTEXPR_DCHECK(base::IsInRange(bytecode,
+                                       Encode(static_cast<TValue>(kMinValue)),
+                                       Encode(static_cast<TValue>(kMaxValue))));
+      return static_cast<TValue>(bytecode - kBytecode + kMinValue);
+    }
+  };
+
+  template <Bytecode bytecode>
+  using SpaceEncoder =
+      BytecodeValueEncoder<bytecode, 0, kNumberOfSpaces, SnapshotSpace>;
+
+  using NewObject = SpaceEncoder<kNewObject>;
+  using BackRef = SpaceEncoder<kBackref>;
 
   //
   // Some other constants.
@@ -245,21 +259,9 @@ class SerializerDeserializer : public RootVisitor {
   static const int kLastEncodableFixedRawDataSize =
       kFirstEncodableFixedRawDataSize + kFixedRawDataCount - 1;
 
-  // Encodes raw data size into a fixed raw data bytecode.
-  static constexpr byte EncodeFixedRawDataSize(int size_in_tagged) {
-    CONSTEXPR_DCHECK(base::IsInRange(size_in_tagged,
-                                     kFirstEncodableFixedRawDataSize,
-                                     kLastEncodableFixedRawDataSize));
-    return kFixedRawData + size_in_tagged - kFirstEncodableFixedRawDataSize;
-  }
-
-  // Decodes raw data size from a fixed raw data bytecode.
-  static constexpr int DecodeFixedRawDataSize(byte bytecode) {
-    CONSTEXPR_DCHECK(base::IsInRange(static_cast<int>(bytecode),
-                                     kFixedRawData + 0,
-                                     kFixedRawData + kFixedRawDataCount));
-    return bytecode - kFixedRawData + kFirstEncodableFixedRawDataSize;
-  }
+  using FixedRawDataWithSize =
+      BytecodeValueEncoder<kFixedRawData, kFirstEncodableFixedRawDataSize,
+                           kLastEncodableFixedRawDataSize>;
 
   // Repeat count encoding helpers.
   static const int kFirstEncodableRepeatCount = 2;
@@ -268,31 +270,31 @@ class SerializerDeserializer : public RootVisitor {
   static const int kFirstEncodableVariableRepeatCount =
       kLastEncodableFixedRepeatCount + 1;
 
-  // Encodes repeat count into a fixed repeat bytecode.
-  static constexpr byte EncodeFixedRepeat(int repeat_count) {
-    CONSTEXPR_DCHECK(base::IsInRange(repeat_count, kFirstEncodableRepeatCount,
-                                     kLastEncodableFixedRepeatCount));
-    return kFixedRepeat + repeat_count - kFirstEncodableRepeatCount;
-  }
-
-  // Decodes repeat count from a fixed repeat bytecode.
-  static constexpr int DecodeFixedRepeatCount(int bytecode) {
-    CONSTEXPR_DCHECK(base::IsInRange(bytecode, kFixedRepeat + 0,
-                                     kFixedRepeat + kFixedRepeatCount));
-    return bytecode - kFixedRepeat + kFirstEncodableRepeatCount;
-  }
-
-  // Encodes repeat count into a serialized variable repeat count value.
-  static constexpr int EncodeVariableRepeatCount(int repeat_count) {
-    CONSTEXPR_DCHECK(kFirstEncodableVariableRepeatCount <= repeat_count);
-    return repeat_count - kFirstEncodableVariableRepeatCount;
-  }
-
-  // Decodes repeat count from a serialized variable repeat count value.
-  static constexpr int DecodeVariableRepeatCount(int value) {
-    CONSTEXPR_DCHECK(0 <= value);
-    return value + kFirstEncodableVariableRepeatCount;
-  }
+  using FixedRepeatWithCount =
+      BytecodeValueEncoder<kFixedRepeat, kFirstEncodableRepeatCount,
+                           kLastEncodableFixedRepeatCount>;
+
+  // Encodes/decodes repeat count into a serialized variable repeat count
+  // value.
+  struct VariableRepeatCount {
+    static constexpr bool IsEncodable(int repeat_count) {
+      return repeat_count >= kFirstEncodableVariableRepeatCount;
+    }
+
+    static constexpr int Encode(int repeat_count) {
+      CONSTEXPR_DCHECK(IsEncodable(repeat_count));
+      return repeat_count - kFirstEncodableVariableRepeatCount;
+    }
+
+    static constexpr int Decode(int value) {
+      return value + kFirstEncodableVariableRepeatCount;
+    }
+  };
+
+  using RootArrayConstant =
+      BytecodeValueEncoder<kRootArrayConstants, 0, kRootArrayConstantsCount - 1,
+                           RootIndex>;
+  using HotObject = BytecodeValueEncoder<kHotObject, 0, kHotObjectCount - 1>;
 
   // ---------- member variable ----------
   HotObjectsList hot_objects_;

src/snapshot/serializer.cc

@@ -130,7 +130,7 @@ bool Serializer::SerializeHotObject(HeapObject obj) {
     obj.ShortPrint();
     PrintF("\n");
   }
-  sink_.Put(kHotObject + index, "HotObject");
+  sink_.Put(HotObject::Encode(index), "HotObject");
   return true;
 }
 
@@ -158,7 +158,7 @@ bool Serializer::SerializeBackReference(HeapObject obj) {
 
     PutAlignmentPrefix(obj);
     SnapshotSpace space = reference.space();
-    sink_.Put(kBackref + static_cast<int>(space), "BackRef");
+    sink_.Put(BackRef::Encode(space), "BackRef");
     PutBackReference(obj, reference);
   }
   return true;
@@ -196,7 +196,7 @@ void Serializer::PutRoot(RootIndex root, HeapObject object) {
   // TODO(ulan): Check that it works with young large objects.
   if (root_index < kRootArrayConstantsCount &&
       !Heap::InYoungGeneration(object)) {
-    sink_.Put(kRootArrayConstants + root_index, "RootConstant");
+    sink_.Put(RootArrayConstant::Encode(root), "RootConstant");
   } else {
     sink_.Put(kRootArray, "RootSerialization");
     sink_.PutInt(root_index, "root_index");
@@ -212,7 +212,7 @@ void Serializer::PutSmiRoot(FullObjectSlot slot) {
   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(EncodeFixedRawDataSize(size_in_tagged), "Smi");
+  sink_.Put(FixedRawDataWithSize::Encode(size_in_tagged), "Smi");
 
   Address raw_value = Smi::cast(*slot).ptr();
   const byte* raw_value_as_bytes = reinterpret_cast<const byte*>(&raw_value);
@@ -259,15 +259,15 @@ int Serializer::PutAlignmentPrefix(HeapObject object) {
 
 void Serializer::PutNextChunk(SnapshotSpace space) {
   sink_.Put(kNextChunk, "NextChunk");
-  sink_.Put(static_cast<int>(space), "NextChunkSpace");
+  sink_.Put(static_cast<byte>(space), "NextChunkSpace");
 }
 
 void Serializer::PutRepeat(int repeat_count) {
   if (repeat_count <= kLastEncodableFixedRepeatCount) {
-    sink_.Put(EncodeFixedRepeat(repeat_count), "FixedRepeat");
+    sink_.Put(FixedRepeatWithCount::Encode(repeat_count), "FixedRepeat");
   } else {
     sink_.Put(kVariableRepeat, "VariableRepeat");
-    sink_.PutInt(EncodeVariableRepeatCount(repeat_count), "repeat count");
+    sink_.PutInt(VariableRepeatCount::Encode(repeat_count), "repeat count");
   }
 }
 
@@ -348,8 +348,6 @@ void Serializer::ObjectSerializer::SerializePrologue(SnapshotSpace space,
         CodeNameEvent(object_.address(), sink_->Position(), code_name));
   }
 
-  const int space_number = static_cast<int>(space);
-
   SerializerReference back_reference;
   if (map == object_) {
     DCHECK_EQ(object_, ReadOnlyRoots(serializer_->isolate()).meta_map());
@@ -359,7 +357,7 @@ void Serializer::ObjectSerializer::SerializePrologue(SnapshotSpace space,
     DCHECK_EQ(size, Map::kSize);
     back_reference = serializer_->allocator()->Allocate(space, size);
   } else {
-    sink_->Put(kNewObject + space_number, "NewObject");
+    sink_->Put(NewObject::Encode(space), "NewObject");
 
     // TODO(leszeks): Skip this when the map has a fixed size.
     sink_->PutInt(size >> kObjectAlignmentBits, "ObjectSizeInWords");
@@ -551,7 +549,8 @@ void Serializer::ObjectSerializer::SerializeExternalStringAsSequentialString() {
   // maybe left-over bytes that need to be padded.
   int padding_size = allocation_size - SeqString::kHeaderSize - content_size;
   DCHECK(0 <= padding_size && padding_size < kObjectAlignment);
-  for (int i = 0; i < padding_size; i++) sink_->PutSection(0, "StringPadding");
+  for (int i = 0; i < padding_size; i++)
+    sink_->Put(static_cast<byte>(0), "StringPadding");
 }
 
 // Clear and later restore the next link in the weak cell or allocation site.
@@ -702,8 +701,7 @@ void Serializer::ObjectSerializer::SerializeDeferred() {
   bytes_processed_so_far_ = kTaggedSize;
 
   serializer_->PutAlignmentPrefix(object_);
-  sink_->Put(kNewObject + static_cast<int>(back_reference.space()),
-             "deferred object");
+  sink_->Put(NewObject::Encode(back_reference.space()), "deferred object");
   serializer_->PutBackReference(object_, back_reference);
   sink_->PutInt(size >> kTaggedSizeLog2, "deferred object size");
 
@@ -824,7 +822,7 @@ void Serializer::ObjectSerializer::OutputExternalReference(Address target,
     CHECK(IsAligned(target_size, kObjectAlignment));
     CHECK_LE(target_size, kFixedRawDataCount * kTaggedSize);
     int size_in_tagged = target_size >> kTaggedSizeLog2;
-    sink_->PutSection(EncodeFixedRawDataSize(size_in_tagged), "FixedRawData");
+    sink_->Put(FixedRawDataWithSize::Encode(size_in_tagged), "FixedRawData");
     sink_->PutRaw(reinterpret_cast<byte*>(&target), target_size, "Bytes");
   } else if (encoded_reference.is_from_api()) {
     if (V8_HEAP_SANDBOX_BOOL && sandboxify) {
@@ -939,7 +937,7 @@ void Serializer::ObjectSerializer::OutputRawData(Address up_to) {
     if (IsAligned(bytes_to_output, kObjectAlignment) &&
         bytes_to_output <= kFixedRawDataCount * kTaggedSize) {
       int size_in_tagged = bytes_to_output >> kTaggedSizeLog2;
-      sink_->PutSection(EncodeFixedRawDataSize(size_in_tagged), "FixedRawData");
+      sink_->Put(FixedRawDataWithSize::Encode(size_in_tagged), "FixedRawData");
     } else {
       sink_->Put(kVariableRawData, "VariableRawData");
       sink_->PutInt(bytes_to_output, "length");

src/snapshot/snapshot-source-sink.cc

@@ -20,13 +20,12 @@ void SnapshotByteSink::PutInt(uintptr_t integer, const char* description) {
   if (integer > 0xFFFF) bytes = 3;
   if (integer > 0xFFFFFF) bytes = 4;
   integer |= (bytes - 1);
-  Put(static_cast<int>(integer & 0xFF), "IntPart1");
-  if (bytes > 1) Put(static_cast<int>((integer >> 8) & 0xFF), "IntPart2");
-  if (bytes > 2) Put(static_cast<int>((integer >> 16) & 0xFF), "IntPart3");
-  if (bytes > 3) Put(static_cast<int>((integer >> 24) & 0xFF), "IntPart4");
+  Put(static_cast<byte>(integer & 0xFF), "IntPart1");
+  if (bytes > 1) Put(static_cast<byte>((integer >> 8) & 0xFF), "IntPart2");
+  if (bytes > 2) Put(static_cast<byte>((integer >> 16) & 0xFF), "IntPart3");
+  if (bytes > 3) Put(static_cast<byte>((integer >> 24) & 0xFF), "IntPart4");
 }
 
-
 void SnapshotByteSink::PutRaw(const byte* data, int number_of_bytes,
                               const char* description) {
   data_.insert(data_.end(), data, data + number_of_bytes);

src/snapshot/snapshot-source-sink.h

@@ -86,11 +86,10 @@ class SnapshotByteSource final {
   DISALLOW_COPY_AND_ASSIGN(SnapshotByteSource);
 };
 
-
 /**
  * Sink to write snapshot files to.
  *
- * Subclasses must implement actual storage or i/o.
+ * Users must implement actual storage or i/o.
  */
 class SnapshotByteSink {
  public:
@@ -101,11 +100,6 @@ class SnapshotByteSink {
 
   void Put(byte b, const char* description) { data_.push_back(b); }
 
-  void PutSection(int b, const char* description) {
-    DCHECK_LE(b, kMaxUInt8);
-    Put(static_cast<byte>(b), description);
-  }
-
   void PutInt(uintptr_t integer, const char* description);
   void PutRaw(const byte* data, int number_of_bytes, const char* description);