From b7b79ad835238ece4e8b4ba0b561b9f71f04e35d Mon Sep 17 00:00:00 2001 From: Corentin Pescheloche Date: Tue, 22 Feb 2022 23:16:43 -0800 Subject: [PATCH] [profiler] Use FilterContext to filter VMState in Samples To avoid leaking VMState cross origin leverage existing FilterContext to filter out VMSTates. GC State is the exception as it is not coupled to any native context and is always included. Bug: chromium:1263871 Change-Id: I5cab8620460f4db24fa183c891cb0c43996e95c4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3465735 Auto-Submit: Corentin Pescheloche Reviewed-by: Camillo Bruni Commit-Queue: Camillo Bruni Cr-Commit-Position: refs/heads/main@{#79234} --- src/profiler/profile-generator.cc | 7 +++++++ src/profiler/tick-sample.cc | 14 +++++++------- test/cctest/test-cpu-profiler.cc | 9 +++++++++ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/profiler/profile-generator.cc b/src/profiler/profile-generator.cc index d01b581633..fc7e080f37 100644 --- a/src/profiler/profile-generator.cc +++ b/src/profiler/profile-generator.cc @@ -1005,6 +1005,13 @@ void CpuProfilesCollection::AddPathToCurrentProfiles( bool accepts_context = context_filter.Accept(native_context_address); bool accepts_embedder_context = context_filter.Accept(embedder_native_context_address); + + // if FilterContext is set, do not propagate StateTag if not accepted. + // GC is exception because native context address is guaranteed to be empty. + DCHECK(state != StateTag::GC || native_context_address == kNullAddress); + if (!accepts_context && state != StateTag::GC) { + state = StateTag::IDLE; + } profile->AddPath(timestamp, accepts_context ? path : empty_path, src_line, update_stats, sampling_interval, state, accepts_embedder_context ? embedder_state_tag diff --git a/src/profiler/tick-sample.cc b/src/profiler/tick-sample.cc index 7f53bea012..cd65f9d651 100644 --- a/src/profiler/tick-sample.cc +++ b/src/profiler/tick-sample.cc @@ -226,6 +226,13 @@ bool TickSample::GetStackSample(Isolate* v8_isolate, RegisterState* regs, sample_info->embedder_state = embedder_state->GetState(); } + Context top_context = isolate->context(); + if (top_context.ptr() != i::Context::kNoContext && + top_context.ptr() != i::Context::kInvalidContext) { + NativeContext top_native_context = top_context.native_context(); + sample_info->context = reinterpret_cast(top_native_context.ptr()); + } + i::Address js_entry_sp = isolate->js_entry_sp(); if (js_entry_sp == 0) return true; // Not executing JS now. @@ -293,13 +300,6 @@ bool TickSample::GetStackSample(Isolate* v8_isolate, RegisterState* regs, reinterpret_cast(regs->lr), js_entry_sp); - Context top_context = isolate->context(); - if (top_context.ptr() != i::Context::kNoContext && - top_context.ptr() != i::Context::kInvalidContext) { - NativeContext top_native_context = top_context.native_context(); - sample_info->context = reinterpret_cast(top_native_context.ptr()); - } - if (it.done()) return true; size_t i = 0; diff --git a/test/cctest/test-cpu-profiler.cc b/test/cctest/test-cpu-profiler.cc index 0779b770f0..8fdc86c2c1 100644 --- a/test/cctest/test-cpu-profiler.cc +++ b/test/cctest/test-cpu-profiler.cc @@ -3920,6 +3920,15 @@ TEST(ContextIsolation) { diff_context_profile->GetTopDownRoot(); // Ensure that no children were recorded (including callbacks, builtins). CHECK(!FindChild(diff_root, "start")); + + CHECK_GT(diff_context_profile->GetSamplesCount(), 0); + for (int i = 0; i < diff_context_profile->GetSamplesCount(); i++) { + CHECK(diff_context_profile->GetSampleState(i) == StateTag::IDLE || + // GC State do not have a context + diff_context_profile->GetSampleState(i) == StateTag::GC || + // first frame and native code reports as external + diff_context_profile->GetSampleState(i) == StateTag::EXTERNAL); + } } }