From b4e7335c0c12d6acecd582cad5f897a47ca47c31 Mon Sep 17 00:00:00 2001 From: "erik.corry@gmail.com" Date: Wed, 5 Nov 2008 10:26:08 +0000 Subject: [PATCH] Removed some unsafe uses of StringShape. Simplified some uses of StringShape. Removed unused function SlicedStringFlatten. Review URL: http://codereview.chromium.org/9408 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@695 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/compiler.cc | 3 +-- src/conversions.cc | 5 ++--- src/factory.cc | 9 ++------- src/factory.h | 1 - src/handles.cc | 2 +- src/heap.cc | 11 +++++------ src/heap.h | 5 +---- src/jsregexp.cc | 1 + src/log.cc | 3 +-- src/objects.cc | 24 ++++-------------------- src/objects.h | 9 ++++----- src/parser.cc | 2 +- src/runtime.cc | 28 ++++++++++++---------------- test/cctest/test-strings.cc | 9 ++------- 14 files changed, 37 insertions(+), 75 deletions(-) diff --git a/src/compiler.cc b/src/compiler.cc index 53c1a662a..2a2bb8726 100644 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -209,8 +209,7 @@ Handle Compiler::Compile(Handle source, Handle Compiler::CompileEval(Handle source, int line_offset, bool is_global) { - StringShape source_shape(*source); - int source_length = source->length(source_shape); + int source_length = source->length(); Counters::total_eval_size.Increment(source_length); Counters::total_compile_size.Increment(source_length); diff --git a/src/conversions.cc b/src/conversions.cc index 20a394a58..4b4faa551 100644 --- a/src/conversions.cc +++ b/src/conversions.cc @@ -121,15 +121,14 @@ static inline bool SubStringEquals(const char* str, static inline bool SubStringEquals(String* str, int index, const char* other) { - StringShape shape(str); HandleScope scope; - int str_length = str->length(shape); + int str_length = str->length(); int other_length = strlen(other); int end = index + other_length < str_length ? index + other_length : str_length; Handle slice = - Factory::NewStringSlice(Handle(str), shape, index, end); + Factory::NewStringSlice(Handle(str), index, end); return slice->IsEqualTo(Vector(other, other_length)); } diff --git a/src/factory.cc b/src/factory.cc index 3b3c2c3e5..209cd4d33 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -95,19 +95,14 @@ Handle Factory::NewConsString(Handle first, StringShape second_shape) { if (first->length(first_shape) == 0) return second; if (second->length(second_shape) == 0) return first; - CALL_HEAP_FUNCTION(Heap::AllocateConsString(*first, - first_shape, - *second, - second_shape), - String); + CALL_HEAP_FUNCTION(Heap::AllocateConsString(*first, *second), String); } Handle Factory::NewStringSlice(Handle str, - StringShape shape, int begin, int end) { - CALL_HEAP_FUNCTION(str->Slice(shape, begin, end), String); + CALL_HEAP_FUNCTION(str->Slice(begin, end), String); } diff --git a/src/factory.h b/src/factory.h index 67fed5746..89e2d698f 100644 --- a/src/factory.h +++ b/src/factory.h @@ -105,7 +105,6 @@ class Factory : public AllStatic { // Create a new sliced string object which represents a substring of a // backing string. static Handle NewStringSlice(Handle str, - StringShape shape, int begin, int end); diff --git a/src/handles.cc b/src/handles.cc index 70433192c..ada10f170 100644 --- a/src/handles.cc +++ b/src/handles.cc @@ -217,7 +217,7 @@ Handle LookupSingleCharacterStringFromCode(uint32_t index) { Handle SubString(Handle str, int start, int end) { - CALL_HEAP_FUNCTION(str->Slice(StringShape(*str), start, end), String); + CALL_HEAP_FUNCTION(str->Slice(start, end), String); } diff --git a/src/heap.cc b/src/heap.cc index 805ca0948..e62234f08 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -783,8 +783,7 @@ static inline bool IsShortcutCandidate(HeapObject* object, Map* map) { // is a candidate for being shortcut by the scavenger. ASSERT(object->map() == map); if (map->instance_type() >= FIRST_NONSTRING_TYPE) return false; - StringShape shape(map); - return (shape.representation_tag() == kConsStringTag) && + return (StringShape(map).representation_tag() == kConsStringTag) && (ConsString::cast(object)->unchecked_second() == Heap::empty_string()); } @@ -1347,9 +1346,9 @@ Object* Heap::AllocateSharedFunctionInfo(Object* name) { Object* Heap::AllocateConsString(String* first, - StringShape first_shape, - String* second, - StringShape second_shape) { + String* second) { + StringShape first_shape(first); + StringShape second_shape(second); int first_length = first->length(first_shape); int second_length = second->length(second_shape); int length = first_length + second_length; @@ -1411,9 +1410,9 @@ Object* Heap::AllocateConsString(String* first, Object* Heap::AllocateSlicedString(String* buffer, - StringShape buffer_shape, int start, int end) { + StringShape buffer_shape(buffer); int length = end - start; // If the resulting string is small make a sub string. diff --git a/src/heap.h b/src/heap.h index dc50c73c9..d89caf93d 100644 --- a/src/heap.h +++ b/src/heap.h @@ -495,9 +495,7 @@ class Heap : public AllStatic { // failed. // Please note this does not perform a garbage collection. static Object* AllocateConsString(String* first, - StringShape first_shape, - String* second, - StringShape second_shape); + String* second); // Allocates a new sliced string object which is a slice of an underlying // string buffer stretching from the index start (inclusive) to the index @@ -506,7 +504,6 @@ class Heap : public AllStatic { // failed. // Please note this does not perform a garbage collection. static Object* AllocateSlicedString(String* buffer, - StringShape buffer_shape, int start, int end); diff --git a/src/jsregexp.cc b/src/jsregexp.cc index 1ad33b262..2dd21bd71 100644 --- a/src/jsregexp.cc +++ b/src/jsregexp.cc @@ -132,6 +132,7 @@ Handle RegExpImpl::StringToTwoByte(Handle pattern) { StringShape shape(*pattern); if (!pattern->IsFlat(shape)) { FlattenString(pattern); + shape = StringShape(*pattern); } Handle flat_string(shape.IsCons() ? String::cast(ConsString::cast(*pattern)->first()) : diff --git a/src/log.cc b/src/log.cc index c7b1539fe..bd85fea9f 100644 --- a/src/log.cc +++ b/src/log.cc @@ -431,8 +431,7 @@ void Logger::RegExpExecEvent(Handle regexp, LogRegExpSource(regexp); fprintf(logfile_, ","); LogString(input_string); - StringShape shape(*input_string); - fprintf(logfile_, ",%d..%d\n", start_index, input_string->length(shape)); + fprintf(logfile_, ",%d..%d\n", start_index, input_string->length()); #endif } diff --git a/src/objects.cc b/src/objects.cc index 842185675..00d4406d7 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -3717,21 +3717,6 @@ uint16_t ConsString::ConsStringGet(int index) { } -Object* SlicedString::SlicedStringFlatten() { - // The SlicedString constructor should ensure that there are no - // SlicedStrings that are constructed directly on top of other - // SlicedStrings. - String* buf = String::cast(buffer()); - StringShape buf_shape(buf); - ASSERT(!buf_shape.IsSliced()); - if (buf_shape.IsCons()) { - Object* ok = buf->Flatten(buf_shape); - if (ok->IsFailure()) return ok; - } - return this; -} - - template void String::WriteToFlat(String* src, StringShape src_shape, @@ -3975,8 +3960,7 @@ bool String::SlowEquals(StringShape this_shape, bool String::MarkAsUndetectable() { - StringShape shape(this); - if (shape.IsSymbol()) return false; + if (StringShape(this).IsSymbol()) return false; Map* map = this->map(); if (map == Heap::short_string_map()) { @@ -4134,7 +4118,8 @@ uint32_t String::ComputeLengthAndHashField(unibrow::CharacterStream* buffer, } -Object* String::Slice(StringShape shape, int start, int end) { +Object* String::Slice(int start, int end) { + StringShape shape(this); if (start == 0 && end == length(shape)) return this; if (shape.representation_tag() == kSlicedStringTag) { // Translate slices of a SlicedString into slices of the @@ -4142,11 +4127,10 @@ Object* String::Slice(StringShape shape, int start, int end) { SlicedString* str = SlicedString::cast(this); String* buf = str->buffer(); return Heap::AllocateSlicedString(buf, - StringShape(buf), str->start() + start, str->start() + end); } - Object* result = Heap::AllocateSlicedString(this, shape, start, end); + Object* result = Heap::AllocateSlicedString(this, start, end); if (result->IsFailure()) { return result; } diff --git a/src/objects.h b/src/objects.h index a29182264..82edc53d4 100644 --- a/src/objects.h +++ b/src/objects.h @@ -3024,7 +3024,9 @@ class StringHasher { // the shape of the string is given its own class so that it can be retrieved // once and used for several string operations. A StringShape is small enough // to be passed by value and is immutable, but be aware that flattening a -// string can potentially alter its shape. +// string can potentially alter its shape. Also be aware that a GC caused by +// something else can alter the shape of a string due to ConsString +// shortcutting. // // Most of the methods designed to interrogate a string as to its exact nature // have been made into methods on StringShape in order to encourage the use of @@ -3116,7 +3118,7 @@ class String: public HeapObject { bool MarkAsUndetectable(); // Slice the string and return a substring. - Object* Slice(StringShape shape, int from, int to); + Object* Slice(int from, int to); // String equality operations. inline bool Equals(String* other); @@ -3471,9 +3473,6 @@ class SlicedString: public String { // Dispatched behavior. uint16_t SlicedStringGet(int index); - // Flatten any ConsString hiding behind this SlicedString. - Object* SlicedStringFlatten(); - // Casting. static inline SlicedString* cast(Object* obj); diff --git a/src/parser.cc b/src/parser.cc index ef92e3c28..bcc439f4c 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -790,8 +790,8 @@ FunctionLiteral* Parser::ParseLazy(Handle source, bool is_expression) { ZoneScope zone_scope(DONT_DELETE_ON_EXIT); StatsRateScope timer(&Counters::parse_lazy); + source->TryFlatten(StringShape(*source)); StringShape shape(*source); - source->TryFlatten(shape); Counters::total_parse_size.Increment(source->length(shape)); SafeStringInputBuffer buffer(source.location()); diff --git a/src/runtime.cc b/src/runtime.cc index cbf45f3c7..132ccb47b 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -971,12 +971,12 @@ static Object* CharCodeAt(String* subject, Object* index) { // Flatten the string. If someone wants to get a char at an index // in a cons string, it is likely that more indices will be // accessed. + subject->TryFlatten(StringShape(subject)); StringShape shape(subject); - subject->TryFlatten(shape); // shape no longer valid! - if (i >= static_cast(subject->length(StringShape(subject)))) { + if (i >= static_cast(subject->length(shape))) { return Heap::nan_value(); } - return Smi::FromInt(subject->Get(StringShape(subject), i)); + return Smi::FromInt(subject->Get(shape, i)); } @@ -1357,10 +1357,9 @@ int Runtime::StringMatch(Handle sub, int start_index) { ASSERT(0 <= start_index); StringShape sub_shape(*sub); - StringShape pat_shape(*pat); ASSERT(start_index <= sub->length(sub_shape)); - int pattern_length = pat->length(pat_shape); + int pattern_length = pat->length(); if (pattern_length == 0) return start_index; int subject_length = sub->length(sub_shape); @@ -1370,6 +1369,7 @@ int Runtime::StringMatch(Handle sub, FlattenString(sub); sub_shape = StringShape(*sub); } + StringShape pat_shape(*pat); // Searching for one specific character is common. For one // character patterns linear search is necessary, so any smart // algorithm is unnecessary overhead. @@ -1388,6 +1388,7 @@ int Runtime::StringMatch(Handle sub, if (!pat->IsFlat(pat_shape)) { FlattenString(pat); pat_shape = StringShape(*pat); + sub_shape = StringShape(*sub); } AssertNoAllocation no_heap_allocation; // ensure vectors stay valid @@ -1522,12 +1523,10 @@ static Object* Runtime_StringSlice(Arguments args) { int start = FastD2I(from_number); int end = FastD2I(to_number); - StringShape shape(value); - RUNTIME_ASSERT(end >= start); RUNTIME_ASSERT(start >= 0); - RUNTIME_ASSERT(end <= value->length(shape)); - return value->Slice(shape, start, end); + RUNTIME_ASSERT(end <= value->length()); + return value->Slice(start, end); } @@ -1910,8 +1909,7 @@ static Object* Runtime_HasLocalProperty(Arguments args) { uint32_t index; if (key->AsArrayIndex(&index)) { String* string = String::cast(args[0]); - StringShape shape(string); - if (index < static_cast(string->length(shape))) + if (index < static_cast(string->length())) return Heap::true_value(); } } @@ -2684,10 +2682,8 @@ static Object* Runtime_StringAdd(Arguments args) { CONVERT_CHECKED(String, str1, args[0]); CONVERT_CHECKED(String, str2, args[1]); - StringShape shape1(str1); - StringShape shape2(str2); - int len1 = str1->length(shape1); - int len2 = str2->length(shape2); + int len1 = str1->length(); + int len2 = str2->length(); if (len1 == 0) return str2; if (len2 == 0) return str1; int length_sum = len1 + len2; @@ -2697,7 +2693,7 @@ static Object* Runtime_StringAdd(Arguments args) { Top::context()->mark_out_of_memory(); return Failure::OutOfMemoryException(); } - return Heap::AllocateConsString(str1, shape1, str2, shape2); + return Heap::AllocateConsString(str1, str2); } diff --git a/test/cctest/test-strings.cc b/test/cctest/test-strings.cc index ec080e245..c5e73521b 100644 --- a/test/cctest/test-strings.cc +++ b/test/cctest/test-strings.cc @@ -215,10 +215,8 @@ static void TraverseFirst(Handle s1, Handle s2, int chars) { CHECK_EQ(c, buffer2.GetNext()); i++; } - StringShape shape1(*s1); - StringShape shape2(*s2); - s1->Get(shape1, s1->length(shape1) - 1); - s2->Get(shape2, s2->length(shape2) - 1); + s1->Get(StringShape(*s1), s1->length() - 1); + s2->Get(StringShape(*s2), s2->length() - 1); } @@ -251,12 +249,10 @@ TEST(Traverse) { printf("7\n"); Handle right_deep_slice = Factory::NewStringSlice(left_deep_asymmetric, - StringShape(*left_deep_asymmetric), left_deep_asymmetric->length() - 1050, left_deep_asymmetric->length() - 50); Handle left_deep_slice = Factory::NewStringSlice(right_deep_asymmetric, - StringShape(*right_deep_asymmetric), right_deep_asymmetric->length() - 1050, right_deep_asymmetric->length() - 50); printf("8\n"); @@ -283,7 +279,6 @@ static Handle SliceOf(Handle underlying) { int start = gen() % underlying->length(); int end = start + gen() % (underlying->length() - start); return Factory::NewStringSlice(underlying, - StringShape(*underlying), start, end); } -- 2.34.1