Commit 4c7cabb1 authored by Michael Lippautz's avatar Michael Lippautz Committed by Commit Bot

[heap] Delay embedder tracing prologue until heap is set up

v8::EmbedderHeapTracer::TracePrologue may call back into V8 during
StartMarking. In this case we expect that the write barriers are set up and
consistent, i.e., global flag matches page flag.

Blink calls back into V8 in a corner case where sweeping is finalized on
incremental marking start which may trigger resettting a V8 Value which may
trigger DescriptorArray re-shuffling.

Bug: chromium:940003
Change-Id: Ia15c798d0faaab802df1c3b569b5b6a323a4fe59
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1514492Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Reviewed-by: 's avatarHannes Payer <hpayer@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60145}
parent 93d1508d
......@@ -355,12 +355,6 @@ void IncrementalMarking::StartMarking() {
SetState(MARKING);
{
TRACE_GC(heap()->tracer(),
GCTracer::Scope::MC_INCREMENTAL_EMBEDDER_PROLOGUE);
heap_->local_embedder_heap_tracer()->TracePrologue();
}
ActivateIncrementalWriteBarrier();
// Marking bits are cleared by the sweeper.
......@@ -386,6 +380,14 @@ void IncrementalMarking::StartMarking() {
if (FLAG_trace_incremental_marking) {
heap()->isolate()->PrintWithTimestamp("[IncrementalMarking] Running\n");
}
{
// TracePrologue may call back into V8 in corner cases, requiring that
// marking (including write barriers) is fully set up.
TRACE_GC(heap()->tracer(),
GCTracer::Scope::MC_INCREMENTAL_EMBEDDER_PROLOGUE);
heap_->local_embedder_heap_tracer()->TracePrologue();
}
}
void IncrementalMarking::StartBlackAllocation() {
......
......@@ -40,9 +40,14 @@ v8::Local<v8::Object> ConstructTraceableJSApiObject(
return scope.Escape(instance);
}
enum class TracePrologueBehavior { kNoop, kCallV8WriteBarrier };
class TestEmbedderHeapTracer final : public v8::EmbedderHeapTracer {
public:
TestEmbedderHeapTracer() = default;
TestEmbedderHeapTracer(TracePrologueBehavior prologue_behavior,
v8::Global<v8::Array> array)
: prologue_behavior_(prologue_behavior), array_(std::move(array)) {}
void RegisterV8References(
const std::vector<std::pair<void*, void*>>& embedder_fields) final {
......@@ -64,7 +69,14 @@ class TestEmbedderHeapTracer final : public v8::EmbedderHeapTracer {
bool IsTracingDone() final { return to_register_with_v8_.empty(); }
void TracePrologue() final {}
void TracePrologue() final {
if (prologue_behavior_ == TracePrologueBehavior::kCallV8WriteBarrier) {
auto local = array_.Get(isolate());
local->Set(local->CreationContext(), 0, v8::Object::New(isolate()))
.Check();
}
}
void TraceEpilogue() final {}
void EnterFinalPause(EmbedderStackState) final {}
......@@ -87,6 +99,8 @@ class TestEmbedderHeapTracer final : public v8::EmbedderHeapTracer {
std::vector<std::pair<void*, void*>> registered_from_v8_;
std::vector<v8::TracedGlobal<v8::Object>*> to_register_with_v8_;
bool consider_traced_global_as_root_ = true;
TracePrologueBehavior prologue_behavior_ = TracePrologueBehavior::kNoop;
v8::Global<v8::Array> array_;
};
class TemporaryEmbedderHeapTracerScope {
......@@ -546,6 +560,24 @@ TEST(TracedGlobalSetFinalizationCallbackMarkSweep) {
CHECK(traced.IsEmpty());
}
TEST(TracePrologueCallingIntoV8WriteBarrier) {
// Regression test: https://crbug.com/940003
ManualGCScope manual_gc;
CcTest::InitializeVM();
v8::Isolate* isolate = CcTest::isolate();
v8::HandleScope scope(isolate);
v8::Global<v8::Array> global;
{
v8::HandleScope scope(isolate);
auto local = v8::Array::New(isolate, 10);
global.Reset(isolate, local);
}
TestEmbedderHeapTracer tracer(TracePrologueBehavior::kCallV8WriteBarrier,
std::move(global));
TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer);
SimulateIncrementalMarking(CcTest::i_isolate()->heap());
}
} // namespace heap
} // namespace internal
} // namespace v8
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