Don't use string slices when processing RexExp replace (re-apply r3153)

Re-apply r3153 with a fix for issue 490. Except for the change in line 1756 and the added test this change is identical to http://codereview.chromium.org/342015.

BUG=490
TEST=test/mjsunit/regress/regress-490.js
Review URL: http://codereview.chromium.org/341064

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

src/runtime.cc

@@ -1357,8 +1357,9 @@ class ReplacementStringBuilder {
           StringBuilderSubstringPosition::encode(from);
       AddElement(Smi::FromInt(encoded_slice));
     } else {
-      Handle<String> slice = Factory::NewStringSlice(subject_, from, to);
+      // Otherwise encode as two smis.
-      AddElement(*slice);
+      AddElement(Smi::FromInt(-length));
+      AddElement(Smi::FromInt(from));
     }
     IncrementCharacterCount(length);
   }
@@ -1750,8 +1751,9 @@ static Object* StringReplaceRegExpWithString(String* subject,
   int prev = 0;
 
   // Number of parts added by compiled replacement plus preceeding string
-  // and possibly suffix after last match.
+  // and possibly suffix after last match. It is possible for compiled
-  const int parts_added_per_loop = compiled_replacement.parts() + 2;
+  // replacements to use two elements when encoded as two smis.
+  const int parts_added_per_loop = compiled_replacement.parts() * 2 + 2;
   bool matched = true;
   do {
     ASSERT(last_match_info_handle->HasFastElements());
@@ -3766,9 +3768,21 @@ static inline void StringBuilderConcatHelper(String* special,
   for (int i = 0; i < array_length; i++) {
     Object* element = fixed_array->get(i);
     if (element->IsSmi()) {
+      // Smi encoding of position and length.
       int encoded_slice = Smi::cast(element)->value();
-      int pos = StringBuilderSubstringPosition::decode(encoded_slice);
+      int pos;
-      int len = StringBuilderSubstringLength::decode(encoded_slice);
+      int len;
+      if (encoded_slice > 0) {
+        // Position and length encoded in one smi.
+        pos = StringBuilderSubstringPosition::decode(encoded_slice);
+        len = StringBuilderSubstringLength::decode(encoded_slice);
+      } else {
+        // Position and length encoded in two smis.
+        Object* obj = fixed_array->get(++i);
+        ASSERT(obj->IsSmi());
+        pos = Smi::cast(obj)->value();
+        len = -encoded_slice;
+      }
       String::WriteToFlat(special,
                           sink + position,
                           pos,
@@ -3789,6 +3803,10 @@ static Object* Runtime_StringBuilderConcat(Arguments args) {
   ASSERT(args.length() == 2);
   CONVERT_CHECKED(JSArray, array, args[0]);
   CONVERT_CHECKED(String, special, args[1]);
+
+  // This assumption is used by the slice encoding in one or two smis.
+  ASSERT(Smi::kMaxValue >= String::kMaxLength);
+
   int special_length = special->length();
   Object* smi_array_length = array->length();
   if (!smi_array_length->IsSmi()) {
@@ -3816,13 +3834,29 @@ static Object* Runtime_StringBuilderConcat(Arguments args) {
   for (int i = 0; i < array_length; i++) {
     Object* elt = fixed_array->get(i);
     if (elt->IsSmi()) {
+      // Smi encoding of position and length.
       int len = Smi::cast(elt)->value();
+      if (len > 0) {
+        // Position and length encoded in one smi.
         int pos = len >> 11;
         len &= 0x7ff;
         if (pos + len > special_length) {
           return Top::Throw(Heap::illegal_argument_symbol());
         }
         position += len;
+      } else {
+        // Position and length encoded in two smis.
+        position += (-len);
+        // Get the position and check that it is also a smi.
+        i++;
+        if (i >= array_length) {
+          return Top::Throw(Heap::illegal_argument_symbol());
+        }
+        Object* pos = fixed_array->get(i);
+        if (!pos->IsSmi()) {
+          return Top::Throw(Heap::illegal_argument_symbol());
+        }
+      }
     } else if (elt->IsString()) {
       String* element = String::cast(elt);
       int element_length = element->length();

src/string.js

@@ -1,4 +1,4 @@
-// Copyright 2006-2008 the V8 project authors. All rights reserved.
+// Copyright 2006-2009 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
@@ -810,10 +810,13 @@ ReplaceResultBuilder.prototype.addSpecialSlice = function(start, end) {
   var len = end - start;
   if (len == 0) return;
   var elements = this.elements;
-  if (start >= 0 && len >= 0 && start < 0x80000 && len < 0x800) {
+  if (start < 0x80000 && len < 0x800) {
     elements[elements.length] = (start << 11) + len;
   } else {
-    elements[elements.length] = SubString(this.special_string, start, end);
+    // 0 < len <= String::kMaxLength and Smi::kMaxValue >= String::kMaxLength,
+    // so -len is a smi.
+    elements[elements.length] = -len;
+    elements[elements.length] = start;
   }
 }
 

test/mjsunit/regress/regress-490.js

@@ -0,0 +1,48 @@
+// Copyright 2009 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// See: http://code.google.com/p/v8/issues/detail?id=490
+
+var kXXX = 11
+// Build a string longer than 2^11. See StringBuilderConcatHelper and
+// Runtime_StringBuilderConcat in runtime.cc and
+// ReplaceResultBuilder.prototype.addSpecialSlice in string.js.
+var a = '';
+while (a.length < (2 << 11)) { a+= 'x'; }
+
+// Test specific for bug introduced in r3153.
+a.replace(/^(.*)/, '$1$1$1');
+
+// More generalized test.
+for (var i = 0; i < 10; i++) {
+  var  b = '';
+  for (var j = 0; j < 10; j++) {
+    b += '$1';
+    a.replace(/^(.*)/, b);
+  }
+  a += a;
+}