Revision: 17260
Author:   [email protected]
Date:     Fri Oct 18 08:56:14 2013 UTC
Log:      Always make a copy of a string when adding it to StringsStorage

Otherwise the string passed as const char* may be disposed and we will end up with a dangling pointer.

Also changed StringsStorage::GetCopy so that a copy is not created if the string is already in the cache.

BUG=None
[email protected], [email protected]

Review URL: https://codereview.chromium.org/27627006
http://code.google.com/p/v8/source/detail?r=17260

Modified:
 /branches/bleeding_edge/src/profile-generator-inl.h
 /branches/bleeding_edge/src/profile-generator.cc
 /branches/bleeding_edge/src/profile-generator.h

=======================================
--- /branches/bleeding_edge/src/profile-generator-inl.h Mon Oct 14 08:57:46 2013 UTC +++ /branches/bleeding_edge/src/profile-generator-inl.h Fri Oct 18 08:56:14 2013 UTC
@@ -33,16 +33,6 @@
 namespace v8 {
 namespace internal {

-const char* StringsStorage::GetFunctionName(Name* name) {
-  return GetFunctionName(GetName(name));
-}
-
-
-const char* StringsStorage::GetFunctionName(const char* name) {
- return strlen(name) > 0 ? name : ProfileGenerator::kAnonymousFunctionName;
-}
-
-
 CodeEntry::CodeEntry(Logger::LogEventsAndTags tag,
                      const char* name,
                      const char* name_prefix,
=======================================
--- /branches/bleeding_edge/src/profile-generator.cc Mon Oct 14 08:57:46 2013 UTC +++ /branches/bleeding_edge/src/profile-generator.cc Fri Oct 18 08:56:14 2013 UTC
@@ -41,6 +41,12 @@
 namespace internal {


+bool StringsStorage::StringsMatch(void* key1, void* key2) {
+  return strcmp(reinterpret_cast<char*>(key1),
+                reinterpret_cast<char*>(key2)) == 0;
+}
+
+
 StringsStorage::StringsStorage(Heap* heap)
     : hash_seed_(heap->HashSeed()), names_(StringsMatch) {
 }
@@ -57,12 +63,15 @@

 const char* StringsStorage::GetCopy(const char* src) {
   int len = static_cast<int>(strlen(src));
-  Vector<char> dst = Vector<char>::New(len + 1);
-  OS::StrNCpy(dst, src, len);
-  dst[len] = '\0';
-  uint32_t hash =
-      StringHasher::HashSequentialString(dst.start(), len, hash_seed_);
-  return AddOrDisposeString(dst.start(), hash);
+  HashMap::Entry* entry = GetEntry(src, len);
+  if (entry->value == NULL) {
+    Vector<char> dst = Vector<char>::New(len + 1);
+    OS::StrNCpy(dst, src, len);
+    dst[len] = '\0';
+    entry->key = dst.start();
+    entry->value = entry->key;
+  }
+  return reinterpret_cast<const char*>(entry->value);
 }


@@ -75,15 +84,16 @@
 }


-const char* StringsStorage::AddOrDisposeString(char* str, uint32_t hash) {
-  HashMap::Entry* cache_entry = names_.Lookup(str, hash, true);
-  if (cache_entry->value == NULL) {
+const char* StringsStorage::AddOrDisposeString(char* str, int len) {
+  HashMap::Entry* entry = GetEntry(str, len);
+  if (entry->value == NULL) {
     // New entry added.
-    cache_entry->value = str;
+    entry->key = str;
+    entry->value = str;
   } else {
     DeleteArray(str);
   }
-  return reinterpret_cast<const char*>(cache_entry->value);
+  return reinterpret_cast<const char*>(entry->value);
 }


@@ -92,11 +102,9 @@
   int len = OS::VSNPrintF(str, format, args);
   if (len == -1) {
     DeleteArray(str.start());
-    return format;
+    return GetCopy(format);
   }
-  uint32_t hash = StringHasher::HashSequentialString(
-      str.start(), len, hash_seed_);
-  return AddOrDisposeString(str.start(), hash);
+  return AddOrDisposeString(str.start(), len);
 }


@@ -104,11 +112,11 @@
   if (name->IsString()) {
     String* str = String::cast(name);
     int length = Min(kMaxNameSize, str->length());
+    int actual_length = 0;
     SmartArrayPointer<char> data =
-        str->ToCString(DISALLOW_NULLS, ROBUST_STRING_TRAVERSAL, 0, length);
-    uint32_t hash = StringHasher::HashSequentialString(
-        *data, length, name->GetHeap()->HashSeed());
-    return AddOrDisposeString(data.Detach(), hash);
+        str->ToCString(DISALLOW_NULLS, ROBUST_STRING_TRAVERSAL, 0, length,
+                       &actual_length);
+    return AddOrDisposeString(data.Detach(), actual_length);
   } else if (name->IsSymbol()) {
     return "<symbol>";
   }
@@ -119,6 +127,21 @@
 const char* StringsStorage::GetName(int index) {
   return GetFormatted("%d", index);
 }
+
+
+const char* StringsStorage::GetFunctionName(Name* name) {
+  return BeautifyFunctionName(GetName(name));
+}
+
+
+const char* StringsStorage::GetFunctionName(const char* name) {
+  return BeautifyFunctionName(GetCopy(name));
+}
+
+
+const char* StringsStorage::BeautifyFunctionName(const char* name) {
+  return (*name == 0) ? ProfileGenerator::kAnonymousFunctionName : name;
+}


 size_t StringsStorage::GetUsedMemorySize() const {
@@ -129,6 +152,12 @@
   }
   return size;
 }
+
+
+HashMap::Entry* StringsStorage::GetEntry(const char* str, int len) {
+  uint32_t hash = StringHasher::HashSequentialString(str, len, hash_seed_);
+  return names_.Lookup(const_cast<char*>(str), hash, true);
+}


 const char* const CodeEntry::kEmptyNamePrefix = "";
=======================================
--- /branches/bleeding_edge/src/profile-generator.h Mon Oct 14 08:57:46 2013 UTC +++ /branches/bleeding_edge/src/profile-generator.h Fri Oct 18 08:56:14 2013 UTC
@@ -49,20 +49,18 @@
   const char* GetVFormatted(const char* format, va_list args);
   const char* GetName(Name* name);
   const char* GetName(int index);
-  inline const char* GetFunctionName(Name* name);
-  inline const char* GetFunctionName(const char* name);
+  const char* GetFunctionName(Name* name);
+  const char* GetFunctionName(const char* name);
   size_t GetUsedMemorySize() const;

  private:
   static const int kMaxNameSize = 1024;

-  static bool StringsMatch(void* key1, void* key2) {
-    return strcmp(reinterpret_cast<char*>(key1),
-                  reinterpret_cast<char*>(key2)) == 0;
-  }
-  const char* AddOrDisposeString(char* str, uint32_t hash);
+  static bool StringsMatch(void* key1, void* key2);
+  const char* BeautifyFunctionName(const char* name);
+  const char* AddOrDisposeString(char* str, int len);
+  HashMap::Entry* GetEntry(const char* str, int len);

-  // Mapping of strings by String::Hash to const char* strings.
   uint32_t hash_seed_;
   HashMap names_;

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to