From 72f3124d90df69fb9d4c92e05e4be951bd20c0e2 Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Wed, 28 Jan 2026 10:27:29 -0800 Subject: [PATCH] [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 Commit-Queue: Ryan Macnak --- runtime/vm/bitfield.h | 6 +++--- runtime/vm/object.cc | 12 ++++-------- runtime/vm/raw_object.cc | 31 +++++++++++++++++-------------- 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/runtime/vm/bitfield.h b/runtime/vm/bitfield.h index 73af9dcc1fc..2a5b593b786 100644 --- a/runtime/vm/bitfield.h +++ b/runtime/vm/bitfield.h @@ -66,14 +66,14 @@ class AtomicBitFieldContainer { field_.fetch_or(TargetBitField::encode(value), std::memory_order_relaxed); } - template + template 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 diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index 2be2318f799..e235f5a8f6b 100644 --- a/runtime/vm/object.cc +++ b/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( + 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 diff --git a/runtime/vm/raw_object.cc b/runtime/vm/raw_object.cc index cb3cffb861d..a6eaa8f3c05 100644 --- a/runtime/vm/raw_object.cc +++ b/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(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;