Fix adding short external ascii strings

BUG=http://code.google.com/p/v8/issues/detail?id=536
TEST=cctest/test-strings/ExternalShortStringAdd
Review URL: http://codereview.chromium.org/466001

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3397 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/heap.cc b/src/heap.cc
index 034a744..79a91cf 100644 (file)
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -1819,10 +1819,19 @@ Object* Heap::AllocateConsString(String* first, String* second) {
       // Copy the characters into the new object.
       char* dest = SeqAsciiString::cast(result)->GetChars();
       // Copy first part.
-      char* src = SeqAsciiString::cast(first)->GetChars();
+      const char* src;
+      if (first->IsExternalString()) {
+        src = ExternalAsciiString::cast(first)->resource()->data();
+      } else {
+        src = SeqAsciiString::cast(first)->GetChars();
+      }
       for (int i = 0; i < first_length; i++) *dest++ = src[i];
       // Copy second part.
-      src = SeqAsciiString::cast(second)->GetChars();
+      if (second->IsExternalString()) {
+        src = ExternalAsciiString::cast(second)->resource()->data();
+      } else {
+        src = SeqAsciiString::cast(second)->GetChars();
+      }
       for (int i = 0; i < second_length; i++) *dest++ = src[i];
       return result;
     } else {

diff --git a/test/cctest/test-strings.cc b/test/cctest/test-strings.cc
index 0e9bf7a..0e47ff3 100644 (file)
--- a/test/cctest/test-strings.cc
+++ b/test/cctest/test-strings.cc
@@ -63,6 +63,21 @@ class Resource: public v8::String::ExternalStringResource,
 };
 
 
+class AsciiResource: public v8::String::ExternalAsciiStringResource,
+                public ZoneObject {
+ public:
+  explicit AsciiResource(Vector<const char> string): data_(string.start()) {
+    length_ = string.length();
+  }
+  virtual const char* data() const { return data_; }
+  virtual size_t length() const { return length_; }
+
+ private:
+  const char* data_;
+  size_t length_;
+};
+
+
 static void InitializeBuildingBlocks(
     Handle<String> building_blocks[NUMBER_OF_BUILDING_BLOCKS]) {
   // A list of pointers that we don't have any interest in cleaning up.
@@ -329,25 +344,65 @@ TEST(Utf8Conversion) {
 }
 
 
-class TwoByteResource: public v8::String::ExternalStringResource {
- public:
-  TwoByteResource(const uint16_t* data, size_t length, bool* destructed)
-      : data_(data), length_(length), destructed_(destructed) {
-    CHECK_NE(destructed, NULL);
-    *destructed_ = false;
-  }
+TEST(ExternalShortStringAdd) {
+  ZoneScope zone(DELETE_ON_EXIT);
 
-  virtual ~TwoByteResource() {
-    CHECK_NE(destructed_, NULL);
-    CHECK(!*destructed_);
-    *destructed_ = true;
-  }
+  InitializeVM();
+  v8::HandleScope handle_scope;
 
-  const uint16_t* data() const { return data_; }
-  size_t length() const { return length_; }
+  // Make sure we cover all always-flat lengths and at least one above.
+  static const int kMaxLength = 20;
+  CHECK_GT(kMaxLength, i::String::kMinNonFlatLength);
+
+  // Allocate two JavaScript arrays for holding short strings.
+  v8::Handle<v8::Array> ascii_external_strings =
+      v8::Array::New(kMaxLength + 1);
+  v8::Handle<v8::Array> non_ascii_external_strings =
+      v8::Array::New(kMaxLength + 1);
+
+  // Generate short ascii and non-ascii external strings.
+  for (int i = 0; i <= kMaxLength; i++) {
+    char* ascii = Zone::NewArray<char>(i + 1);
+    for (int j = 0; j < i; j++) {
+      ascii[j] = 'a';
+    }
+    // Terminating '\0' is left out on purpose. It is not required for external
+    // string data.
+    AsciiResource* ascii_resource =
+        new AsciiResource(Vector<const char>(ascii, i));
+    v8::Local<v8::String> ascii_external_string =
+        v8::String::NewExternal(ascii_resource);
+
+    ascii_external_strings->Set(v8::Integer::New(i), ascii_external_string);
+    uc16* non_ascii = Zone::NewArray<uc16>(i + 1);
+    for (int j = 0; j < i; j++) {
+      non_ascii[j] = 1234;
+    }
+    // Terminating '\0' is left out on purpose. It is not required for external
+    // string data.
+    Resource* resource = new Resource(Vector<const uc16>(non_ascii, i));
+    v8::Local<v8::String> non_ascii_external_string =
+      v8::String::NewExternal(resource);
+    non_ascii_external_strings->Set(v8::Integer::New(i),
+                                    non_ascii_external_string);
+  }
 
- private:
-  const uint16_t* data_;
-  size_t length_;
-  bool* destructed_;
-};
+  // Add the arrays with the short external strings in the global object.
+  v8::Handle<v8::Object> global = env->Global();
+  global->Set(v8_str("ascii"), ascii_external_strings);
+  global->Set(v8_str("non_ascii"), non_ascii_external_strings);
+
+  // Add short external ascii and non-ascii strings checking the result.
+  static const char* source =
+    "function test() {"
+    "  for (var i = 0; i <= 20; i++) {"
+    "    for (var j = 0; j < i; j++) {"
+    "      if (non_ascii[i] != (non_ascii[j] + non_ascii[i - j])) return false;"
+    "      if (ascii[i] != (ascii[j] + ascii[i - j])) return false;"
+    "    }"
+    "  }"
+    "  return true;"
+    "};"
+    "test()";
+  CHECK(v8::Script::Compile(v8::String::New(source))->Run()->BooleanValue());
+}