Reviewers: alph, loislo, Sven Panne,
Message:
There is at least one case when the string stored in the storage will be
destroyed very soon after:
https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/code-stubs.cc&q=code-stubs.cc:75&sq=package:chromium&type=cs&l=76
Description:
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
Please review this at https://codereview.chromium.org/27627006/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+52, -37 lines):
M src/profile-generator-inl.h
M src/profile-generator.h
M src/profile-generator.cc
Index: src/profile-generator-inl.h
diff --git a/src/profile-generator-inl.h b/src/profile-generator-inl.h
index
ac23ee3f01195be14df4de15c951ac1f030a98ee..e363f67761b78483e57c61287d25b4109f02af06
100644
--- a/src/profile-generator-inl.h
+++ b/src/profile-generator-inl.h
@@ -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,
Index: src/profile-generator.cc
diff --git a/src/profile-generator.cc b/src/profile-generator.cc
index
cf268afc9b41e18f0b763816df783a1f0008798d..edee2886ca2027ab32820b35dc844dedb8c25458
100644
--- a/src/profile-generator.cc
+++ b/src/profile-generator.cc
@@ -40,6 +40,10 @@
namespace v8 {
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 +61,15 @@ StringsStorage::~StringsStorage() {
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 +82,16 @@ const char* StringsStorage::GetFormatted(const char*
format, ...) {
}
-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 +100,9 @@ const char* StringsStorage::GetVFormatted(const char*
format, va_list args) {
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 +110,11 @@ const char* StringsStorage::GetName(Name* name) {
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>";
}
@@ -121,6 +127,21 @@ const char* StringsStorage::GetName(int 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 {
size_t size = sizeof(*this);
size += sizeof(HashMap::Entry) * names_.capacity();
@@ -131,6 +152,12 @@ size_t StringsStorage::GetUsedMemorySize() const {
}
+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 = "";
const char* const CodeEntry::kEmptyResourceName = "";
const char* const CodeEntry::kEmptyBailoutReason = "";
Index: src/profile-generator.h
diff --git a/src/profile-generator.h b/src/profile-generator.h
index
890fb2e5f80a28b63bbf96e7980433d73578918b..6e4758bece76b2285d3115056a6bae6ebfe0ff79
100644
--- a/src/profile-generator.h
+++ b/src/profile-generator.h
@@ -49,20 +49,18 @@ class StringsStorage {
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.
