Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(105)

Unified Diff: src/runtime.cc

Issue 42221: Added handle scope to inner loop of replace to avoid unbounded handle creation. (Closed)
Patch Set: Created 11 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index d0c8e30be611aa9294895c769185aa82f22de339..69d83f6881d330670d59609745b0a37a61016c93 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -1135,37 +1135,49 @@ class ReplacementStringBuilder {
ASSERT(estimated_part_count > 0);
}
+ void EnsureCapacity(int elements) {
+ int length = parts_->length();
+ int required_length = part_count_ + elements;
+ if (length < required_length) {
+ int new_length = length;
+ do {
+ new_length *= 2;
+ } while (new_length < required_length);
+ Handle<FixedArray> extended_array =
+ Factory::NewFixedArray(new_length);
+ parts_->CopyTo(0, *extended_array, 0, part_count_);
+ parts_ = extended_array;
+ }
+ }
+
void AddSubjectSlice(int from, int to) {
ASSERT(from >= 0);
int length = to - from;
- ASSERT(length >= 0);
- if (length > 0) {
- // Can we encode the slice in 11 bits for length and 19 bits for
- // start position - as used by StringBuilderConcatHelper?
- if (StringBuilderSubstringLength::is_valid(length) &&
- StringBuilderSubstringPosition::is_valid(from)) {
- int encoded_slice = StringBuilderSubstringLength::encode(length) |
- StringBuilderSubstringPosition::encode(from);
- AddElement(Handle<Object>(Smi::FromInt(encoded_slice)));
- } else {
- Handle<String> slice = Factory::NewStringSlice(subject_, from, to);
- AddElement(slice);
- }
- IncrementCharacterCount(length);
+ ASSERT(length > 0);
+ // Can we encode the slice in 11 bits for length and 19 bits for
+ // start position - as used by StringBuilderConcatHelper?
+ if (StringBuilderSubstringLength::is_valid(length) &&
+ StringBuilderSubstringPosition::is_valid(from)) {
+ int encoded_slice = StringBuilderSubstringLength::encode(length) |
+ StringBuilderSubstringPosition::encode(from);
+ AddElement(Handle<Object>(Smi::FromInt(encoded_slice)));
+ } else {
+ Handle<String> slice = Factory::NewStringSlice(subject_, from, to);
+ AddElement(slice);
}
+ IncrementCharacterCount(length);
}
void AddString(Handle<String> string) {
StringShape shape(*string);
int length = string->length(shape);
- if (length > 0) {
- AddElement(string);
- if (!shape.IsAsciiRepresentation()) {
- is_ascii_ = false;
- }
- IncrementCharacterCount(length);
+ ASSERT(length > 0);
+ AddElement(string);
+ if (!shape.IsAsciiRepresentation()) {
+ is_ascii_ = false;
}
+ IncrementCharacterCount(length);
}
@@ -1223,12 +1235,6 @@ class ReplacementStringBuilder {
void AddElement(Handle<Object> element) {
ASSERT(element->IsSmi() || element->IsString());
// Extend parts_ array if necessary.
- if (parts_->length() == part_count_) {
- Handle<FixedArray> extended_array =
- Factory::NewFixedArray(part_count_ * 2);
- parts_->CopyTo(0, *extended_array, 0, part_count_);
- parts_ = extended_array;
- }
parts_->set(part_count_, *element);
part_count_++;
}
@@ -1474,11 +1480,13 @@ void CompiledReplacement::Apply(ReplacementStringBuilder* builder,
ReplacementPart part = parts_[i];
switch (part.tag) {
case SUBJECT_PREFIX:
- builder->AddSubjectSlice(0, match_from);
+ if (match_from > 0) builder->AddSubjectSlice(0, match_from);
break;
case SUBJECT_SUFFIX: {
int subject_length = part.data;
- builder->AddSubjectSlice(match_to, subject_length);
+ if (match_to < subject_length) {
+ builder->AddSubjectSlice(match_to, subject_length);
+ }
break;
}
case SUBJECT_CAPTURE: {
@@ -1549,8 +1557,17 @@ static Object* StringReplaceRegExpWithString(String* subject,
// Index of end of last match.
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;
+ bool matched;
do {
ASSERT(last_match_info_handle->HasFastElements());
+ // Increase the capacity of the builder before entering local handle-scope,
+ // so its internal buffer can safely allocate a new handle if it grows.
+ builder.EnsureCapacity(parts_added_per_loop);
+
+ HandleScope loop_scope;
int start, end;
{
AssertNoAllocation match_info_array_is_not_in_a_handle;
@@ -1588,7 +1605,8 @@ static Object* StringReplaceRegExpWithString(String* subject,
if (match.is_null()) {
return Failure::Exception();
}
- } while (!match->IsNull());
+ matched = !match->IsNull();
+ } while (matched);
if (prev < length) {
builder.AddSubjectSlice(prev, length);
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698