Reviewers: rossberg,
Message:
rossberg, ptal at this fixed fix.
The failing test was WebPageNewSerializeTest.BlankFrames and it passes with
the
fix.
The failure was:
#
# Fatal error in ../../v8/src/scopeinfo.cc, line 300
# CHECK(name->IsInternalizedString()) failed
#
First patch set is the original CL and second is the fixed version.
Description:
Retry: Parser & internalization fix: ensure no heap allocs during
GetString(Handle<String>).
The bug has always been there: when the parser is operating in
the "immediately
internalize" mode and calls GetString, we get FlatContent of a string and
then
do heap allocation.
The bug was uncovered by https://codereview.chromium.org/693803004/ (which
put
the parser to the "immediately internalize" mode more often), but looking
at the
code, it's possible that it can happen in other cases too.
This CL makes AstValueFactory handle this situation gracefully: it won't
try to
internalize inside GetString(Handle<String>); it's unnecessary anyway since
we
have the Handle<String> already.
BUG=
Please review this at https://codereview.chromium.org/706533005/
Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+31, -14 lines):
M src/ast-value-factory.h
M src/ast-value-factory.cc
Index: src/ast-value-factory.cc
diff --git a/src/ast-value-factory.cc b/src/ast-value-factory.cc
index
895ce39f652845fa83674d61ec98b0c9b8f1b5e1..518be23abcb2ab7646581778e1c4866eaf72f604
100644
--- a/src/ast-value-factory.cc
+++ b/src/ast-value-factory.cc
@@ -212,7 +212,7 @@ void AstValue::Internalize(Isolate* isolate) {
}
-const AstRawString* AstValueFactory::GetOneByteString(
+AstRawString* AstValueFactory::GetOneByteStringInternal(
Vector<const uint8_t> literal) {
uint32_t hash = StringHasher::HashSequentialString<uint8_t>(
literal.start(), literal.length(), hash_seed_);
@@ -220,7 +220,7 @@ const AstRawString* AstValueFactory::GetOneByteString(
}
-const AstRawString* AstValueFactory::GetTwoByteString(
+AstRawString* AstValueFactory::GetTwoByteStringInternal(
Vector<const uint16_t> literal) {
uint32_t hash = StringHasher::HashSequentialString<uint16_t>(
literal.start(), literal.length(), hash_seed_);
@@ -229,13 +229,24 @@ const AstRawString* AstValueFactory::GetTwoByteString(
const AstRawString* AstValueFactory::GetString(Handle<String> literal) {
- DisallowHeapAllocation no_gc;
- String::FlatContent content = literal->GetFlatContent();
- if (content.IsOneByte()) {
- return GetOneByteString(content.ToOneByteVector());
+ // For the FlatContent to stay valid, we shouldn't do any heap
+ // allocation. Make sure we won't try to internalize the string in
GetString.
+ AstRawString* result = NULL;
+ Isolate* saved_isolate = isolate_;
+ isolate_ = NULL;
+ {
+ DisallowHeapAllocation no_gc;
+ String::FlatContent content = literal->GetFlatContent();
+ if (content.IsOneByte()) {
+ result = GetOneByteStringInternal(content.ToOneByteVector());
+ } else {
+ DCHECK(content.IsTwoByte());
+ result = GetTwoByteStringInternal(content.ToUC16Vector());
+ }
}
- DCHECK(content.IsTwoByte());
- return GetTwoByteString(content.ToUC16Vector());
+ isolate_ = saved_isolate;
+ if (isolate_) result->Internalize(isolate_);
+ return result;
}
@@ -348,8 +359,8 @@ const AstValue* AstValueFactory::NewTheHole() {
#undef GENERATE_VALUE_GETTER
-const AstRawString* AstValueFactory::GetString(
- uint32_t hash, bool is_one_byte, Vector<const byte> literal_bytes) {
+AstRawString* AstValueFactory::GetString(uint32_t hash, bool is_one_byte,
+ Vector<const byte> literal_bytes)
{
// literal_bytes here points to whatever the user passed, and this is OK
// because we use vector_compare (which checks the contents) to compare
// against the AstRawStrings which are in the string_table_. We should
not
Index: src/ast-value-factory.h
diff --git a/src/ast-value-factory.h b/src/ast-value-factory.h
index
071ca9ce9cc18abf23e17f09c31e69ca69a8ccce..09a41400fb3ec9b4b89f97b3ebe57e097f374e07
100644
--- a/src/ast-value-factory.h
+++ b/src/ast-value-factory.h
@@ -287,12 +287,16 @@ class AstValueFactory {
Zone* zone() const { return zone_; }
- const AstRawString* GetOneByteString(Vector<const uint8_t> literal);
+ const AstRawString* GetOneByteString(Vector<const uint8_t> literal) {
+ return GetOneByteStringInternal(literal);
+ }
const AstRawString* GetOneByteString(const char* string) {
return GetOneByteString(Vector<const uint8_t>(
reinterpret_cast<const uint8_t*>(string), StrLength(string)));
}
- const AstRawString* GetTwoByteString(Vector<const uint16_t> literal);
+ const AstRawString* GetTwoByteString(Vector<const uint16_t> literal) {
+ return GetTwoByteStringInternal(literal);
+ }
const AstRawString* GetString(Handle<String> literal);
const AstConsString* NewConsString(const AstString* left,
const AstString* right);
@@ -327,8 +331,10 @@ class AstValueFactory {
const AstValue* NewTheHole();
private:
- const AstRawString* GetString(uint32_t hash, bool is_one_byte,
- Vector<const byte> literal_bytes);
+ AstRawString* GetOneByteStringInternal(Vector<const uint8_t> literal);
+ AstRawString* GetTwoByteStringInternal(Vector<const uint16_t> literal);
+ AstRawString* GetString(uint32_t hash, bool is_one_byte,
+ Vector<const byte> literal_bytes);
// All strings are copied here, one after another (no NULLs inbetween).
HashMap string_table_;
--
--
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/d/optout.
