From 4bbf058d53e35c50771b361c981431324486925d Mon Sep 17 00:00:00 2001 From: "mikhail.naganov@gmail.com" Date: Tue, 10 Aug 2010 12:06:42 +0000 Subject: [PATCH] Fix CPU profiler crash in start / stop sequence when non-existent name is passed BUG=51594 TEST=test-cpu-profiler/CrashIfStoppingLastNonExistentProfile Review URL: http://codereview.chromium.org/3108004 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5230 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/cpu-profiler.cc | 11 ++++++----- src/cpu-profiler.h | 2 +- src/profile-generator-inl.h | 7 ------- src/profile-generator.cc | 16 +++++++++------- src/profile-generator.h | 5 +---- test/cctest/test-cpu-profiler.cc | 15 +++++++++++++++ 6 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/cpu-profiler.cc b/src/cpu-profiler.cc index c8d29f8..3e554cc 100644 --- a/src/cpu-profiler.cc +++ b/src/cpu-profiler.cc @@ -476,7 +476,7 @@ void CpuProfiler::StartProcessorIfNotStarted() { CpuProfile* CpuProfiler::StopCollectingProfile(const char* title) { const double actual_sampling_rate = generator_->actual_sampling_rate(); - StopProcessorIfLastProfile(); + StopProcessorIfLastProfile(title); CpuProfile* result = profiles_->StopProfiling(TokenEnumerator::kNoSecurityToken, title, @@ -491,14 +491,15 @@ CpuProfile* CpuProfiler::StopCollectingProfile(const char* title) { CpuProfile* CpuProfiler::StopCollectingProfile(Object* security_token, String* title) { const double actual_sampling_rate = generator_->actual_sampling_rate(); - StopProcessorIfLastProfile(); + const char* profile_title = profiles_->GetName(title); + StopProcessorIfLastProfile(profile_title); int token = token_enumerator_->GetTokenId(security_token); - return profiles_->StopProfiling(token, title, actual_sampling_rate); + return profiles_->StopProfiling(token, profile_title, actual_sampling_rate); } -void CpuProfiler::StopProcessorIfLastProfile() { - if (profiles_->is_last_profile()) { +void CpuProfiler::StopProcessorIfLastProfile(const char* title) { + if (profiles_->IsLastProfile(title)) { reinterpret_cast(Logger::ticker_)->Stop(); processor_->Stop(); processor_->Join(); diff --git a/src/cpu-profiler.h b/src/cpu-profiler.h index 03b8176..4d5559e 100644 --- a/src/cpu-profiler.h +++ b/src/cpu-profiler.h @@ -260,7 +260,7 @@ class CpuProfiler { void StartProcessorIfNotStarted(); CpuProfile* StopCollectingProfile(const char* title); CpuProfile* StopCollectingProfile(Object* security_token, String* title); - void StopProcessorIfLastProfile(); + void StopProcessorIfLastProfile(const char* title); CpuProfilesCollection* profiles_; unsigned next_profile_uid_; diff --git a/src/profile-generator-inl.h b/src/profile-generator-inl.h index fecb70b..0c50581 100644 --- a/src/profile-generator-inl.h +++ b/src/profile-generator-inl.h @@ -97,13 +97,6 @@ void CodeMap::DeleteCode(Address addr) { } -bool CpuProfilesCollection::is_last_profile() { - // Called from VM thread, and only it can mutate the list, - // so no locking is needed here. - return current_profiles_.length() == 1; -} - - const char* CpuProfilesCollection::GetFunctionName(String* name) { return GetFunctionName(GetName(name)); } diff --git a/src/profile-generator.cc b/src/profile-generator.cc index 8316079..1d1aa81 100644 --- a/src/profile-generator.cc +++ b/src/profile-generator.cc @@ -542,13 +542,6 @@ CpuProfile* CpuProfilesCollection::StopProfiling(int security_token_id, } -CpuProfile* CpuProfilesCollection::StopProfiling(int security_token_id, - String* title, - double actual_sampling_rate) { - return StopProfiling(security_token_id, GetName(title), actual_sampling_rate); -} - - CpuProfile* CpuProfilesCollection::GetProfile(int security_token_id, unsigned uid) { HashMap::Entry* entry = profiles_uids_.Lookup(reinterpret_cast(uid), @@ -574,6 +567,15 @@ CpuProfile* CpuProfilesCollection::GetProfile(int security_token_id, } +bool CpuProfilesCollection::IsLastProfile(const char* title) { + // Called from VM thread, and only it can mutate the list, + // so no locking is needed here. + if (current_profiles_.length() != 1) return false; + return StrLength(title) == 0 + || strcmp(current_profiles_[0]->title(), title) == 0; +} + + int CpuProfilesCollection::TokenToIndex(int security_token_id) { ASSERT(TokenEnumerator::kNoSecurityToken == -1); return security_token_id + 1; // kNoSecurityToken -> 0, 0 -> 1, ... diff --git a/src/profile-generator.h b/src/profile-generator.h index 4c5eb3f..4936f8f 100644 --- a/src/profile-generator.h +++ b/src/profile-generator.h @@ -279,15 +279,12 @@ class CpuProfilesCollection { CpuProfile* StopProfiling(int security_token_id, const char* title, double actual_sampling_rate); - CpuProfile* StopProfiling(int security_token_id, - String* title, - double actual_sampling_rate); List* Profiles(int security_token_id); const char* GetName(String* name) { return function_and_resource_names_.GetName(name); } CpuProfile* GetProfile(int security_token_id, unsigned uid); - inline bool is_last_profile(); + bool IsLastProfile(const char* title); CodeEntry* NewCodeEntry(Logger::LogEventsAndTags tag, String* name, String* resource_name, int line_number); diff --git a/test/cctest/test-cpu-profiler.cc b/test/cctest/test-cpu-profiler.cc index 0e6f09d..239d8ae 100644 --- a/test/cctest/test-cpu-profiler.cc +++ b/test/cctest/test-cpu-profiler.cc @@ -12,6 +12,7 @@ namespace i = v8::internal; using i::CodeEntry; using i::CpuProfile; +using i::CpuProfiler; using i::CpuProfilesCollection; using i::ProfileGenerator; using i::ProfileNode; @@ -225,4 +226,18 @@ TEST(TickEvents) { CHECK_EQ("bbb", bottom_up_ddd_stub_children->last()->entry()->name()); } + +// http://crbug/51594 +// This test must not crash. +TEST(CrashIfStoppingLastNonExistentProfile) { + InitializeVM(); + TestSetup test_setup; + CpuProfiler::Setup(); + CpuProfiler::StartProfiling("1"); + CpuProfiler::StopProfiling("2"); + CpuProfiler::StartProfiling("1"); + CpuProfiler::StopProfiling(""); + CpuProfiler::TearDown(); +} + #endif // ENABLE_LOGGING_AND_PROFILING -- 2.7.4