Commit 956d32e3 authored by Dominik Inführ's avatar Dominik Inführ Committed by V8 LUCI CQ

[heap] Fix accounting for left-trimmed arrays

ShouldVisit() uses obj.Size() to increment the live bytes counter after
the object was successfully marked grey. However, this re-reads the
length field which could have already been overwritten by a
concurrent left-trimming operation on the main thread. Fix this by
calculating the object size later with the length field we read before
marking that object black. That value is guaranteed to be a SMI.

Bug: chromium:1273352
Change-Id: I47e5a2df3eef61b4ef07af943f30123e5c2f7f9d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3302793
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78093}
parent 88c9b832
......@@ -169,6 +169,10 @@ class ConcurrentMarkingVisitor final
return marking_state_.GreyToBlack(object);
}
bool ShouldVisitUnaccounted(HeapObject object) {
return marking_state_.GreyToBlackUnaccounted(object);
}
private:
// Helper class for collecting in-object slot addresses and values.
class SlotSnapshottingVisitor final : public ObjectVisitorWithCageBases {
......@@ -248,11 +252,15 @@ class ConcurrentMarkingVisitor final
// The length() function checks that the length is a Smi.
// This is not necessarily the case if the array is being left-trimmed.
Object length = object.unchecked_length(kAcquireLoad);
if (!ShouldVisit(object)) return 0;
// No accounting here to avoid re-reading the length which could already
// contain a non-SMI value when left-trimming happens concurrently.
if (!ShouldVisitUnaccounted(object)) return 0;
// The cached length must be the actual length as the array is not black.
// Left trimming marks the array black before over-writing the length.
DCHECK(length.IsSmi());
int size = T::SizeFor(Smi::ToInt(length));
marking_state_.IncrementLiveBytes(MemoryChunk::FromHeapObject(object),
size);
VisitMapPointer(object);
T::BodyDescriptor::IterateBody(map, object, size, this);
return size;
......
......@@ -77,6 +77,10 @@ class MarkingStateBase {
return true;
}
V8_INLINE bool GreyToBlackUnaccounted(HeapObject obj) {
return Marking::GreyToBlack<access_mode>(MarkBitFrom(obj));
}
void ClearLiveness(MemoryChunk* chunk) {
static_cast<ConcreteState*>(this)->bitmap(chunk)->Clear();
static_cast<ConcreteState*>(this)->SetLiveBytes(chunk, 0);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment