From 41458ba4b1fc8a49fab4cfcd29ae019ebd60fbd7 Mon Sep 17 00:00:00 2001 From: Tess Strickland Date: Wed, 12 Aug 2020 14:56:44 +0000 Subject: [PATCH] [vm] Add more checks when packing nullability during serialization. Also only shift as many bits as needed to fit nullability into the serialized value in case additional type states or flags are added in the future. Change-Id: Ic821070971f77040e05d5aeb53a5a0ed85cc8d09 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/158180 Commit-Queue: Tess Strickland Reviewed-by: Ryan Macnak --- runtime/vm/clustered_snapshot.cc | 85 +++++++++++++++++--------------- runtime/vm/object.h | 1 + runtime/vm/raw_object.h | 21 ++++---- 3 files changed, 56 insertions(+), 51 deletions(-) diff --git a/runtime/vm/clustered_snapshot.cc b/runtime/vm/clustered_snapshot.cc index 637cf2ee4cd..956b5eebc1e 100644 --- a/runtime/vm/clustered_snapshot.cc +++ b/runtime/vm/clustered_snapshot.cc @@ -3366,6 +3366,10 @@ class LibraryPrefixDeserializationCluster : public DeserializationCluster { } }; +// Used to pack nullability into other serialized values. +static constexpr intptr_t kNullabilityBitSize = 2; +static constexpr intptr_t kNullabilityBitMask = (1 << kNullabilityBitSize) - 1; + #if !defined(DART_PRECOMPILED_RUNTIME) class TypeSerializationCluster : public SerializationCluster { public: @@ -3412,31 +3416,31 @@ class TypeSerializationCluster : public SerializationCluster { void WriteFill(Serializer* s) { intptr_t count = canonical_objects_.length(); for (intptr_t i = 0; i < count; i++) { - TypePtr type = canonical_objects_[i]; - AutoTraceObject(type); - WriteFromTo(type); - s->WriteTokenPosition(type->ptr()->token_pos_); - const uint8_t combined = - (type->ptr()->type_state_ << 4) | type->ptr()->nullability_; - ASSERT(type->ptr()->type_state_ == (combined >> 4)); - ASSERT(type->ptr()->nullability_ == (combined & 0xf)); - s->Write(combined); + WriteType(s, canonical_objects_[i]); } count = objects_.length(); for (intptr_t i = 0; i < count; i++) { - TypePtr type = objects_[i]; - AutoTraceObject(type); - WriteFromTo(type); - s->WriteTokenPosition(type->ptr()->token_pos_); - const uint8_t combined = - (type->ptr()->type_state_ << 4) | type->ptr()->nullability_; - ASSERT(type->ptr()->type_state_ == (combined >> 4)); - ASSERT(type->ptr()->nullability_ == (combined & 0xf)); - s->Write(combined); + WriteType(s, objects_[i]); } } private: + void WriteType(Serializer* s, TypePtr type) { + AutoTraceObject(type); + WriteFromTo(type); + s->WriteTokenPosition(type->ptr()->token_pos_); + ASSERT(type->ptr()->type_state_ < (1 << TypeLayout::kTypeStateBitSize)); + ASSERT(type->ptr()->nullability_ < (1 << kNullabilityBitSize)); + static_assert(TypeLayout::kTypeStateBitSize + kNullabilityBitSize <= + kBitsPerByte * sizeof(uint8_t), + "Cannot pack type_state_ and nullability_ into a uint8_t"); + const uint8_t combined = (type->ptr()->type_state_ << kNullabilityBitSize) | + type->ptr()->nullability_; + ASSERT_EQUAL(type->ptr()->type_state_, combined >> kNullabilityBitSize); + ASSERT_EQUAL(type->ptr()->nullability_, combined & kNullabilityBitMask); + s->Write(combined); + } + GrowableArray canonical_objects_; GrowableArray objects_; }; @@ -3468,26 +3472,12 @@ class TypeDeserializationCluster : public DeserializationCluster { for (intptr_t id = canonical_start_index_; id < canonical_stop_index_; id++) { TypePtr type = static_cast(d->Ref(id)); - bool is_canonical = true; - Deserializer::InitializeHeader(type, kTypeCid, Type::InstanceSize(), - is_canonical); - ReadFromTo(type); - type->ptr()->token_pos_ = d->ReadTokenPosition(); - const uint8_t combined = d->Read(); - type->ptr()->type_state_ = combined >> 4; - type->ptr()->nullability_ = combined & 0xf; + ReadType(d, type, /*is_canonical=*/true); } for (intptr_t id = start_index_; id < stop_index_; id++) { TypePtr type = static_cast(d->Ref(id)); - bool is_canonical = false; - Deserializer::InitializeHeader(type, kTypeCid, Type::InstanceSize(), - is_canonical); - ReadFromTo(type); - type->ptr()->token_pos_ = d->ReadTokenPosition(); - const uint8_t combined = d->Read(); - type->ptr()->type_state_ = combined >> 4; - type->ptr()->nullability_ = combined & 0xf; + ReadType(d, type, /*is_canonical=*/false); } } @@ -3523,6 +3513,16 @@ class TypeDeserializationCluster : public DeserializationCluster { } private: + void ReadType(Deserializer* d, TypePtr type, bool is_canonical) { + Deserializer::InitializeHeader(type, kTypeCid, Type::InstanceSize(), + is_canonical); + ReadFromTo(type); + type->ptr()->token_pos_ = d->ReadTokenPosition(); + const uint8_t combined = d->Read(); + type->ptr()->type_state_ = combined >> kNullabilityBitSize; + type->ptr()->nullability_ = combined & kNullabilityBitMask; + } + intptr_t canonical_start_index_; intptr_t canonical_stop_index_; }; @@ -3659,10 +3659,15 @@ class TypeParameterSerializationCluster : public SerializationCluster { s->Write(type->ptr()->parameterized_class_id_); s->WriteTokenPosition(type->ptr()->token_pos_); s->Write(type->ptr()->index_); - const uint8_t combined = - (type->ptr()->flags_ << 4) | type->ptr()->nullability_; - ASSERT(type->ptr()->flags_ == (combined >> 4)); - ASSERT(type->ptr()->nullability_ == (combined & 0xf)); + ASSERT(type->ptr()->flags_ < (1 << TypeParameterLayout::kFlagsBitSize)); + ASSERT(type->ptr()->nullability_ < (1 << kNullabilityBitSize)); + static_assert(TypeParameterLayout::kFlagsBitSize + kNullabilityBitSize <= + kBitsPerByte * sizeof(uint8_t), + "Cannot pack flags_ and nullability_ into a uint8_t"); + const uint8_t combined = (type->ptr()->flags_ << kNullabilityBitSize) | + type->ptr()->nullability_; + ASSERT_EQUAL(type->ptr()->flags_, combined >> kNullabilityBitSize); + ASSERT_EQUAL(type->ptr()->nullability_, combined & kNullabilityBitMask); s->Write(combined); } @@ -3752,8 +3757,8 @@ class TypeParameterDeserializationCluster : public DeserializationCluster { type->ptr()->token_pos_ = d->ReadTokenPosition(); type->ptr()->index_ = d->Read(); const uint8_t combined = d->Read(); - type->ptr()->flags_ = combined >> 4; - type->ptr()->nullability_ = combined & 0xf; + type->ptr()->flags_ = combined >> kNullabilityBitSize; + type->ptr()->nullability_ = combined & kNullabilityBitMask; } intptr_t canonical_start_index_; diff --git a/runtime/vm/object.h b/runtime/vm/object.h index cd1e1f0131a..2be3ae6d963 100644 --- a/runtime/vm/object.h +++ b/runtime/vm/object.h @@ -900,6 +900,7 @@ enum class Nullability : int8_t { kNullable = 0, kNonNullable = 1, kLegacy = 2, + // Adjust kNullabilityBitSize in clustered_snapshot.cc if adding new values. }; // Equality kind between types. diff --git a/runtime/vm/raw_object.h b/runtime/vm/raw_object.h index afcf33deb78..b4108418f81 100644 --- a/runtime/vm/raw_object.h +++ b/runtime/vm/raw_object.h @@ -2190,9 +2190,12 @@ class AbstractTypeLayout : public InstanceLayout { kBeingFinalized, // In the process of being finalized. kFinalizedInstantiated, // Instantiated type ready for use. kFinalizedUninstantiated, // Uninstantiated type ready for use. + // Adjust kTypeStateBitSize if more are added. }; protected: + static constexpr intptr_t kTypeStateBitSize = 2; + uword type_test_stub_entry_point_; // Accessed from generated code. CodePtr type_test_stub_; // Must be the last field, since subclasses use it // in their VISIT_FROM. @@ -2237,17 +2240,6 @@ class TypeRefLayout : public AbstractTypeLayout { }; class TypeParameterLayout : public AbstractTypeLayout { - public: - enum { - kFinalizedBit = 0, - kGenericCovariantImplBit, - kDeclarationBit, - }; - class FinalizedBit : public BitField {}; - class GenericCovariantImplBit - : public BitField {}; - class DeclarationBit : public BitField {}; - private: RAW_HEAP_OBJECT_IMPLEMENTATION(TypeParameter); @@ -2263,6 +2255,13 @@ class TypeParameterLayout : public AbstractTypeLayout { uint8_t flags_; int8_t nullability_; + using FinalizedBit = BitField; + using GenericCovariantImplBit = + BitField; + using DeclarationBit = + BitField; + static constexpr intptr_t kFlagsBitSize = DeclarationBit::kNextBit; + ObjectPtr* to_snapshot(Snapshot::Kind kind) { return to(); } friend class CidRewriteVisitor;