[vm] Possible fix for data race during TypeRef canonicalization

AbstractType::flags_ is changed to atomic with relaxed memory order
in order to allow accesses to a mutable type state outside of the type
canonicalization mutex and program lock.

TEST=iso-stress bot
Fixes #50065

Change-Id: Iaebd4ea809e7f1bb0b9afe29308e0e6a2bd3d6ec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/262503
Reviewed-by: Ryan Macnak <[email protected]>
Commit-Queue: Alexander Markov <[email protected]>

Author: alexmarkov

runtime/vm/app_snapshot.cc

@@ -4124,7 +4124,7 @@ class TypeSerializationCluster
     }
 #endif
     WriteFromTo(type);
-    s->WriteUnsigned(type->untag()->flags_);
+    s->WriteUnsigned(type->untag()->flags());
   }
 };
 #endif  // !DART_PRECOMPILED_RUNTIME
@@ -4153,7 +4153,7 @@ class TypeDeserializationCluster
       Deserializer::InitializeHeader(type, kTypeCid, Type::InstanceSize(),
                                      mark_canonical);
       d.ReadFromTo(type);
-      type->untag()->flags_ = d.ReadUnsigned();
+      type->untag()->set_flags(d.ReadUnsigned());
     }
   }
 
@@ -4235,8 +4235,8 @@ class FunctionTypeSerializationCluster
   void WriteFunctionType(Serializer* s, FunctionTypePtr type) {
     AutoTraceObject(type);
     WriteFromTo(type);
-    ASSERT(Utils::IsUint(8, type->untag()->flags_));
-    s->Write<uint8_t>(type->untag()->flags_);
+    ASSERT(Utils::IsUint(8, type->untag()->flags()));
+    s->Write<uint8_t>(type->untag()->flags());
     s->Write<uint32_t>(type->untag()->packed_parameter_counts_);
     s->Write<uint16_t>(type->untag()->packed_type_parameter_counts_);
   }
@@ -4267,7 +4267,7 @@ class FunctionTypeDeserializationCluster
       Deserializer::InitializeHeader(
           type, kFunctionTypeCid, FunctionType::InstanceSize(), mark_canonical);
       d.ReadFromTo(type);
-      type->untag()->flags_ = d.Read<uint8_t>();
+      type->untag()->set_flags(d.Read<uint8_t>());
       type->untag()->packed_parameter_counts_ = d.Read<uint32_t>();
       type->untag()->packed_type_parameter_counts_ = d.Read<uint16_t>();
     }
@@ -4351,8 +4351,8 @@ class RecordTypeSerializationCluster
   void WriteRecordType(Serializer* s, RecordTypePtr type) {
     AutoTraceObject(type);
     WriteFromTo(type);
-    ASSERT(Utils::IsUint(8, type->untag()->flags_));
-    s->Write<uint8_t>(type->untag()->flags_);
+    ASSERT(Utils::IsUint(8, type->untag()->flags()));
+    s->Write<uint8_t>(type->untag()->flags());
   }
 };
 #endif  // !DART_PRECOMPILED_RUNTIME
@@ -4380,7 +4380,7 @@ class RecordTypeDeserializationCluster
       Deserializer::InitializeHeader(
           type, kRecordTypeCid, RecordType::InstanceSize(), mark_canonical);
       d.ReadFromTo(type);
-      type->untag()->flags_ = d.Read<uint8_t>();
+      type->untag()->set_flags(d.Read<uint8_t>());
     }
   }
 
@@ -4556,8 +4556,8 @@ class TypeParameterSerializationCluster
     s->Write<int32_t>(type->untag()->parameterized_class_id_);
     s->Write<uint8_t>(type->untag()->base_);
     s->Write<uint8_t>(type->untag()->index_);
-    ASSERT(Utils::IsUint(8, type->untag()->flags_));
-    s->Write<uint8_t>(type->untag()->flags_);
+    ASSERT(Utils::IsUint(8, type->untag()->flags()));
+    s->Write<uint8_t>(type->untag()->flags());
   }
 };
 #endif  // !DART_PRECOMPILED_RUNTIME
@@ -4590,7 +4590,7 @@ class TypeParameterDeserializationCluster
       type->untag()->parameterized_class_id_ = d.Read<int32_t>();
       type->untag()->base_ = d.Read<uint8_t>();
       type->untag()->index_ = d.Read<uint8_t>();
-      type->untag()->flags_ = d.Read<uint8_t>();
+      type->untag()->set_flags(d.Read<uint8_t>());
     }
   }
 

runtime/vm/object.cc

