Commit b4b33a36 authored by Michael Lippautz's avatar Michael Lippautz Committed by Commit Bot

heap,wtf: Fix weakness processing of floating garbage

The marking verifier assumes that any marked backing stores contains
pointers to only live objects.

In case a backing is removed after the table has been added for later
processing, buckets were not cleared if they were trivially
destructible. The fix clears weak pointers when marking is on as well.

Bug: 1054363
Change-Id: If082a518d39b97d1d04661e784ac8b5fbb05c069
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2066918Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarOmer Katz <omerkatz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743473}
parent d7897335
...@@ -175,6 +175,11 @@ class PLATFORM_EXPORT HeapAllocator { ...@@ -175,6 +175,11 @@ class PLATFORM_EXPORT HeapAllocator {
return ThreadState::Current()->SweepForbidden(); return ThreadState::Current()->SweepForbidden();
} }
static bool IsIncrementalMarking() {
return ThreadState::IsAnyIncrementalMarking() &&
ThreadState::Current()->IsIncrementalMarking();
}
template <typename T> template <typename T>
static bool IsHeapObjectAlive(T* object) { static bool IsHeapObjectAlive(T* object) {
return ThreadHeap::IsHeapObjectAlive(object); return ThreadHeap::IsHeapObjectAlive(object);
......
...@@ -223,6 +223,23 @@ TEST_F(WeaknessMarkingTest, EmptyEphemeronCollection) { ...@@ -223,6 +223,23 @@ TEST_F(WeaknessMarkingTest, EmptyEphemeronCollection) {
TestSupportingGC::PreciselyCollectGarbage(); TestSupportingGC::PreciselyCollectGarbage();
} }
TEST_F(WeaknessMarkingTest, ClearWeakHashTableAfterMarking) {
// Regression test: https://crbug.com/1054363
//
// Test ensures that no marked backing with weak pointers to dead object is
// left behind after marking. The test creates a backing that is floating
// garbage. The marking verifier ensures that all buckets are properly
// deleted.
using Set = HeapHashSet<WeakMember<IntegerObject>>;
Persistent<Set> holder(MakeGarbageCollected<Set>());
holder->insert(MakeGarbageCollected<IntegerObject>(1));
IncrementalMarkingTestDriver driver(ThreadState::Current());
driver.Start();
driver.FinishSteps();
holder->clear();
driver.FinishGC();
}
} // namespace weakness_marking_test } // namespace weakness_marking_test
} // namespace blink } // namespace blink
...@@ -85,6 +85,7 @@ class WTF_EXPORT PartitionAllocator { ...@@ -85,6 +85,7 @@ class WTF_EXPORT PartitionAllocator {
static bool IsAllocationAllowed() { return true; } static bool IsAllocationAllowed() { return true; }
static bool IsObjectResurrectionForbidden() { return false; } static bool IsObjectResurrectionForbidden() { return false; }
static bool IsSweepForbidden() { return false; } static bool IsSweepForbidden() { return false; }
static bool IsIncrementalMarking() { return false; }
static void EnterGCForbiddenScope() {} static void EnterGCForbiddenScope() {}
static void LeaveGCForbiddenScope() {} static void LeaveGCForbiddenScope() {}
......
...@@ -1680,7 +1680,17 @@ void HashTable<Key, ...@@ -1680,7 +1680,17 @@ void HashTable<Key,
KeyTraits, KeyTraits,
Allocator>::DeleteAllBucketsAndDeallocate(ValueType* table, Allocator>::DeleteAllBucketsAndDeallocate(ValueType* table,
unsigned size) { unsigned size) {
if (!std::is_trivially_destructible<ValueType>::value) { // We delete a bucket in the following cases:
// - It is not trivially destructible.
// - The table is weak (thus garbage collected) and we are currently marking.
// This is to handle the case where a backing store is removed from the
// HashTable after HashTable has been enqueued for processing. If we remove
// the backing in that case it stays unprocessed which upsets the marking
// verifier that checks that all backings are in consistent state.
const bool needs_bucket_deletion =
!std::is_trivially_destructible<ValueType>::value ||
(WTF::IsWeak<ValueType>::value && Allocator::IsIncrementalMarking());
if (needs_bucket_deletion) {
for (unsigned i = 0; i < size; ++i) { for (unsigned i = 0; i < size; ++i) {
// This code is called when the hash table is cleared or resized. We // This code is called when the hash table is cleared or resized. We
// have allocated a new backing store and we need to run the // have allocated a new backing store and we need to run the
......
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