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

diff --git a/src/cpu-profiler.cc b/src/cpu-profiler.cc
index c8d29f8cf5747907e924fe3dd3643e324438024c..3e554ccebdc9faa9190313feb7374e78312ffb1f 100644 (file)
--- 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<Sampler*>(Logger::ticker_)->Stop();
     processor_->Stop();
     processor_->Join();

diff --git a/src/cpu-profiler.h b/src/cpu-profiler.h
index 03b81764851cea6881e33a4d42cdbd130ecaab51..4d5559e4febdc9de02a7ca105e007347a6686170 100644 (file)
--- 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 fecb70b7753347686ed35ab86ef71e9a571db4a6..0c50581ab795ef2c3793a98f9c3b2c3324461a09 100644 (file)
--- 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 8316079a8d09ab427056e15e6ac7d04696182fae..1d1aa812bc09e32ab00fe37f4086250ca1dc055d 100644 (file)
--- 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<void*>(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 4c5eb3f1587c8e3ae04676bacb70df84c0465c7c..4936f8f3831160e30fb9c3994ef47d096af4e8ce 100644 (file)
--- 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<CpuProfile*>* 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 0e6f09d2ef0159c6aff2b0d8cebf0fc5e8c79fb8..239d8ae695106745f108324d914e09c2d5917a40 100644 (file)
--- 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