Fix CPU profiler crash in start / stop sequence when non-existent name is passed
authormikhail.naganov@gmail.com <mikhail.naganov@gmail.com@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Tue, 10 Aug 2010 12:06:42 +0000 (12:06 +0000)
committermikhail.naganov@gmail.com <mikhail.naganov@gmail.com@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Tue, 10 Aug 2010 12:06:42 +0000 (12:06 +0000)
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
src/cpu-profiler.h
src/profile-generator-inl.h
src/profile-generator.cc
src/profile-generator.h
test/cctest/test-cpu-profiler.cc

index c8d29f8..3e554cc 100644 (file)
@@ -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();
index 03b8176..4d5559e 100644 (file)
@@ -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_;
index fecb70b..0c50581 100644 (file)
@@ -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));
 }
index 8316079..1d1aa81 100644 (file)
@@ -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, ...
index 4c5eb3f..4936f8f 100644 (file)
@@ -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);
index 0e6f09d..239d8ae 100644 (file)
@@ -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