Revision: 3197 Author: [email protected] Date: Mon Nov 2 04:21:43 2009 Log: 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 http://code.google.com/p/v8/source/detail?r=3197 Added: /branches/bleeding_edge/test/mjsunit/regress/regress-490.js Modified: /branches/bleeding_edge/src/runtime.cc /branches/bleeding_edge/src/string.js ======================================= --- /dev/null +++ /branches/bleeding_edge/test/mjsunit/regress/regress-490.js Mon Nov 2 04:21:43 2009 @@ -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; +} ======================================= --- /branches/bleeding_edge/src/runtime.cc Thu Oct 29 06:17:11 2009 +++ /branches/bleeding_edge/src/runtime.cc Mon Nov 2 04:21:43 2009 @@ -1357,8 +1357,9 @@ StringBuilderSubstringPosition::encode(from); AddElement(Smi::FromInt(encoded_slice)); } else { - Handle<String> slice = Factory::NewStringSlice(subject_, from, to); - AddElement(*slice); + // Otherwise encode as two smis. + AddElement(Smi::FromInt(-length)); + AddElement(Smi::FromInt(from)); } IncrementCharacterCount(length); } @@ -1750,8 +1751,9 @@ int prev = 0; // Number of parts added by compiled replacement plus preceeding string - // and possibly suffix after last match. - const int parts_added_per_loop = compiled_replacement.parts() + 2; + // and possibly suffix after last match. It is possible for compiled + // 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 @@ 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 len = StringBuilderSubstringLength::decode(encoded_slice); + int pos; + 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 @@ 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 @@ 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(); - int pos = len >> 11; - len &= 0x7ff; - if (pos + len > special_length) { - return Top::Throw(Heap::illegal_argument_symbol()); - } - position += len; + 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(); ======================================= --- /branches/bleeding_edge/src/string.js Thu Oct 29 00:31:23 2009 +++ /branches/bleeding_edge/src/string.js Mon Nov 2 04:21:43 2009 @@ -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 @@ 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; } } --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