@@ -20336,19 +20336,19 @@ void AbstractType::SetIsBeingFinalized() const {
 }
 
 void AbstractType::set_flags(uint32_t value) const {
-  StoreNonPointer(&untag()->flags_, value);
+  untag()->set_flags(value);
 }
 
 void AbstractType::set_type_state(UntaggedAbstractType::TypeState value) const {
   ASSERT(!IsCanonical());
   set_flags(
-      UntaggedAbstractType::TypeStateBits::update(value, untag()->flags_));
+      UntaggedAbstractType::TypeStateBits::update(value, untag()->flags()));
 }
 
 void AbstractType::set_nullability(Nullability value) const {
   ASSERT(!IsCanonical());
   set_flags(UntaggedAbstractType::NullabilityBits::update(
-      static_cast<uint8_t>(value), untag()->flags_));
+      static_cast<uint8_t>(value), untag()->flags()));
 }
 
 bool AbstractType::IsEquivalent(const Instance& other,

runtime/vm/object.h

@@ -8197,7 +8197,7 @@ class AbstractType : public Instance {
 
   Nullability nullability() const {
     return static_cast<Nullability>(
-        UntaggedAbstractType::NullabilityBits::decode(untag()->flags_));
+        UntaggedAbstractType::NullabilityBits::decode(untag()->flags()));
   }
   // Returns true if type has '?' nullability suffix, or it is a
   // built-in type which is always nullable (Null, dynamic or void).
@@ -8493,7 +8493,7 @@ class AbstractType : public Instance {
 
   UntaggedAbstractType::TypeState type_state() const {
     return static_cast<UntaggedAbstractType::TypeState>(
-        UntaggedAbstractType::TypeStateBits::decode(untag()->flags_));
+        UntaggedAbstractType::TypeStateBits::decode(untag()->flags()));
   }
   void set_flags(uint32_t value) const;
   void set_type_state(UntaggedAbstractType::TypeState value) const;

runtime/vm/raw_object.h

@@ -2613,10 +2613,15 @@ class UntaggedAbstractType : public UntaggedInstance {
   // Accessed from generated code.
   std::atomic<uword> type_test_stub_entry_point_;
   // Accessed from generated code.
-  uint32_t flags_;
+  std::atomic<uint32_t> flags_;
   COMPRESSED_POINTER_FIELD(CodePtr, type_test_stub)
   VISIT_FROM(type_test_stub)
 
+  uint32_t flags() const { return flags_.load(std::memory_order_relaxed); }
+  void set_flags(uint32_t value) {
+    flags_.store(value, std::memory_order_relaxed);
+  }
+
  public:
   enum TypeState {
     kAllocated,                // Initial state.
@@ -2625,13 +2630,13 @@ class UntaggedAbstractType : public UntaggedInstance {
     kFinalizedUninstantiated,  // Uninstantiated type ready for use.
   };
 
-  using NullabilityBits = BitField<decltype(flags_), uint8_t, 0, 2>;
+  using NullabilityBits = BitField<uint32_t, uint8_t, 0, 2>;
   static constexpr intptr_t kNullabilityMask = NullabilityBits::mask();
 
   static constexpr intptr_t kTypeStateShift = NullabilityBits::kNextBit;
   static constexpr intptr_t kTypeStateBits = 2;
   using TypeStateBits =
-      BitField<decltype(flags_), uint8_t, kTypeStateShift, kTypeStateBits>;
+      BitField<uint32_t, uint8_t, kTypeStateShift, kTypeStateBits>;
 
  private:
   RAW_HEAP_OBJECT_IMPLEMENTATION(AbstractType);
@@ -2643,7 +2648,7 @@ class UntaggedAbstractType : public UntaggedInstance {
 class UntaggedType : public UntaggedAbstractType {
  public:
   static constexpr intptr_t kTypeClassIdShift = TypeStateBits::kNextBit;
-  using TypeClassIdBits = BitField<decltype(flags_),
+  using TypeClassIdBits = BitField<uint32_t,
                                    ClassIdTagType,
                                    kTypeClassIdShift,
                                    sizeof(ClassIdTagType) * kBitsPerByte>;
@@ -2658,10 +2663,10 @@ class UntaggedType : public UntaggedAbstractType {
   CompressedObjectPtr* to_snapshot(Snapshot::Kind kind) { return to(); }
 
   ClassIdTagType type_class_id() const {
-    return TypeClassIdBits::decode(flags_);
+    return TypeClassIdBits::decode(flags());
   }
   void set_type_class_id(ClassIdTagType value) {
-    flags_ = TypeClassIdBits::update(value, flags_);
+    set_flags(TypeClassIdBits::update(value, flags()));
   }
 
   friend class compiler::target::UntaggedType;