[vm] Adjust array size consistency check.

The previous check can fail if marker reads the old tags, then the mutator truncates the array, then the marker reads the length field. Rereading the length in the marker won't lead to consistency, and rereading the tags is not permitted when running in the scavenger.

Also avoid a compiler bug where the compare_exchange_weak desired value was not reloaded on failure, resulting in the CAS infinitely looping after a failure.

TEST=ci
Bug: https://github.com/dart-lang/sdk/issues/62373
Change-Id: If1bfeeef648a909509558401b25ce5085d5f1727
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/475924
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>

runtime/vm/bitfield.h

@@ -66,14 +66,14 @@ class AtomicBitFieldContainer {
     field_.fetch_or(TargetBitField::encode(value), std::memory_order_relaxed);
   }
 
-  template <class TargetBitField>
+  template <class TargetBitField,
+            std::memory_order order = std::memory_order_relaxed>
   void Update(typename TargetBitField::Type value) {
     T old_field = field_.load(std::memory_order_relaxed);
     T new_field;
     do {
       new_field = TargetBitField::update(value, old_field);
-    } while (!field_.compare_exchange_weak(old_field, new_field,
-                                           std::memory_order_relaxed));
+    } while (!field_.compare_exchange_weak(old_field, new_field, order));
   }
 
   template <class TargetBitField>

runtime/vm/object.cc

@@ -25845,7 +25845,7 @@ void Array::Truncate(intptr_t new_len) const {
   intptr_t old_size = Array::InstanceSize(old_len);
   intptr_t new_size = Array::InstanceSize(new_len);
 
-  NoSafepointScope no_safepoint;
+  NoSafepointScope no_safepoint(thread);
 
   // If there is any left over space fill it with either an Array object or
   // just a plain object (depending on the amount of left over space) so
@@ -25855,13 +25855,9 @@ void Array::Truncate(intptr_t new_len) const {
   // Update the size in the header field and length of the array object.
   // These release operations are balanced by acquire operations in the
   // concurrent sweeper.
-  uword old_tags = array.untag()->tags_;
-  uword new_tags;
-  ASSERT(kArrayCid == UntaggedObject::ClassIdTag::decode(old_tags));
-  do {
-    new_tags = UntaggedObject::SizeTag::update(new_size, old_tags);
-  } while (!array.untag()->tags_.compare_exchange_weak(
-      old_tags, new_tags, std::memory_order_release));
+  array.untag()
+      ->tags_.Update<UntaggedObject::SizeTag, std::memory_order_release>(
+          new_size);
 
   // Between the CAS of the header above and the SetLength below, the array is
   // temporarily in an inconsistent state. The header is considered the

runtime/vm/raw_object.cc

@@ -255,21 +255,24 @@ intptr_t UntaggedObject::HeapSizeFromClass(uword tags) const {
   ASSERT(instance_size != 0);
 #if defined(DEBUG)
   intptr_t tags_size = SizeTag::decode(tags);
-  if ((class_id == kArrayCid) && (instance_size > tags_size && tags_size > 0)) {
-    // TODO(22501): Array::MakeFixedLength could be in the process of shrinking
-    // the array (see comment therein), having already updated the tags but not
-    // yet set the new length. Wait a millisecond and try again.
-    int retries_remaining = 1000;  // ... but not forever.
-    do {
-      OS::Sleep(1);
-      const ArrayPtr raw_array = static_cast<const ArrayPtr>(this);
-      intptr_t array_length = Smi::Value(raw_array->untag()->length());
-      instance_size = Array::InstanceSize(array_length);
-    } while ((instance_size > tags_size) && (--retries_remaining > 0));
-  }
   if ((instance_size != tags_size) && (tags_size != 0)) {
-    FATAL("Size mismatch: %" Pd " from class vs %" Pd " from tags %" Px "\n",
-          instance_size, tags_size, tags);
+    // Array::Truncate could be in the process of shrinking the array.
+    // Unfortunately, we cannot do a sanity check by reload tags here expecting
+    // to eventually see consistent sizes because another scavenge worker may
+    // have already replaced the tags with a forwarding pointer. But the
+    // truncation should be ensuring the heap remains iterable no matter what
+    // intermediate state we see, so instead check there appears to be a filler
+    // covering the gap between the two sizes.
+    if (class_id == kArrayCid) {
+      intptr_t smaller_size = Utils::Minimum(tags_size, instance_size);
+      intptr_t larger_size = Utils::Maximum(tags_size, instance_size);
+      ObjectPtr filler = FromAddr(ToAddr(this) + smaller_size);
+      intptr_t filler_size = filler->untag()->HeapSize();
+      ASSERT(smaller_size + filler_size == larger_size);
+    } else {
+      FATAL("Size mismatch: %" Pd " from class vs %" Pd " from tags %" Px "\n",
+            instance_size, tags_size, tags);
+    }
   }
 #endif  // DEBUG
   return instance_size;