[vm/concurrency] Add store-release/read-acquire barriers for Class::{state_bits_,functions_,fields_}

Issue https://github.com/dart-lang/sdk/issues/36097

Change-Id: I7e1fc63066ce813bc317afa8ad1a58394cea6afa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/166856
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>

runtime/vm/object.cc

@@ -2906,7 +2906,7 @@ void Class::set_num_type_arguments(intptr_t value) const {
 }
 
 void Class::set_has_pragma(bool value) const {
-  set_state_bits(HasPragmaBit::update(value, raw_ptr()->state_bits_));
+  set_state_bits(HasPragmaBit::update(value, state_bits()));
 }
 
 // Initialize class fields of type Array with empty array.
@@ -2917,8 +2917,8 @@ void Class::InitEmptyFields() {
   }
   StorePointer(&raw_ptr()->interfaces_, Object::empty_array().raw());
   StorePointer(&raw_ptr()->constants_, Object::null_array().raw());
-  StorePointer(&raw_ptr()->functions_, Object::empty_array().raw());
-  StorePointer(&raw_ptr()->fields_, Object::empty_array().raw());
+  set_functions(Object::empty_array());
+  set_fields(Object::empty_array());
   StorePointer(&raw_ptr()->invocation_dispatcher_cache_,
                Object::empty_array().raw());
 }
