[v8-dev] Make internalized string parser in JSON.parse GC-safe (issue 484703002 by [email protected])

Mon, 18 Aug 2014 08:42:48 -0700 

Reviewers: ,

Message:
Unfortunately I wasn't able to construct a test case. It reproduces at r23141 
with ARM64 debug:
out/arm64.debug/d8 --test --random-seed=-2073445275 --stress-opt --always-opt 
--nohard-abort --nodead-code-elimination --nofold-constants
--enable-slow-asserts --debug-code --verify-heap --expose-debug-as debug
test/mjsunit/mjsunit.js test/mjsunit/debug-scripts-request.js --gc-interval=500 
--stress-compaction

Description:
Make internalized string parser in JSON.parse GC-safe

SubStringKey::AsHandle is not GC-safe because the string backing store
may move.

Please review this at https://codereview.chromium.org/484703002/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+27, -70 lines):
  M src/factory.h
  M src/factory.cc
  M src/objects.cc
  M src/objects-inl.h


Index: src/factory.cc
diff --git a/src/factory.cc b/src/factory.cc
index bb5e97a34e1f09c5260c47e64f73eba6badb1324..5ecdfcfd2f4aec25c94ab22927b53363715b3b59 100644 
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -186,7 +186,7 @@ Handle<String> Factory::InternalizeOneByteString(Vector<const uint8_t> string) { 

 Handle<String> Factory::InternalizeOneByteString(
     Handle<SeqOneByteString> string, int from, int length) {
-  SubStringKey<uint8_t> key(string, from, length);
+  SeqOneByteSubStringKey key(string, from, length);
   return InternalizeStringWithKey(&key);
 }
@@ -203,12 +203,6 @@ Handle<String> Factory::InternalizeStringWithKey(StringTableKey* key) { 
 }


-template Handle<String> Factory::InternalizeStringWithKey<
-    SubStringKey<uint8_t> > (SubStringKey<uint8_t>* key);
-template Handle<String> Factory::InternalizeStringWithKey<
-    SubStringKey<uint16_t> > (SubStringKey<uint16_t>* key);
-
-
MaybeHandle<String> Factory::NewStringFromOneByte(Vector<const uint8_t> string, PretenureFlag pretenure) { 
   int length = string.length();
@@ -313,6 +307,17 @@ MUST_USE_RESULT Handle<String> Factory::NewOneByteInternalizedString( 
 }


+MUST_USE_RESULT Handle<String> Factory::NewOneByteInternalizedSubString(
+    Handle<SeqOneByteString> string, int offset, int length,
+    uint32_t hash_field) {
+  CALL_HEAP_FUNCTION(
+      isolate(), isolate()->heap()->AllocateOneByteInternalizedString(
+ Vector<const uint8_t>(string->GetChars() + offset, length), 
+                     hash_field),
+      String);
+}
+
+
 MUST_USE_RESULT Handle<String> Factory::NewTwoByteInternalizedString(
       Vector<const uc16> str,
       uint32_t hash_field) {
Index: src/factory.h
diff --git a/src/factory.h b/src/factory.h
index aa1f94d8145207640f554ece31c9707165f9ad0f..c3ad0dc5a5aafe1c8cfdb7692a7edb24197fa2d3 100644 
--- a/src/factory.h
+++ b/src/factory.h
@@ -164,8 +164,11 @@ class Factory V8_FINAL {
       uint32_t hash_field);

   MUST_USE_RESULT Handle<String> NewOneByteInternalizedString(
-        Vector<const uint8_t> str,
-        uint32_t hash_field);
+      Vector<const uint8_t> str, uint32_t hash_field);
+
+  MUST_USE_RESULT Handle<String> NewOneByteInternalizedSubString(
+      Handle<SeqOneByteString> string, int offset, int length,
+      uint32_t hash_field);

   MUST_USE_RESULT Handle<String> NewTwoByteInternalizedString(
         Vector<const uc16> str,
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index 89ac974e89e27b4c2e4c5a0b34ae5153cc3084f6..e2414d290046927d3d3f4a7e13d3b09e0311d5df 100644 
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -533,21 +533,17 @@ class OneByteStringKey : public SequentialStringKey<uint8_t> { 
 };


-template<class Char>
-class SubStringKey : public HashTableKey {
+class SeqOneByteSubStringKey : public HashTableKey {
  public:
-  SubStringKey(Handle<String> string, int from, int length)
+ SeqOneByteSubStringKey(Handle<SeqOneByteString> string, int from, int length) 
       : string_(string), from_(from), length_(length) {
-    if (string_->IsSlicedString()) {
-      string_ = Handle<String>(Unslice(*string_, &from_));
-    }
-    DCHECK(string_->IsSeqString() || string->IsExternalString());
+    DCHECK(string_->IsSeqOneByteString());
   }

   virtual uint32_t Hash() V8_OVERRIDE {
     DCHECK(length_ >= 0);
     DCHECK(from_ + length_ <= string_->length());
-    const Char* chars = GetChars() + from_;
+    const uint8_t* chars = string_->GetChars() + from_;
     hash_field_ = StringHasher::HashSequentialString(
         chars, length_, string_->GetHeap()->HashSeed());
     uint32_t result = hash_field_ >> String::kHashShift;
@@ -563,17 +559,7 @@ class SubStringKey : public HashTableKey {
   virtual Handle<Object> AsHandle(Isolate* isolate) V8_OVERRIDE;

  private:
-  const Char* GetChars();
-  String* Unslice(String* string, int* offset) {
-    while (string->IsSlicedString()) {
-      SlicedString* sliced = SlicedString::cast(string);
-      *offset += sliced->offset();
-      string = sliced->parent();
-    }
-    return string;
-  }
-
-  Handle<String> string_;
+  Handle<SeqOneByteString> string_;
   int from_;
   int length_;
   uint32_t hash_field_;
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index d407f74c644c619c71c4af592071901bad26b2b5..127290a993e17cab0296bb53d5466578e2fe2789 100644 
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -13894,56 +13894,19 @@ Handle<Object> TwoByteStringKey::AsHandle(Isolate* isolate) { 
 }


-template<>
-const uint8_t* SubStringKey<uint8_t>::GetChars() {
-  return string_->IsSeqOneByteString()
-      ? SeqOneByteString::cast(*string_)->GetChars()
-      : ExternalAsciiString::cast(*string_)->GetChars();
-}
-
-
-template<>
-const uint16_t* SubStringKey<uint16_t>::GetChars() {
-  return string_->IsSeqTwoByteString()
-      ? SeqTwoByteString::cast(*string_)->GetChars()
-      : ExternalTwoByteString::cast(*string_)->GetChars();
-}
-
-
-template<>
-Handle<Object> SubStringKey<uint8_t>::AsHandle(Isolate* isolate) {
+Handle<Object> SeqOneByteSubStringKey::AsHandle(Isolate* isolate) {
   if (hash_field_ == 0) Hash();
-  Vector<const uint8_t> chars(GetChars() + from_, length_);
- return isolate->factory()->NewOneByteInternalizedString(chars, hash_field_); 
+  return isolate->factory()->NewOneByteInternalizedSubString(
+      string_, from_, length_, hash_field_);
 }


-template<>
-Handle<Object> SubStringKey<uint16_t>::AsHandle(Isolate* isolate) {
-  if (hash_field_ == 0) Hash();
-  Vector<const uint16_t> chars(GetChars() + from_, length_);
- return isolate->factory()->NewTwoByteInternalizedString(chars, hash_field_); 
-}
-
-
-template<>
-bool SubStringKey<uint8_t>::IsMatch(Object* string) {
-  Vector<const uint8_t> chars(GetChars() + from_, length_);
+bool SeqOneByteSubStringKey::IsMatch(Object* string) {
+  Vector<const uint8_t> chars(string_->GetChars() + from_, length_);
   return String::cast(string)->IsOneByteEqualTo(chars);
 }


-template<>
-bool SubStringKey<uint16_t>::IsMatch(Object* string) {
-  Vector<const uint16_t> chars(GetChars() + from_, length_);
-  return String::cast(string)->IsTwoByteEqualTo(chars);
-}
-
-
-template class SubStringKey<uint8_t>;
-template class SubStringKey<uint16_t>;
-
-
// InternalizedStringKey carries a string/internalized-string object as key. 
 class InternalizedStringKey : public HashTableKey {
  public:


--
--
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.

Reply via email to