Commit 1ccf7663 authored by Dominik Inführ's avatar Dominik Inführ Committed by V8 LUCI CQ

[heap] Verify usages of SKIP_WRITE_BARRIER

Verify usages of SKIP_WRITE_BARRIER in builds with SLOW_DCHECKs enabled.
We can only remove the write barrier in specific circumstances that
can also be DCHECK'ed.

I also switched some write barriers to UPDATE_WRITE_BARRIER where those
simple rules didn't hold but relied on more elaborate explanations.

Bug: v8:12544
Change-Id: I4caa43627f8a3209d853e3352caabc161568e6eb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3386803Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78649}
parent cbcedb04
......@@ -283,6 +283,19 @@ void WriteBarrier::MarkingFromInternalFields(JSObject host) {
MarkingSlowFromInternalFields(*heap, host);
}
#ifdef ENABLE_SLOW_DCHECKS
// static
template <typename T>
bool WriteBarrier::IsRequired(HeapObject host, T value) {
if (BasicMemoryChunk::FromHeapObject(host)->InYoungGeneration()) return false;
if (value.IsSmi()) return false;
if (value.IsCleared()) return false;
HeapObject target = value.GetHeapObject();
if (ReadOnlyHeap::Contains(target)) return false;
return !IsImmortalImmovableHeapObject(target);
}
#endif
} // namespace internal
} // namespace v8
......
......@@ -7,6 +7,7 @@
#include "src/heap/embedder-tracing.h"
#include "src/heap/heap-write-barrier-inl.h"
#include "src/heap/marking-barrier.h"
#include "src/objects/code-inl.h"
#include "src/objects/descriptor-array.h"
#include "src/objects/js-objects.h"
#include "src/objects/maybe-object.h"
......@@ -96,5 +97,21 @@ int WriteBarrier::MarkingFromCode(Address raw_host, Address raw_slot) {
return 0;
}
#ifdef ENABLE_SLOW_DCHECKS
bool WriteBarrier::IsImmortalImmovableHeapObject(HeapObject object) {
BasicMemoryChunk* basic_chunk = BasicMemoryChunk::FromHeapObject(object);
// All objects in readonly space are immortal and immovable.
if (basic_chunk->InReadOnlySpace()) return true;
MemoryChunk* chunk = MemoryChunk::FromHeapObject(object);
// There are also objects in "regular" spaces which are immortal and
// immovable. Objects on a page that can get compacted are movable and can be
// filtered out.
if (!chunk->IsFlagSet(MemoryChunk::NEVER_EVACUATE)) return false;
// Now we know the object is immovable, check whether it is also immortal.
// Builtins are roots and therefore always kept alive by the GC.
return object.IsCode() && Code::cast(object).is_builtin();
}
#endif
} // namespace internal
} // namespace v8
......@@ -65,6 +65,12 @@ class V8_EXPORT_PRIVATE WriteBarrier {
static MarkingBarrier* CurrentMarkingBarrier(Heap* heap);
#ifdef ENABLE_SLOW_DCHECKS
template <typename T>
static inline bool IsRequired(HeapObject host, T value);
static bool IsImmortalImmovableHeapObject(HeapObject object);
#endif
private:
static inline base::Optional<Heap*> GetHeapIfMarking(HeapObject object);
......
......@@ -5671,9 +5671,9 @@ void Genesis::InitializeMapCaches() {
DisallowGarbageCollection no_gc;
native_context()->set_map_cache(*cache);
Map initial = native_context()->object_function().initial_map();
cache->Set(0, HeapObjectReference::Weak(initial), SKIP_WRITE_BARRIER);
cache->Set(0, HeapObjectReference::Weak(initial));
cache->Set(initial.GetInObjectProperties(),
HeapObjectReference::Weak(initial), SKIP_WRITE_BARRIER);
HeapObjectReference::Weak(initial));
}
}
......
......@@ -1009,8 +1009,9 @@ void FeedbackNexus::SetSpeculationMode(SpeculationMode mode) {
uint32_t count = static_cast<uint32_t>(Smi::ToInt(call_count));
count = SpeculationModeField::update(count, mode);
MaybeObject feedback = GetFeedback();
// We can skip the write barrier for {feedback} because it's not changing.
SetFeedback(feedback, SKIP_WRITE_BARRIER, Smi::FromInt(count),
// We could've skipped WB here (since we set the slot to the same value again)
// but we don't to make WB verification happy.
SetFeedback(feedback, UPDATE_WRITE_BARRIER, Smi::FromInt(count),
SKIP_WRITE_BARRIER);
}
......
......@@ -484,6 +484,8 @@
WriteBarrier::Marking(object, (object).RawField(offset), value); \
} \
GenerationalBarrier(object, (object).RawField(offset), value); \
} else { \
SLOW_DCHECK(!WriteBarrier::IsRequired(object, value)); \
} \
} while (false)
#endif
......@@ -504,6 +506,8 @@
value); \
} \
GenerationalBarrier(object, (object).RawMaybeWeakField(offset), value); \
} else { \
SLOW_DCHECK(!WriteBarrier::IsRequired(object, value)); \
} \
} while (false)
#endif
......@@ -522,6 +526,8 @@
} \
GenerationalEphemeronKeyBarrier(table, (object).RawField(offset), \
value); \
} else { \
SLOW_DCHECK(!WriteBarrier::IsRequired(object, value)); \
} \
} while (false)
#endif
......
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