@@ -3002,7 +3002,7 @@ typedef UnorderedHashSet<ClassFunctionsTraits> ClassFunctionsSet;
 void Class::SetFunctions(const Array& value) const {
   ASSERT(Thread::Current()->IsMutatorThread());
   ASSERT(!value.IsNull());
-  StorePointer(&raw_ptr()->functions_, value.raw());
+  set_functions(value);
   const intptr_t len = value.Length();
   if (len >= kFunctionLookupHashTreshold) {
     ClassFunctionsSet set(HashTables::New<ClassFunctionsSet>(len, Heap::kOld));
@@ -3022,15 +3022,15 @@ void Class::SetFunctions(const Array& value) const {
 void Class::AddFunction(const Function& function) const {
   ASSERT(Thread::Current()->IsMutatorThread());
   const Array& arr = Array::Handle(functions());
-  const Array& new_arr =
+  const Array& new_array =
       Array::Handle(Array::Grow(arr, arr.Length() + 1, Heap::kOld));
-  new_arr.SetAt(arr.Length(), function);
-  StorePointer(&raw_ptr()->functions_, new_arr.raw());
+  new_array.SetAt(arr.Length(), function);
+  set_functions(new_array);
   // Add to hash table, if any.
-  const intptr_t new_len = new_arr.Length();
+  const intptr_t new_len = new_array.Length();
   if (new_len == kFunctionLookupHashTreshold) {
     // Transition to using hash table.
-    SetFunctions(new_arr);
+    SetFunctions(new_array);
   } else if (new_len > kFunctionLookupHashTreshold) {
     ClassFunctionsSet set(raw_ptr()->functions_hash_table_);
     set.Insert(function);
@@ -3141,7 +3141,8 @@ void Class::set_signature_function(const Function& value) const {
 }
 
 void Class::set_state_bits(intptr_t bits) const {
-  StoreNonPointer(&raw_ptr()->state_bits_, static_cast<uint32_t>(bits));
+  StoreNonPointer<uint32_t, uint32_t, std::memory_order_release>(
+      &raw_ptr()->state_bits_, static_cast<uint32_t>(bits));
 }
 
 void Class::set_library(const Library& value) const {
@@ -3154,6 +3155,20 @@ void Class::set_type_parameters(const TypeArguments& value) const {
   StorePointer(&raw_ptr()->type_parameters_, value.raw());
 }
 
+void Class::set_functions(const Array& value) const {
+  // Ensure all writes to the [Function]s are visible by the time the array
+  // is visible.
+  StorePointer<ArrayPtr, std::memory_order_release>(&raw_ptr()->functions_,
+                                                    value.raw());
+}
+
+void Class::set_fields(const Array& value) const {
+  // Ensure all writes to the [Field]s are visible by the time the array
+  // is visible.
+  StorePointer<ArrayPtr, std::memory_order_release>(&raw_ptr()->fields_,
+                                                    value.raw());
+}
+
 intptr_t Class::NumTypeParameters(Thread* thread) const {
   if (!is_declaration_loaded()) {
     ASSERT(is_prefinalized());
@@ -4350,7 +4365,7 @@ void Class::SetFields(const Array& value) const {
   }
 #endif
   // The value of static fields is already initialized to null.
-  StorePointer(&raw_ptr()->fields_, value.raw());
+  set_fields(value);
 }
 
 void Class::AddField(const Field& field) const {
@@ -4864,71 +4879,70 @@ int32_t Class::SourceFingerprint() const {
 }
 
 void Class::set_is_implemented() const {
-  set_state_bits(ImplementedBit::update(true, raw_ptr()->state_bits_));
+  set_state_bits(ImplementedBit::update(true, state_bits()));
 }
 
 void Class::set_is_abstract() const {
-  set_state_bits(AbstractBit::update(true, raw_ptr()->state_bits_));
+  set_state_bits(AbstractBit::update(true, state_bits()));
 }
 
 void Class::set_is_declaration_loaded() const {
   ASSERT(!is_declaration_loaded());
-  set_state_bits(ClassLoadingBits::update(ClassLayout::kDeclarationLoaded,
-                                          raw_ptr()->state_bits_));
+  set_state_bits(
+      ClassLoadingBits::update(ClassLayout::kDeclarationLoaded, state_bits()));
 }
 
 void Class::set_is_type_finalized() const {
   ASSERT(is_declaration_loaded());
   ASSERT(!is_type_finalized());
-  set_state_bits(ClassLoadingBits::update(ClassLayout::kTypeFinalized,
-                                          raw_ptr()->state_bits_));
+  set_state_bits(
+      ClassLoadingBits::update(ClassLayout::kTypeFinalized, state_bits()));
 }
 
 void Class::set_is_synthesized_class() const {
-  set_state_bits(SynthesizedClassBit::update(true, raw_ptr()->state_bits_));
+  set_state_bits(SynthesizedClassBit::update(true, state_bits()));
 }
 
 void Class::set_is_enum_class() const {
-  set_state_bits(EnumBit::update(true, raw_ptr()->state_bits_));
+  set_state_bits(EnumBit::update(true, state_bits()));
 }
 
 void Class::set_is_const() const {
-  set_state_bits(ConstBit::update(true, raw_ptr()->state_bits_));
+  set_state_bits(ConstBit::update(true, state_bits()));
 }
 
 void Class::set_is_transformed_mixin_application() const {
-  set_state_bits(
-      TransformedMixinApplicationBit::update(true, raw_ptr()->state_bits_));
+  set_state_bits(TransformedMixinApplicationBit::update(true, state_bits()));
 }
 
 void Class::set_is_fields_marked_nullable() const {
-  set_state_bits(FieldsMarkedNullableBit::update(true, raw_ptr()->state_bits_));
+  set_state_bits(FieldsMarkedNullableBit::update(true, state_bits()));
 }
 
 void Class::set_is_allocated(bool value) const {
-  set_state_bits(IsAllocatedBit::update(value, raw_ptr()->state_bits_));
+  set_state_bits(IsAllocatedBit::update(value, state_bits()));
 }
 
 void Class::set_is_loaded(bool value) const {
-  set_state_bits(IsLoadedBit::update(value, raw_ptr()->state_bits_));
+  set_state_bits(IsLoadedBit::update(value, state_bits()));
 }
 
 void Class::set_is_finalized() const {
   ASSERT(!is_finalized());
-  set_state_bits(ClassFinalizedBits::update(ClassLayout::kFinalized,
-                                            raw_ptr()->state_bits_));
+  set_state_bits(
+      ClassFinalizedBits::update(ClassLayout::kFinalized, state_bits()));
 }
 
 void Class::set_is_allocate_finalized() const {
   ASSERT(!is_allocate_finalized());
   set_state_bits(ClassFinalizedBits::update(ClassLayout::kAllocateFinalized,
-                                            raw_ptr()->state_bits_));
+                                            state_bits()));
 }
 
 void Class::set_is_prefinalized() const {
   ASSERT(!is_finalized());
-  set_state_bits(ClassFinalizedBits::update(ClassLayout::kPreFinalized,
-                                            raw_ptr()->state_bits_));
+  set_state_bits(
+      ClassFinalizedBits::update(ClassLayout::kPreFinalized, state_bits()));
 }
 
 void Class::set_interfaces(const Array& value) const {

runtime/vm/object.h

@@ -1271,7 +1271,11 @@ class Class : public Object {
   ErrorPtr VerifyEntryPoint() const;
 
   // Returns an array of instance and static fields defined by this class.
-  ArrayPtr fields() const { return raw_ptr()->fields_; }
+  ArrayPtr fields() const {
+    // We rely on the fact that any loads from the array are dependent loads
+    // and avoid the load-acquire barrier here.
+    return raw_ptr()->fields_;
+  }
   void SetFields(const Array& value) const;
   void AddField(const Field& field) const;
   void AddFields(const GrowableArray<const Field*>& fields) const;
@@ -1291,8 +1295,11 @@ class Class : public Object {
   // Returns true if non-static fields are defined.
   bool HasInstanceFields() const;
 
-  // TODO(koda): Unite w/ hash table.
-  ArrayPtr functions() const { return raw_ptr()->functions_; }
+  ArrayPtr functions() const {
+    // We rely on the fact that any loads from the array are dependent loads
+    // and avoid the load-acquire barrier here.
+    return raw_ptr()->functions_;
+  }
   void SetFunctions(const Array& value) const;
   void AddFunction(const Function& function) const;
   FunctionPtr FunctionFromIndex(intptr_t idx) const;
@@ -1339,18 +1346,14 @@ class Class : public Object {
     return RoundedAllocationSize(sizeof(ClassLayout));
   }
 
-  bool is_implemented() const {
-    return ImplementedBit::decode(raw_ptr()->state_bits_);
-  }
+  bool is_implemented() const { return ImplementedBit::decode(state_bits()); }
   void set_is_implemented() const;
 
-  bool is_abstract() const {
-    return AbstractBit::decode(raw_ptr()->state_bits_);
-  }
+  bool is_abstract() const { return AbstractBit::decode(state_bits()); }
   void set_is_abstract() const;
 
   ClassLayout::ClassLoadingState class_loading_state() const {
-    return ClassLoadingBits::decode(raw_ptr()->state_bits_);
+    return ClassLoadingBits::decode(state_bits());
   }
 
   bool is_declaration_loaded() const {
@@ -1364,35 +1367,35 @@ class Class : public Object {
   void set_is_type_finalized() const;
 
   bool is_synthesized_class() const {
-    return SynthesizedClassBit::decode(raw_ptr()->state_bits_);
+    return SynthesizedClassBit::decode(state_bits());
   }
   void set_is_synthesized_class() const;
 
-  bool is_enum_class() const { return EnumBit::decode(raw_ptr()->state_bits_); }
+  bool is_enum_class() const { return EnumBit::decode(state_bits()); }
   void set_is_enum_class() const;
 
   bool is_finalized() const {
-    return ClassFinalizedBits::decode(raw_ptr()->state_bits_) ==
+    return ClassFinalizedBits::decode(state_bits()) ==
                ClassLayout::kFinalized ||
-           ClassFinalizedBits::decode(raw_ptr()->state_bits_) ==
+           ClassFinalizedBits::decode(state_bits()) ==
                ClassLayout::kAllocateFinalized;
   }
   void set_is_finalized() const;
 
   bool is_allocate_finalized() const {
-    return ClassFinalizedBits::decode(raw_ptr()->state_bits_) ==
+    return ClassFinalizedBits::decode(state_bits()) ==
            ClassLayout::kAllocateFinalized;
   }
   void set_is_allocate_finalized() const;
 
   bool is_prefinalized() const {
-    return ClassFinalizedBits::decode(raw_ptr()->state_bits_) ==
+    return ClassFinalizedBits::decode(state_bits()) ==
            ClassLayout::kPreFinalized;
   }
 
   void set_is_prefinalized() const;
 
-  bool is_const() const { return ConstBit::decode(raw_ptr()->state_bits_); }
+  bool is_const() const { return ConstBit::decode(state_bits()); }
   void set_is_const() const;
 
   // Tests if this is a mixin application class which was desugared
@@ -1402,21 +1405,19 @@ class Class : public Object {
   // In such case, its mixed-in type was pulled into the end of
   // interfaces list.
   bool is_transformed_mixin_application() const {
-    return TransformedMixinApplicationBit::decode(raw_ptr()->state_bits_);
+    return TransformedMixinApplicationBit::decode(state_bits());
   }
   void set_is_transformed_mixin_application() const;
 
   bool is_fields_marked_nullable() const {
-    return FieldsMarkedNullableBit::decode(raw_ptr()->state_bits_);
+    return FieldsMarkedNullableBit::decode(state_bits());
   }
   void set_is_fields_marked_nullable() const;
 
-  bool is_allocated() const {
-    return IsAllocatedBit::decode(raw_ptr()->state_bits_);
-  }
+  bool is_allocated() const { return IsAllocatedBit::decode(state_bits()); }
   void set_is_allocated(bool value) const;
 
-  bool is_loaded() const { return IsLoadedBit::decode(raw_ptr()->state_bits_); }
+  bool is_loaded() const { return IsLoadedBit::decode(state_bits()); }
   void set_is_loaded(bool value) const;
 
   uint16_t num_native_fields() const { return raw_ptr()->num_native_fields_; }
@@ -1730,15 +1731,23 @@ class Class : public Object {
 
   int16_t num_type_arguments() const { return raw_ptr()->num_type_arguments_; }
 
+  uint32_t state_bits() const {
+    // Ensure any following load instructions do not get performed before this
+    // one.
+    return LoadNonPointer<uint32_t, std::memory_order_acquire>(
+        &raw_ptr()->state_bits_);
+  }
+
  public:
   void set_num_type_arguments(intptr_t value) const;
 
-  bool has_pragma() const {
-    return HasPragmaBit::decode(raw_ptr()->state_bits_);
-  }
+  bool has_pragma() const { return HasPragmaBit::decode(state_bits()); }
   void set_has_pragma(bool has_pragma) const;
 
  private:
+  void set_functions(const Array& value) const;
+  void set_fields(const Array& value) const;
+
   // Calculates number of type arguments of this class.
   // This includes type arguments of a superclass and takes overlapping
   // of type arguments into account.