[objects] Store Name hash fields in a BitFieldGroup

Group Name's hash, array-index, and forwarding-index layouts and
constants in Name::Hash. Use std::atomic<Hash> for raw_hash_field_ so
metagen can extract its layout without V8_TQ_TYPE. Keep aliases on Name
for existing callers.

Support overlapping fields in metagen and Torque. Preserve C++ bit
offsets, check each field against storage bounds, and allow explicit
`at` offsets in authored Torque declarations. Mirror all Name hash
fields in Torque and verify them against C++.

Allow partial bitfield initializers, leaving unwritten bits zero. Reject
overlapping supplied fields, including duplicate names.

Bug: 531344950
Change-Id: Id336d49023f09529bf4225359aa97202f7e60e61
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8559139
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Auto-Submit: Jakob Linke <jgruber@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#110869}

src/objects/bit-field-group.h

@@ -30,7 +30,7 @@ namespace v8::internal {
 // layout JSON, from which Torque derives (or verifies) the corresponding
 // bitfield struct. Naming and layout requirements:
 //  - Every type alias in the struct must be a base::BitField; they are read in
-//    declaration order, which must be bit order without gaps.
+//    declaration order. Fields may overlap, but must fit the storage.
 //  - The Torque field name is the alias name without its Bit/Bits suffix, in
 //    snake_case (IsCallableBit -> is_callable). The getter names follow the
 //    same convention.

src/objects/name.h

@@ -171,28 +171,6 @@ V8_OBJECT class Name : public PrimitiveHeapObject {
     kEmpty = 0b11
   };
 
-  using HashFieldTypeBits = base::BitField<HashFieldType, 0, 2>;
-  using HashBits =
-      HashFieldTypeBits::Next<uint32_t, kBitsPerInt - HashFieldTypeBits::kSize>;
-
-  static constexpr int kHashNotComputedMask = 1;
-  // Value of empty hash field indicating that the hash is not computed.
-  static constexpr int kEmptyHashField =
-      HashFieldTypeBits::encode(HashFieldType::kEmpty);
-
-  // Empty hash and forwarding indices can not be used as hash.
-  static_assert((kEmptyHashField & kHashNotComputedMask) != 0);
-  static_assert((HashFieldTypeBits::encode(HashFieldType::kForwardingIndex) &
-                 kHashNotComputedMask) != 0);
-
-  using IsInternalizedForwardingIndexBit = HashFieldTypeBits::Next<bool, 1>;
-  using IsExternalForwardingIndexBit =
-      IsInternalizedForwardingIndexBit::Next<bool, 1>;
-  using ForwardingIndexValueBits = IsExternalForwardingIndexBit::Next<
-      unsigned int, kBitsPerInt - HashFieldTypeBits::kSize -
-                        IsInternalizedForwardingIndexBit::kSize -
-                        IsExternalForwardingIndexBit::kSize>;
-
   // Array index strings this short can keep their index in the hash field.
   static const int kMaxCachedArrayIndexLength = 7;
 
@@ -226,17 +204,64 @@ V8_OBJECT class Name : public PrimitiveHeapObject {
                 "kArrayIndexHashShift must be at least half of "
                 "kArrayIndexValueBits");
 #endif  // V8_ENABLE_SEEDED_ARRAY_INDEX_HASH
+
+  struct Hash : BitFieldGroup<Hash, uint32_t> {
+    using BitFieldGroup::BitFieldGroup;
+
+    using HashFieldTypeBits = First<HashFieldType, 2>;
+
+    // Hash code view (kHash).
+    using HashBits =
+        HashFieldTypeBits::Next<uint32_t,
+                                kBitsPerInt - HashFieldTypeBits::kSize>;
+
+    // Array index view (kIntegerIndex).
+    using ArrayIndexValueBits =
+        HashFieldTypeBits::Next<unsigned int, kArrayIndexValueBits>;
+    using ArrayIndexLengthBits =
+        ArrayIndexValueBits::Next<unsigned int, kBitsPerInt -
+                                                    kArrayIndexValueBits -
+                                                    HashFieldTypeBits::kSize>;
+
+    // String forwarding table view (kForwardingIndex).
+    using IsInternalizedForwardingIndexBit = HashFieldTypeBits::Next<bool, 1>;
+    using IsExternalForwardingIndexBit =
+        IsInternalizedForwardingIndexBit::Next<bool, 1>;
+    using ForwardingIndexValueBits = IsExternalForwardingIndexBit::Next<
+        unsigned int, kBitsPerInt - HashFieldTypeBits::kSize -
+                          IsInternalizedForwardingIndexBit::kSize -
+                          IsExternalForwardingIndexBit::kSize>;
+
+    BIT_FIELD_GROUP_ACCESSORS(hash_field_type, HashFieldTypeBits)
+    BIT_FIELD_GROUP_ACCESSORS(array_index_value, ArrayIndexValueBits)
+    BIT_FIELD_GROUP_ACCESSORS(array_index_length, ArrayIndexLengthBits)
+  };
+  // TODO(jgruber): Remove these aliases after migrating callers to Hash.
+  using HashFieldTypeBits = Hash::HashFieldTypeBits;
+  using HashBits = Hash::HashBits;
+  using IsInternalizedForwardingIndexBit =
+      Hash::IsInternalizedForwardingIndexBit;
+  using IsExternalForwardingIndexBit = Hash::IsExternalForwardingIndexBit;
+  using ForwardingIndexValueBits = Hash::ForwardingIndexValueBits;
+  using ArrayIndexValueBits = Hash::ArrayIndexValueBits;
+  using ArrayIndexLengthBits = Hash::ArrayIndexLengthBits;
+
+  static constexpr int kHashNotComputedMask = 1;
+  // Value of empty hash field indicating that the hash is not computed.
+  static constexpr int kEmptyHashField =
+      HashFieldTypeBits::encode(HashFieldType::kEmpty);
+
+  // Empty hash and forwarding indices can not be used as hash.
+  static_assert((kEmptyHashField & kHashNotComputedMask) != 0);
+  static_assert((HashFieldTypeBits::encode(HashFieldType::kForwardingIndex) &
+                 kHashNotComputedMask) != 0);
+
   static const int kArrayIndexLengthBits =
       kBitsPerInt - kArrayIndexValueBits - HashFieldTypeBits::kSize;
 
   static_assert(kArrayIndexLengthBits > 0);
   static_assert(kMaxArrayIndexSize < (1 << kArrayIndexLengthBits));
 
-  using ArrayIndexValueBits =
-      HashFieldTypeBits::Next<unsigned int, kArrayIndexValueBits>;
-  using ArrayIndexLengthBits =
-      ArrayIndexValueBits::Next<unsigned int, kArrayIndexLengthBits>;
-
   // Check that kMaxCachedArrayIndexLength + 1 is a power of two so we
   // could use a mask to test if the length of string is less than or equal to
   // kMaxCachedArrayIndexLength.
@@ -305,7 +330,7 @@ V8_OBJECT class Name : public PrimitiveHeapObject {
 
   inline uint32_t GetRawHashFromForwardingTable(uint32_t raw_hash) const;
 
-  std::atomic_uint32_t raw_hash_field_ V8_TQ_TYPE(NameHash);
+  std::atomic<Hash> raw_hash_field_;
 } V8_OBJECT_END;
 
 inline bool IsUniqueName(Tagged<Name> obj);

src/objects/name.tq

@@ -8,10 +8,21 @@ extern class Name extends PrimitiveHeapObject {
   raw_hash_field: NameHash;
 }
 
+@cppScope('Name::Hash')
 bitfield struct NameHash extends uint32 {
   hash_field_type: HashFieldType: 2 bit;
-  array_index_value: uint32: 24 bit;
+
+  // hash_field_type: kHash
+  hash: uint32: 30 bit at 2;
+
+  // hash_field_type: kIntegerIndex
+  array_index_value: uint32: 24 bit at 2;
   array_index_length: uint32: 6 bit;
+
+  // hash_field_type: kForwardingIndex
+  is_internalized_forwarding_index: bool: 1 bit at 2;
+  is_external_forwarding_index: bool: 1 bit;
+  forwarding_index_value: uint32: 28 bit;
 }
 
 // This is the same as Name, but with the information that there are no other
@@ -52,11 +63,8 @@ const kArrayIndexValueBits: constexpr int31
     generates 'Name::kArrayIndexValueBits';
 const kDoesNotContainCachedArrayIndexMask: constexpr uint32
     generates 'Name::kDoesNotContainCachedArrayIndexMask';
-const kNameEmptyHashField: NameHash = NameHash{
-  hash_field_type: HashFieldType::kEmpty,
-  array_index_value: 0,
-  array_index_length: 0
-};
+const kNameEmptyHashField: NameHash =
+NameHash{hash_field_type: HashFieldType::kEmpty};
 
 macro ContainsCachedArrayIndex(hash: uint32): bool {
   return (hash & kDoesNotContainCachedArrayIndexMask) == 0;

src/objects/string.cc

@@ -1938,7 +1938,7 @@ uint32_t String::ComputeAndSetRawHash(bool* out_one_byte_content) {
 // only used right below, so putting it here keeps headers leaner and still
 // allows the compiler to inline it.
 void Name::set_raw_hash_field_if_empty(uint32_t hash) {
-  uint32_t field_value = kEmptyHashField;
+  Hash field_value = kEmptyHashField;
   bool result = raw_hash_field_.compare_exchange_strong(field_value, hash);
   USE(result);
   // CAS can only fail if the string is shared or we use the forwarding table

src/torque/ast.h

@@ -936,6 +936,7 @@ struct StructFieldExpression {
 struct BitFieldDeclaration {
   NameAndTypeExpression name_and_type;
   int num_bits;
+  std::optional<int> offset = std::nullopt;
 };
 
 enum class ConditionalAnnotationType {

src/torque/earley-parser.h

@@ -43,6 +43,7 @@ enum class ParseResultHolderBase::TypeId {
   kStdString,
   kBool,
   kInt32,
+  kOptionalInt32,
   kDouble,
   kIntegerLiteral,
   kStdVectorOfString,

src/torque/implementation-visitor.cc

@@ -2370,8 +2370,6 @@ VisitResult ImplementationVisitor::Visit(StructExpression* expr) {
     return stack_scope.Yield(VisitResult(struct_type, struct_range));
   } else {
     const auto* bitfield_struct_type = BitFieldStructType::cast(type);
-    CheckInitializersWellformed(bitfield_struct_type->name(),
-                                bitfield_struct_type->fields(), initializers);
 
     // Create a zero and cast it to the desired bitfield struct type.
     VisitResult result{TypeOracle::GetConstInt32Type(), "0"};
@@ -2383,11 +2381,19 @@ VisitResult ImplementationVisitor::Visit(StructExpression* expr) {
     // Set each field in the result. If these fields are constexpr, then all of
     // this initialization will end up reduced to a single value during TurboFan
     // optimization.
-    auto& fields = bitfield_struct_type->fields();
+    uint64_t initialized_bits = 0;
     for (size_t i = 0; i < values.size(); i++) {
-      values[i] =
-          GenerateImplicitConvert(fields[i].name_and_type.type, values[i]);
-      result = GenerateSetBitField(bitfield_struct_type, fields[i], result,
+      CurrentSourcePosition::Scope position_scope(initializers[i].name->pos);
+      const BitField& field =
+          bitfield_struct_type->LookupField(initializers[i].name->value);
+      uint64_t mask = (~uint64_t{0} >> (64 - field.num_bits)) << field.offset;
+      if (initialized_bits & mask) {
+        ReportError("Overlapping bitfield initializer: ",
+                    initializers[i].name->value);
+      }
+      initialized_bits |= mask;
+      values[i] = GenerateImplicitConvert(field.name_and_type.type, values[i]);
+      result = GenerateSetBitField(bitfield_struct_type, field, result,
                                    values[i], /*starts_as_zero=*/true);
     }
 

src/torque/layout-loader.cc

@@ -1283,9 +1283,12 @@ class LayoutImporter {
       return false;
     }
     bool equivalent = true;
+    int64_t offset = 0;
     for (size_t i = 0; i < built.size(); ++i) {
-      std::string a = BitFieldKey(decl.fields[i]);
-      std::string b = BitFieldKey(built[i]);
+      offset = decl.fields[i].offset ? *decl.fields[i].offset : offset;
+      std::string a = BitFieldKey(decl.fields[i], offset);
+      std::string b = BitFieldKey(built[i], *built[i].offset);
+      offset += decl.fields[i].num_bits;
       if (a != b) {
         Error("bitfield struct ", name, ": .tq field \"", a,
               "\" differs from C++ field \"", b, "\"")
@@ -1296,10 +1299,11 @@ class LayoutImporter {
     return equivalent;
   }
 
-  static std::string BitFieldKey(const BitFieldDeclaration& field) {
+  static std::string BitFieldKey(const BitFieldDeclaration& field,
+                                 int64_t offset) {
     return field.name_and_type.name->value + ": " +
            TypeExpressionKey(field.name_and_type.type) + ": " +
-           std::to_string(field.num_bits) + " bit";
+           std::to_string(field.num_bits) + " bit at " + std::to_string(offset);
   }
 
   // Groups stored as Smis use uint31; other groups use their storage width.
@@ -1322,7 +1326,6 @@ class LayoutImporter {
       ReportError("layout JSON: ", context, ": bitfield without fields");
     }
     std::vector<BitFieldDeclaration> fields;
-    size_t next_bit = 0;
     for (const JsonValue& bit : bits.ToArray()) {
       if (!bit.IsObject()) {
         ReportError("layout JSON: ", context, ": bit field is not an object");
@@ -1330,14 +1333,10 @@ class LayoutImporter {
       const JsonObject& b = bit.ToObject();
       const std::string& cpp_name = RequireString(b, "cpp_name", context);
       size_t num_bits = RequireSize(b, "num_bits", context);
-      // Torque derives each offset from declaration order; a gap there would
-      // silently shift every field after it.
       size_t offset = RequireSize(b, "offset", context);
-      if (offset != next_bit) {
-        ReportError("layout JSON: ", context, ": bit field \"", cpp_name,
-                    "\" starts at bit ", offset, ", expected ", next_bit);
+      if (offset > 64 || num_bits > 64) {
+        ReportError("layout JSON: ", context, ": bit field exceeds 64 bits");
       }
-      next_bit = offset + num_bits;
       CurrentSourcePosition::Scope position_scope(
           positions != nullptr ? positions->at(fields.size())
                                : CurrentSourcePosition::Get());
@@ -1346,7 +1345,7 @@ class LayoutImporter {
               MakeNode<Identifier>(
                   TorqueBitFieldName(cpp_name, num_bits, context)),
               NamedType(TorqueTypeName(RequireString(b, "type", context)))},
-          static_cast<int>(num_bits)});
+          static_cast<int>(num_bits), static_cast<int>(offset)});
     }
     return fields;
   }

src/torque/torque-parser.cc

@@ -117,6 +117,12 @@ V8_EXPORT_PRIVATE const ParseResultTypeId ParseResultHolder<bool>::id =
 template <>
 V8_EXPORT_PRIVATE const ParseResultTypeId ParseResultHolder<int32_t>::id =
     ParseResultTypeId::kInt32;
+
+template <>
+V8_EXPORT_PRIVATE const ParseResultTypeId
+    ParseResultHolder<std::optional<int32_t>>::id =
+        ParseResultTypeId::kOptionalInt32;
+
 template <>
 V8_EXPORT_PRIVATE const ParseResultTypeId ParseResultHolder<double>::id =
     ParseResultTypeId::kDouble;
@@ -2269,7 +2275,8 @@ std::optional<ParseResult> MakeBitFieldDeclaration(
   auto name = child_results->NextAs<Identifier*>();
   auto type = child_results->NextAs<TypeExpression*>();
   auto num_bits = child_results->NextAs<int32_t>();
-  return ParseResult{BitFieldDeclaration{{name, type}, num_bits}};
+  auto offset = child_results->NextAs<std::optional<int32_t>>();
+  return ParseResult{BitFieldDeclaration{{name, type}, num_bits, offset}};
 }
 
 std::optional<ParseResult> ExtractAssignmentOperator(
@@ -2584,9 +2591,10 @@ struct TorqueGrammar : Grammar {
            MakeStructField)};
 
   // Result: BitFieldDeclaration
-  Symbol bitFieldDeclaration = {Rule({&name, Token(":"), &type, Token(":"),
-                                      &int32Literal, Token("bit"), Token(";")},
-                                     MakeBitFieldDeclaration)};
+  Symbol bitFieldDeclaration = {Rule(
+      {&name, Token(":"), &type, Token(":"), &int32Literal, Token("bit"),
+       Optional<int32_t>(Sequence({Token("at"), &int32Literal})), Token(";")},
+      MakeBitFieldDeclaration)};
 
   // Result: ParameterList
   Symbol parameterListNoVararg = {

src/torque/type-visitor.cc

@@ -173,14 +173,15 @@ const BitFieldStructType* TypeVisitor::ComputeType(
       ReportError("Invalid number of bits for ",
                   field.name_and_type.name->value);
     }
+    offset = field.offset.value_or(offset);
+    if (offset < 0 || static_cast<size_t>(offset) + field.num_bits > size) {
+      ReportError("Bitfield outside storage in ", decl->name->value);
+    }
     type->RegisterField({field.name_and_type.name->pos,
                          {field.name_and_type.name->value, field_type},
                          offset,
                          field.num_bits});
     offset += field.num_bits;
-    if (static_cast<size_t>(offset) > size) {
-      ReportError("Too many total bits in ", decl->name->value);
-    }
   }
 
   return type;

test/torque/test-torque.tq

@@ -1015,10 +1015,34 @@ macro TestBitFieldStore(val: TestBitFieldStruct): void {
   TestBitFieldLoad(val, !a, b, c, b == c);
 }
 
+bitfield struct TestOverlappingBitFieldStruct extends uint32 {
+  low: uint32: 4 bit;
+  high: uint32: 4 bit;
+  all: uint32: 8 bit at 0;
+}
+
 @export
 macro TestBitFieldInit(a: bool, b: uint16, c: uint32, d: bool): void {
   const val: TestBitFieldStruct = TestBitFieldStruct{a: a, b: b, c: c, d: d};
   TestBitFieldLoad(val, a, b, c, d);
+  TestBitFieldLoad(
+      TestBitFieldStruct{}, false, FromConstexpr<uint8>(0), 0, false);
+  TestBitFieldLoad(TestBitFieldStruct{d: d, b: b}, false, b, 0, d);
+  TestBitFieldLoad(
+      TestBitFieldStruct{c: c}, false, FromConstexpr<uint8>(0), c, false);
+
+  const low = TestOverlappingBitFieldStruct{low: c};
+  check(low.low == c);
+  check(low.high == 0);
+  check(low.all == c);
+  const high = TestOverlappingBitFieldStruct{high: c};
+  check(high.low == 0);
+  check(high.high == c);
+  check(high.all == (c << 4));
+  const all = TestOverlappingBitFieldStruct{all: c};
+  check(all.low == c);
+  check(all.high == 0);
+  check(all.all == c);
 }
 
 // Some other bitfield structs, to verify getting uintptr values out of word32

test/unittests/torque/torque-unittest.cc

@@ -917,6 +917,22 @@ TEST(Torque, CatchFirstHandler) {
           "catch handler always has to be first, before any label handler"));
 }
 
+TEST(Torque, OverlappingBitFieldInitializers) {
+  const std::string source = R"(
+    extern macro Unsigned(int32): uint32;
+    bitfield struct S extends uint32 {
+      a: uint32: 3 bit;
+      b: uint32: 3 bit at 2;
+    }
+    @export
+    macro Test(a: uint32, b: uint32): S {
+      return S{a: a, b: b};
+    }
+  )";
+  ExpectFailingCompilation(source,
+                           HasSubstr("Overlapping bitfield initializer: b"));
+}
+
 TEST(Torque, BitFieldLogicalAnd) {
   std::string prelude = R"(
     bitfield struct S extends uint32 {
@@ -1663,8 +1679,8 @@ TEST(TorqueLayoutLoader, ImporterRejectsMismatchedBitFieldStruct) {
           TestScalarClass("TestBitFields"),
       TestScalarJson(TestBitFieldStorage(), 1),
       /*use_cpp_layouts=*/true,
-      ".tq field \"bar: uint8: 2 bit\" differs from C++ field "
-      "\"bar: uint8: 3 bit\"");
+      ".tq field \"bar: uint8: 2 bit at 1\" differs from C++ field "
+      "\"bar: uint8: 3 bit at 1\"");
 }
 
 TEST(TorqueLayoutLoader, VerifierAcceptsMatchingBitFieldStruct) {
@@ -1814,10 +1830,8 @@ TEST(TorqueLayoutLoader, VerifierRejectsSmiBitFieldWithoutSmiTagged) {
       "a bit field group in a Smi in C++, Smi in Torque");
 }
 
-TEST(TorqueLayoutLoader, RejectsBitFieldGap) {
-  // Torque derives offsets from declaration order, so a hole in the record
-  // would shift every following field instead of being preserved.
-  ExpectFailingLayoutCompilation(
+TEST(TorqueLayoutLoader, AcceptsBitFieldGap) {
+  ExpectSuccessfulLayoutCompilation(
       TestScalarClass("TestBitFields"),
       TestScalarJson(
           TestBitFieldStorage(R"({"cpp_name": "IsFooBit", "type": "bool", )"
@@ -1825,7 +1839,7 @@ TEST(TorqueLayoutLoader, RejectsBitFieldGap) {
                               R"({"cpp_name": "BarBits", "type": "uint8_t", )"
                               R"("offset": 2, "num_bits": 3})"),
           1),
-      /*use_cpp_layouts=*/true, "starts at bit 2, expected 1");
+      /*use_cpp_layouts=*/true);
 }
 
 TEST(TorqueLayoutLoader, RejectsBitFieldNameThatDoesNotRoundTrip) {

tools/metagen/layout_extract.py

@@ -333,7 +333,7 @@ class _Extractor:
                         group: cindex.Cursor,
                         base: cindex.Type,
                         smi: bool = False) -> StorageType:
-    """Read base::BitField aliases in bit order.
+    """Read direct base::BitField aliases in declaration order.
 
     Smi storage uses a tagged slot instead of the group's integer width.
     """

tools/metagen/layout_ir.py

@@ -310,7 +310,7 @@ class StorageType:
   # C++ type name without namespaces; includes enclosing classes for bitfields.
   name: str | None = None
   # "bitfield" / "smi_bitfield": the group's fields, in declaration
-  # order, which is bit order.
+  # order. Fields may overlap.
   bits: tuple[BitFieldEntry, ...] = ()
 
   def __post_init__(self) -> None:
@@ -345,17 +345,11 @@ class StorageType:
         _check(self.width is None, "smi_bitfield storage with a width")
         capacity = 31
       _check(bool(self.bits), f"{self.kind} storage without fields")
-      # A base::BitField chain starts at bit 0 and has no holes; a gap
-      # means an alias is missing from the group.
-      next_bit = 0
       for bit in self.bits:
+        end = bit.offset + bit.num_bits
         _check(
-            bit.offset == next_bit,
-            f"bitfield {self.name}: {bit.cpp_name} starts at {bit.offset}, "
-            f"expected {next_bit}")
-        next_bit += bit.num_bits
-      _check(next_bit <= capacity,
-             f"bitfield {self.name}: {next_bit} bits exceed the storage")
+            end <= capacity, f"bitfield {self.name}: {bit.cpp_name}: "
+            f"{end} bits exceed the storage")
     else:
       _check(not self.bits, f"{self.kind} storage with bit fields")
     if self.pointer_width_alias is not None:

tools/metagen/tests/layout_extract_test.py

@@ -724,6 +724,7 @@ class BitFieldGroupTest(unittest.TestCase):
           using IsFooBit = First<bool, 1>;
           using KindBits = IsFooBit::Next<Kind, 2>;
           using CountBits = KindBits::Next<int32_t, 5>;
+          using AlternativeBits = IsFooBit::Next<uint32_t, 31>;
         };
         Bits bits_;
       };
@@ -737,6 +738,7 @@ class BitFieldGroupTest(unittest.TestCase):
         ("IsFooBit", "bool", 0, 1),
         ("KindBits", "Kind", 1, 2),
         ("CountBits", "int32_t", 3, 5),
+        ("AlternativeBits", "uint32_t", 1, 31),
     ])
 
   def test_enclosing_classes_name_the_group(self):

tools/metagen/tests/layout_ir_test.py

@@ -154,17 +154,19 @@ class StorageTypeTest(unittest.TestCase):
     with self.assertRaisesRegex(LayoutError, "without fields"):
       bitfield()
 
-  def test_bitfield_chain_must_be_gapless(self):
-    with self.assertRaisesRegex(LayoutError, "starts at 2, expected 1"):
-      bitfield(
-          BitFieldEntry("IsFooBit", "bool", 0, 1),
-          BitFieldEntry("KindBits", "Kind", 2, 3))
+  def test_bitfield_gaps_and_overlaps_roundtrip(self):
+    s = bitfield(
+        BitFieldEntry("KindBits", "Kind", 2, 3),
+        BitFieldEntry("PayloadBits", "uint8_t", 1, 7),
+        BitFieldEntry("IsFooBit", "bool", 0, 1))
+    self.assertEqual(StorageType.from_json(s.to_json()), s)
 
   def test_bitfield_chain_must_fit_the_storage(self):
     with self.assertRaisesRegex(LayoutError, "exceed the storage"):
       bitfield(
           BitFieldEntry("LowBits", "uint8_t", 0, 5),
-          BitFieldEntry("HighBits", "uint8_t", 5, 4))
+          BitFieldEntry("HighBits", "uint8_t", 5, 4),
+          BitFieldEntry("AlternativeBits", "uint8_t", 0, 8))
 
   def test_smi_bitfield_roundtrip(self):
     s = smi_bitfield(
@@ -177,7 +179,8 @@ class StorageTypeTest(unittest.TestCase):
     with self.assertRaisesRegex(LayoutError, "32 bits exceed the storage"):
       smi_bitfield(
           BitFieldEntry("IsFooBit", "bool", 0, 1),
-          BitFieldEntry("CountBits", "uint32_t", 1, 31))
+          BitFieldEntry("CountBits", "uint32_t", 1, 31),
+          BitFieldEntry("AlternativeBits", "uint32_t", 0, 31))
 
   def test_smi_bitfield_has_no_width(self):
     with self.assertRaisesRegex(LayoutError, "with a width"):