From 82f630a9f705d571bd46f5846bc724fe38c14361 Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Tue, 25 Mar 2014 09:09:24 +0000 Subject: [PATCH] Reland "No longer OOM on invalid string length." R=ishell@chromium.org Review URL: https://codereview.chromium.org/210683003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20225 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/api.cc | 22 +++++--- src/bootstrapper.cc | 10 +++- src/bootstrapper.h | 1 + src/contexts.cc | 2 +- src/debug.cc | 2 + src/factory.cc | 5 +- src/heap-inl.h | 4 +- src/heap.cc | 12 ++--- src/hydrogen-instructions.cc | 12 +++-- src/json-parser.h | 2 + src/json-stringifier.h | 6 ++- src/jsregexp.cc | 1 + src/liveedit.cc | 4 +- src/parser.cc | 12 +++-- src/runtime.cc | 50 ++++++++++-------- src/uri.h | 10 ++-- test/cctest/cctest.status | 1 + test/cctest/test-strings.cc | 59 ++++++++++++++++++++++ ...string-oom-replace-global-regexp-with-string.js | 16 +++++- tools/lexer-shell.cc | 2 + 20 files changed, 179 insertions(+), 54 deletions(-) diff --git a/src/api.cc b/src/api.cc index cbcbe5b..ef627d0 100644 --- a/src/api.cc +++ b/src/api.cc @@ -5403,6 +5403,8 @@ inline Local NewString(Isolate* v8_isolate, if (length == -1) length = StringLength(data); i::Handle result = NewString( isolate->factory(), type, i::Vector(data, length)); + // We do not expect this to fail. Change this if it does. + CHECK(!result.is_null()); if (type == String::kUndetectableString) { result->MarkAsUndetectable(); } @@ -5460,8 +5462,8 @@ Local v8::String::Concat(Handle left, Handle right) { i::Handle right_string = Utils::OpenHandle(*right); i::Handle result = isolate->factory()->NewConsString(left_string, right_string); - // We do not expect this to throw an exception. Change this if it does. - CHECK_NOT_EMPTY_HANDLE(isolate, result); + // We do not expect this to fail. Change this if it does. + CHECK(!result.is_null()); return Utils::ToLocal(result); } @@ -5469,14 +5471,22 @@ Local v8::String::Concat(Handle left, Handle right) { static i::Handle NewExternalStringHandle( i::Isolate* isolate, v8::String::ExternalStringResource* resource) { - return isolate->factory()->NewExternalStringFromTwoByte(resource); + i::Handle result = + isolate->factory()->NewExternalStringFromTwoByte(resource); + // We do not expect this to fail. Change this if it does. + CHECK(!result.is_null()); + return result; } static i::Handle NewExternalAsciiStringHandle( i::Isolate* isolate, v8::String::ExternalAsciiStringResource* resource) { - return isolate->factory()->NewExternalStringFromAscii(resource); + i::Handle result = + isolate->factory()->NewExternalStringFromAscii(resource); + // We do not expect this to fail. Change this if it does. + CHECK(!result.is_null()); + return result; } @@ -7000,8 +7010,8 @@ Handle CpuProfileNode::GetFunctionName() const { i::Handle cons = isolate->factory()->NewConsString( isolate->factory()->InternalizeUtf8String(entry->name_prefix()), isolate->factory()->InternalizeUtf8String(entry->name())); - // We do not expect this to throw an exception. Change this if it does. - CHECK_NOT_EMPTY_HANDLE(isolate, cons); + // We do not expect this to fail. Change this if it does. + CHECK(!cons.is_null()); return ToApiHandle(cons); } } diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index c466e7e..b11fd01 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -88,6 +88,8 @@ Handle Bootstrapper::NativesSourceLookup(int index) { source.length()); Handle source_code = isolate_->factory()->NewExternalStringFromAscii(resource); + // We do not expect this to throw an exception. Change this if it does. + CHECK_NOT_EMPTY_HANDLE(isolate_, source_code); heap->natives_source_cache()->set(index, *source_code); } Handle cached_source(heap->natives_source_cache()->get(index), @@ -1462,6 +1464,7 @@ bool Genesis::CompileExperimentalBuiltin(Isolate* isolate, int index) { Handle source_code = factory->NewStringFromAscii( ExperimentalNatives::GetRawScriptSource(index)); + RETURN_IF_EMPTY_HANDLE_VALUE(isolate, source_code, false); return CompileNative(isolate, name, source_code); } @@ -1511,6 +1514,7 @@ bool Genesis::CompileScriptCached(Isolate* isolate, if (cache == NULL || !cache->Lookup(name, &function_info)) { ASSERT(source->IsOneByteRepresentation()); Handle script_name = factory->NewStringFromUtf8(name); + ASSERT(!script_name.is_null()); function_info = Compiler::CompileScript( source, script_name, @@ -2078,8 +2082,10 @@ static Handle ResolveBuiltinIdHolder( ASSERT_EQ(".prototype", period_pos); Vector property(holder_expr, static_cast(period_pos - holder_expr)); + Handle property_string = factory->InternalizeUtf8String(property); + ASSERT(!property_string.is_null()); Handle function = Handle::cast( - GetProperty(isolate, global, factory->InternalizeUtf8String(property))); + GetProperty(isolate, global, property_string)); return Handle(JSObject::cast(function->prototype())); } @@ -2347,6 +2353,8 @@ bool Genesis::InstallExtension(Isolate* isolate, } Handle source_code = isolate->factory()->NewExternalStringFromAscii(extension->source()); + // We do not expect this to throw an exception. Change this if it does. + CHECK_NOT_EMPTY_HANDLE(isolate, source_code); bool result = CompileScriptCached(isolate, CStrVector(extension->name()), source_code, diff --git a/src/bootstrapper.h b/src/bootstrapper.h index 14dd1bd..e683a45 100644 --- a/src/bootstrapper.h +++ b/src/bootstrapper.h @@ -73,6 +73,7 @@ class SourceCodeCache BASE_EMBEDDED { cache_->CopyTo(0, *new_array, 0, cache_->length()); cache_ = *new_array; Handle str = factory->NewStringFromAscii(name, TENURED); + ASSERT(!str.is_null()); cache_->set(length, *str); cache_->set(length + 1, *shared); Script::cast(shared->script())->set_type(Smi::FromInt(type_)); diff --git a/src/contexts.cc b/src/contexts.cc index ada6e04..33d47e9 100644 --- a/src/contexts.cc +++ b/src/contexts.cc @@ -368,7 +368,7 @@ Handle Context::ErrorMessageForCodeGenerationFromStrings() { Handle result(error_message_for_code_gen_from_strings(), GetIsolate()); if (!result->IsUndefined()) return result; - return GetIsolate()->factory()->NewStringFromAscii(i::CStrVector( + return GetIsolate()->factory()->NewStringFromOneByte(STATIC_ASCII_VECTOR( "Code generation from strings disallowed for this context")); } diff --git a/src/debug.cc b/src/debug.cc index c8a99d5..d7667f1 100644 --- a/src/debug.cc +++ b/src/debug.cc @@ -754,6 +754,7 @@ bool Debug::CompileDebuggerScript(Isolate* isolate, int index) { isolate->bootstrapper()->NativesSourceLookup(index); Vector name = Natives::GetScriptName(index); Handle script_name = factory->NewStringFromAscii(name); + ASSERT(!script_name.is_null()); Handle context = isolate->native_context(); // Compile the script. @@ -2599,6 +2600,7 @@ Handle Debugger::MakeJSObject(Vector constructor_name, // Create the execution state object. Handle constructor_str = isolate_->factory()->InternalizeUtf8String(constructor_name); + ASSERT(!constructor_str.is_null()); Handle constructor( isolate_->global_object()->GetPropertyNoExceptionThrown(*constructor_str), isolate_); diff --git a/src/factory.cc b/src/factory.cc index 5e66202..5c4ac9c 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -289,7 +289,7 @@ Handle Factory::NewStringFromTwoByte(Vector string, Handle Factory::NewRawOneByteString(int length, - PretenureFlag pretenure) { + PretenureFlag pretenure) { CALL_HEAP_FUNCTION( isolate(), isolate()->heap()->AllocateRawOneByteString(length, pretenure), @@ -411,6 +411,7 @@ Handle Factory::NewConsString(Handle left, ASSERT(left->IsFlat()); ASSERT(right->IsFlat()); + STATIC_ASSERT(ConsString::kMinLength <= String::kMaxLength); if (is_one_byte) { Handle result = NewRawOneByteString(length); DisallowHeapAllocation no_gc; @@ -496,12 +497,14 @@ Handle Factory::NewProperSubString(Handle str, if (!FLAG_string_slices || length < SlicedString::kMinLength) { if (str->IsOneByteRepresentation()) { Handle result = NewRawOneByteString(length); + ASSERT(!result.is_null()); uint8_t* dest = result->GetChars(); DisallowHeapAllocation no_gc; String::WriteToFlat(*str, dest, begin, end); return result; } else { Handle result = NewRawTwoByteString(length); + ASSERT(!result.is_null()); uc16* dest = result->GetChars(); DisallowHeapAllocation no_gc; String::WriteToFlat(*str, dest, begin, end); diff --git a/src/heap-inl.h b/src/heap-inl.h index 7e465d5..063cf30 100644 --- a/src/heap-inl.h +++ b/src/heap-inl.h @@ -138,7 +138,7 @@ MaybeObject* Heap::AllocateInternalizedStringImpl( MaybeObject* Heap::AllocateOneByteInternalizedString(Vector str, uint32_t hash_field) { if (str.length() > String::kMaxLength) { - v8::internal::Heap::FatalProcessOutOfMemory("invalid string length", true); + return isolate()->ThrowInvalidStringLength(); } // Compute map and object size. Map* map = ascii_internalized_string_map(); @@ -171,7 +171,7 @@ MaybeObject* Heap::AllocateOneByteInternalizedString(Vector str, MaybeObject* Heap::AllocateTwoByteInternalizedString(Vector str, uint32_t hash_field) { if (str.length() > String::kMaxLength) { - v8::internal::Heap::FatalProcessOutOfMemory("invalid string length", true); + return isolate()->ThrowInvalidStringLength(); } // Compute map and object size. Map* map = internalized_string_map(); diff --git a/src/heap.cc b/src/heap.cc index e785629..16b21ea 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -3866,7 +3866,7 @@ MaybeObject* Heap::AllocateExternalStringFromAscii( const ExternalAsciiString::Resource* resource) { size_t length = resource->length(); if (length > static_cast(String::kMaxLength)) { - v8::internal::Heap::FatalProcessOutOfMemory("invalid string length", true); + return isolate()->ThrowInvalidStringLength(); } Map* map = external_ascii_string_map(); @@ -3888,7 +3888,7 @@ MaybeObject* Heap::AllocateExternalStringFromTwoByte( const ExternalTwoByteString::Resource* resource) { size_t length = resource->length(); if (length > static_cast(String::kMaxLength)) { - v8::internal::Heap::FatalProcessOutOfMemory("invalid string length", true); + return isolate()->ThrowInvalidStringLength(); } // For small strings we check whether the resource contains only @@ -4973,8 +4973,8 @@ MaybeObject* Heap::AllocateInternalizedStringImpl( int size; Map* map; - if (chars > String::kMaxLength) { - v8::internal::Heap::FatalProcessOutOfMemory("invalid string length", true); + if (chars < 0 || chars > String::kMaxLength) { + return isolate()->ThrowInvalidStringLength(); } if (is_one_byte) { map = ascii_internalized_string_map(); @@ -5022,7 +5022,7 @@ MaybeObject* Heap::AllocateInternalizedStringImpl( MaybeObject* Heap::AllocateRawOneByteString(int length, PretenureFlag pretenure) { if (length < 0 || length > String::kMaxLength) { - v8::internal::Heap::FatalProcessOutOfMemory("invalid string length", true); + return isolate()->ThrowInvalidStringLength(); } int size = SeqOneByteString::SizeFor(length); ASSERT(size <= SeqOneByteString::kMaxSize); @@ -5046,7 +5046,7 @@ MaybeObject* Heap::AllocateRawOneByteString(int length, MaybeObject* Heap::AllocateRawTwoByteString(int length, PretenureFlag pretenure) { if (length < 0 || length > String::kMaxLength) { - v8::internal::Heap::FatalProcessOutOfMemory("invalid string length", true); + return isolate()->ThrowInvalidStringLength(); } int size = SeqTwoByteString::SizeFor(length); ASSERT(size <= SeqTwoByteString::kMaxSize); diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index 1919490..b822d84 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -4024,9 +4024,15 @@ HInstruction* HStringAdd::New(Zone* zone, HConstant* c_right = HConstant::cast(right); HConstant* c_left = HConstant::cast(left); if (c_left->HasStringValue() && c_right->HasStringValue()) { - Handle concat = zone->isolate()->factory()->NewFlatConcatString( - c_left->StringValue(), c_right->StringValue()); - return HConstant::New(zone, context, concat); + Handle left_string = c_left->StringValue(); + Handle right_string = c_right->StringValue(); + // Prevent possible exception by invalid string length. + if (left_string->length() + right_string->length() < String::kMaxLength) { + Handle concat = zone->isolate()->factory()->NewFlatConcatString( + c_left->StringValue(), c_right->StringValue()); + ASSERT(!concat.is_null()); + return HConstant::New(zone, context, concat); + } } } return new(zone) HStringAdd( diff --git a/src/json-parser.h b/src/json-parser.h index 0973589..4c2b479 100644 --- a/src/json-parser.h +++ b/src/json-parser.h @@ -606,6 +606,7 @@ Handle JsonParser::SlowScanJsonString( int length = Min(max_length, Max(kInitialSpecialStringLength, 2 * count)); Handle seq_string = NewRawString(factory(), length, pretenure_); + ASSERT(!seq_string.is_null()); // Copy prefix into seq_str. SinkChar* dest = seq_string->GetChars(); String::WriteToFlat(*prefix, dest, start, end); @@ -793,6 +794,7 @@ Handle JsonParser::ScanJsonString() { } while (c0_ != '"'); int length = position_ - beg_pos; Handle result = factory()->NewRawOneByteString(length, pretenure_); + ASSERT(!result.is_null()); uint8_t* dest = SeqOneByteString::cast(*result)->GetChars(); String::WriteToFlat(*source_, dest, beg_pos, position_); diff --git a/src/json-stringifier.h b/src/json-stringifier.h index a75b3de..c063b67 100644 --- a/src/json-stringifier.h +++ b/src/json-stringifier.h @@ -266,6 +266,7 @@ BasicJsonStringifier::BasicJsonStringifier(Isolate* isolate) factory_->ToObject(factory_->empty_string())); part_length_ = kInitialPartLength; current_part_ = factory_->NewRawOneByteString(part_length_); + ASSERT(!current_part_.is_null()); tojson_string_ = factory_->toJSON_string(); stack_ = factory_->NewJSArray(8); } @@ -309,6 +310,7 @@ MaybeObject* BasicJsonStringifier::StringifyString(Isolate* isolate, if (object->IsOneByteRepresentationUnderneath()) { Handle result = isolate->factory()->NewRawOneByteString(worst_case_length); + ASSERT(!result.is_null()); DisallowHeapAllocation no_gc; return StringifyString_( isolate, @@ -317,6 +319,7 @@ MaybeObject* BasicJsonStringifier::StringifyString(Isolate* isolate, } else { Handle result = isolate->factory()->NewRawTwoByteString(worst_case_length); + ASSERT(!result.is_null()); DisallowHeapAllocation no_gc; return StringifyString_( isolate, @@ -722,7 +725,6 @@ void BasicJsonStringifier::ShrinkCurrentPart() { void BasicJsonStringifier::Accumulate() { if (accumulator()->length() + current_part_->length() > String::kMaxLength) { // Screw it. Simply set the flag and carry on. Throw exception at the end. - // We most likely will trigger a real OOM before even reaching this point. set_accumulator(factory_->empty_string()); overflowed_ = true; } else { @@ -741,6 +743,7 @@ void BasicJsonStringifier::Extend() { } else { current_part_ = factory_->NewRawTwoByteString(part_length_); } + ASSERT(!current_part_.is_null()); current_index_ = 0; } @@ -749,6 +752,7 @@ void BasicJsonStringifier::ChangeEncoding() { ShrinkCurrentPart(); Accumulate(); current_part_ = factory_->NewRawTwoByteString(part_length_); + ASSERT(!current_part_.is_null()); current_index_ = 0; is_ascii_ = false; } diff --git a/src/jsregexp.cc b/src/jsregexp.cc index 012c251..830dd7b 100644 --- a/src/jsregexp.cc +++ b/src/jsregexp.cc @@ -466,6 +466,7 @@ bool RegExpImpl::CompileIrregexp(Handle re, // Unable to compile regexp. Handle error_message = isolate->factory()->NewStringFromUtf8(CStrVector(result.error_message)); + ASSERT(!error_message.is_null()); CreateRegExpErrorObjectAndThrow(re, is_ascii, error_message, isolate); return false; } diff --git a/src/liveedit.cc b/src/liveedit.cc index a812b75..5eae107 100644 --- a/src/liveedit.cc +++ b/src/liveedit.cc @@ -2013,8 +2013,8 @@ Handle LiveEdit::CheckAndDropActivations( DropActivationsInActiveThread(shared_info_array, result, do_drop); if (error_message != NULL) { // Add error message as an array extra element. - Vector vector_message(error_message, StrLength(error_message)); - Handle str = isolate->factory()->NewStringFromAscii(vector_message); + Handle str = isolate->factory()->NewStringFromAscii( + CStrVector(error_message)); SetElementSloppy(result, len, str); } return result; diff --git a/src/parser.cc b/src/parser.cc index 56eec54..c7ea634 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -215,6 +215,7 @@ Handle Parser::LookupCachedSymbol(int symbol_id) { Handle result = symbol_cache_.at(symbol_id); if (result.is_null()) { result = scanner()->AllocateInternalizedString(isolate_); + ASSERT(!result.is_null()); symbol_cache_.at(symbol_id) = result; return result; } @@ -615,6 +616,7 @@ void ParserTraits::ReportMessageAt(Scanner::Location source_location, Handle elements = factory->NewFixedArray(args.length()); for (int i = 0; i < args.length(); i++) { Handle arg_string = factory->NewStringFromUtf8(CStrVector(args[i])); + ASSERT(!arg_string.is_null()); elements->set(i, *arg_string); } Handle array = factory->NewJSArrayWithElements(elements); @@ -672,7 +674,10 @@ Handle ParserTraits::GetSymbol(Scanner* scanner) { parser_->scanner()->LogSymbol(parser_->log_, parser_->position()); } } - return parser_->scanner()->AllocateInternalizedString(parser_->isolate_); + Handle result = + parser_->scanner()->AllocateInternalizedString(parser_->isolate_); + ASSERT(!result.is_null()); + return result; } @@ -1709,8 +1714,8 @@ void Parser::Declare(Declaration* declaration, bool resolve, bool* ok) { return; } Handle message_string = - isolate()->factory()->NewStringFromUtf8(CStrVector("Variable"), - TENURED); + isolate()->factory()->InternalizeOneByteString( + STATIC_ASCII_VECTOR("Variable")); Expression* expression = NewThrowTypeError(isolate()->factory()->redeclaration_string(), message_string, name); @@ -3816,6 +3821,7 @@ bool RegExpParser::simple() { RegExpTree* RegExpParser::ReportError(Vector message) { failed_ = true; *error_ = isolate()->factory()->NewStringFromAscii(message, NOT_TENURED); + ASSERT(!error_->is_null()); // Zip to the end to make sure the no more input is read. current_ = kEndMarker; next_pos_ = in()->length(); diff --git a/src/runtime.cc b/src/runtime.cc index 8d98f27..d7eada1 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -3363,8 +3363,7 @@ class ReplacementStringBuilder { array_builder_(heap->isolate(), estimated_part_count), subject_(subject), character_count_(0), - is_ascii_(subject->IsOneByteRepresentation()), - overflowed_(false) { + is_ascii_(subject->IsOneByteRepresentation()) { // Require a non-zero initial size. Ensures that doubling the size to // extend the array will work. ASSERT(estimated_part_count > 0); @@ -3412,11 +3411,6 @@ class ReplacementStringBuilder { Handle ToString() { - if (overflowed_) { - heap_->isolate()->ThrowInvalidStringLength(); - return Handle(); - } - if (array_builder_.length() == 0) { return heap_->isolate()->factory()->empty_string(); } @@ -3424,6 +3418,7 @@ class ReplacementStringBuilder { Handle joined_string; if (is_ascii_) { Handle seq = NewRawOneByteString(character_count_); + RETURN_IF_EMPTY_HANDLE_VALUE(heap_->isolate(), seq, Handle()); DisallowHeapAllocation no_gc; uint8_t* char_buffer = seq->GetChars(); StringBuilderConcatHelper(*subject_, @@ -3434,6 +3429,7 @@ class ReplacementStringBuilder { } else { // Non-ASCII. Handle seq = NewRawTwoByteString(character_count_); + RETURN_IF_EMPTY_HANDLE_VALUE(heap_->isolate(), seq, Handle()); DisallowHeapAllocation no_gc; uc16* char_buffer = seq->GetChars(); StringBuilderConcatHelper(*subject_, @@ -3448,9 +3444,11 @@ class ReplacementStringBuilder { void IncrementCharacterCount(int by) { if (character_count_ > String::kMaxLength - by) { - overflowed_ = true; + STATIC_ASSERT(String::kMaxLength < kMaxInt); + character_count_ = kMaxInt; + } else { + character_count_ += by; } - character_count_ += by; } private: @@ -3475,7 +3473,6 @@ class ReplacementStringBuilder { Handle subject_; int character_count_; bool is_ascii_; - bool overflowed_; }; @@ -3932,22 +3929,25 @@ MUST_USE_RESULT static MaybeObject* StringReplaceGlobalAtomRegExpWithString( static_cast(pattern_len)) * static_cast(matches) + static_cast(subject_len); - if (result_len_64 > INT_MAX) { - v8::internal::Heap::FatalProcessOutOfMemory("invalid string length", true); + int result_len; + if (result_len_64 > static_cast(String::kMaxLength)) { + STATIC_ASSERT(String::kMaxLength < kMaxInt); + result_len = kMaxInt; // Provoke exception. + } else { + result_len = static_cast(result_len_64); } - int result_len = static_cast(result_len_64); int subject_pos = 0; int result_pos = 0; - Handle result; + Handle result_seq; if (ResultSeqString::kHasAsciiEncoding) { - result = Handle::cast( - isolate->factory()->NewRawOneByteString(result_len)); + result_seq = isolate->factory()->NewRawOneByteString(result_len); } else { - result = Handle::cast( - isolate->factory()->NewRawTwoByteString(result_len)); + result_seq = isolate->factory()->NewRawTwoByteString(result_len); } + RETURN_IF_EMPTY_HANDLE(isolate, result_seq); + Handle result = Handle::cast(result_seq); for (int i = 0; i < matches; i++) { // Copy non-matched subject content. @@ -4127,6 +4127,7 @@ MUST_USE_RESULT static MaybeObject* StringReplaceGlobalRegExpWithEmptyString( answer = Handle::cast( isolate->factory()->NewRawTwoByteString(new_length)); } + ASSERT(!answer.is_null()); int prev = 0; int position = 0; @@ -6584,7 +6585,7 @@ MUST_USE_RESULT static MaybeObject* ConvertCase( if (s->IsOneByteRepresentationUnderneath()) { Handle result = isolate->factory()->NewRawOneByteString(length); - + ASSERT(!result.is_null()); // Same length as input. DisallowHeapAllocation no_gc; String::FlatContent flat_content = s->GetFlatContent(); ASSERT(flat_content.IsFlat()); @@ -6604,6 +6605,8 @@ MUST_USE_RESULT static MaybeObject* ConvertCase( } else { result = isolate->factory()->NewRawTwoByteString(length); } + ASSERT(!result.is_null()); // Same length as input. + MaybeObject* maybe = ConvertCaseHelper(isolate, *s, *result, length, mapping); Object* answer; if (!maybe->ToObject(&answer)) return maybe; @@ -6617,6 +6620,7 @@ MUST_USE_RESULT static MaybeObject* ConvertCase( if (length < 0) length = -length; result = isolate->factory()->NewRawTwoByteString(length); } + RETURN_IF_EMPTY_HANDLE(isolate, result); return ConvertCaseHelper(isolate, *s, *result, length, mapping); } @@ -7261,13 +7265,16 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_StringBuilderJoin) { String* element = String::cast(element_obj); int increment = element->length(); if (increment > String::kMaxLength - length) { - return isolate->ThrowInvalidStringLength(); + STATIC_ASSERT(String::kMaxLength < kMaxInt); + length = kMaxInt; // Provoke exception; + break; } length += increment; } Handle answer = isolate->factory()->NewRawTwoByteString(length); + RETURN_IF_EMPTY_HANDLE(isolate, answer); DisallowHeapAllocation no_gc; @@ -9512,8 +9519,9 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_ThrowMessage) { CONVERT_SMI_ARG_CHECKED(message_id, 0); const char* message = GetBailoutReason( static_cast(message_id)); - Handle message_handle = + Handle message_handle = isolate->factory()->NewStringFromAscii(CStrVector(message)); + RETURN_IF_EMPTY_HANDLE(isolate, message_handle); return isolate->Throw(*message_handle); } diff --git a/src/uri.h b/src/uri.h index 81ec0c5..1e73ddd 100644 --- a/src/uri.h +++ b/src/uri.h @@ -127,9 +127,11 @@ Handle URIUnescape::UnescapeSlow( int dest_position = 0; Handle second_part; + ASSERT(unescaped_length <= String::kMaxLength); if (one_byte) { Handle dest = isolate->factory()->NewRawOneByteString(unescaped_length); + ASSERT(!dest.is_null()); DisallowHeapAllocation no_allocation; Vector vector = GetCharVector(string); for (int i = start_index; i < length; dest_position++) { @@ -142,6 +144,7 @@ Handle URIUnescape::UnescapeSlow( } else { Handle dest = isolate->factory()->NewRawTwoByteString(unescaped_length); + ASSERT(!dest.is_null()); DisallowHeapAllocation no_allocation; Vector vector = GetCharVector(string); for (int i = start_index; i < length; dest_position++) { @@ -263,11 +266,7 @@ Handle URIEscape::Escape(Isolate* isolate, Handle string) { // We don't allow strings that are longer than a maximal length. ASSERT(String::kMaxLength < 0x7fffffff - 6); // Cannot overflow. - if (escaped_length > String::kMaxLength) { - AllowHeapAllocation allocate_error_and_return; - isolate->ThrowInvalidStringLength(); - return Handle::null(); - } + if (escaped_length > String::kMaxLength) break; // Provoke exception. } } @@ -276,6 +275,7 @@ Handle URIEscape::Escape(Isolate* isolate, Handle string) { Handle dest = isolate->factory()->NewRawOneByteString(escaped_length); + RETURN_IF_EMPTY_HANDLE_VALUE(isolate, dest, Handle()); int dest_position = 0; { DisallowHeapAllocation no_allocation; diff --git a/test/cctest/cctest.status b/test/cctest/cctest.status index 182a35b..2f09743 100644 --- a/test/cctest/cctest.status +++ b/test/cctest/cctest.status @@ -74,6 +74,7 @@ 'test-api/Threading2': [PASS, ['mode == debug', SLOW]], 'test-api/Threading3': [PASS, ['mode == debug', SLOW]], 'test-api/Threading4': [PASS, ['mode == debug', SLOW]], + 'test-strings/StringOOM*': [PASS, ['mode == debug', SKIP]], }], # ALWAYS ############################################################################## diff --git a/test/cctest/test-strings.cc b/test/cctest/test-strings.cc index 0f37c3e..6ff5200 100644 --- a/test/cctest/test-strings.cc +++ b/test/cctest/test-strings.cc @@ -1352,3 +1352,62 @@ TEST(Latin1IgnoreCase) { CHECK_EQ(Min(upper, lower), test); } } + + +class DummyResource: public v8::String::ExternalStringResource { + public: + virtual const uint16_t* data() const { return NULL; } + virtual size_t length() const { return 1 << 30; } +}; + + +class DummyOneByteResource: public v8::String::ExternalOneByteStringResource { + public: + virtual const char* data() const { return NULL; } + virtual size_t length() const { return 1 << 30; } +}; + + +TEST(InvalidExternalString) { + CcTest::InitializeVM(); + LocalContext context; + Isolate* isolate = CcTest::i_isolate(); + { HandleScope scope(isolate); + DummyOneByteResource r; + CHECK(isolate->factory()->NewExternalStringFromAscii(&r).is_null()); + CHECK(isolate->has_pending_exception()); + isolate->clear_pending_exception(); + } + + { HandleScope scope(isolate); + DummyResource r; + CHECK(isolate->factory()->NewExternalStringFromTwoByte(&r).is_null()); + CHECK(isolate->has_pending_exception()); + isolate->clear_pending_exception(); + } +} + + +#define INVALID_STRING_TEST(FUN, TYPE) \ + TEST(StringOOM##FUN) { \ + CcTest::InitializeVM(); \ + LocalContext context; \ + Isolate* isolate = CcTest::i_isolate(); \ + STATIC_ASSERT(String::kMaxLength < kMaxInt); \ + static const int invalid = String::kMaxLength + 1; \ + HandleScope scope(isolate); \ + Vector dummy = Vector::New(invalid); \ + CHECK(isolate->factory()->FUN(Vector::cast(dummy)).is_null()); \ + memset(dummy.start(), 0x20, dummy.length() * sizeof(TYPE)); \ + CHECK(isolate->has_pending_exception()); \ + isolate->clear_pending_exception(); \ + dummy.Dispose(); \ + } + +INVALID_STRING_TEST(NewStringFromAscii, char) +INVALID_STRING_TEST(NewStringFromUtf8, char) +INVALID_STRING_TEST(NewStringFromOneByte, uint8_t) +INVALID_STRING_TEST(InternalizeOneByteString, uint8_t) +INVALID_STRING_TEST(InternalizeUtf8String, char) + +#undef INVALID_STRING_TEST diff --git a/test/mjsunit/string-oom-replace-global-regexp-with-string.js b/test/mjsunit/string-oom-replace-global-regexp-with-string.js index a527ae6..2de0110 100644 --- a/test/mjsunit/string-oom-replace-global-regexp-with-string.js +++ b/test/mjsunit/string-oom-replace-global-regexp-with-string.js @@ -2,13 +2,25 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. + + var a = 'a'; for (var i = 0; i < 5; i++) a += a; var b = 'b'; for (var i = 0; i < 23; i++) b += b; -function replace() { +function replace1() { a.replace(/./g, b); } -assertThrows(replace, RangeError); +assertThrows(replace1, RangeError); + + +var a = 'a'; +for (var i = 0; i < 16; i++) a += a; + +function replace2() { + a.replace(/a/g, a); +} + +assertThrows(replace2, RangeError); diff --git a/tools/lexer-shell.cc b/tools/lexer-shell.cc index 8c7debc..e2e4a9c 100644 --- a/tools/lexer-shell.cc +++ b/tools/lexer-shell.cc @@ -68,6 +68,7 @@ class BaselineScanner { Vector( reinterpret_cast(source_), length / 2)); + CHECK_NOT_EMPTY_HANDLE(isolate, result); stream_ = new GenericStringUtf16CharacterStream(result, 0, result->length()); break; @@ -75,6 +76,7 @@ class BaselineScanner { case LATIN1: { Handle result = isolate->factory()->NewStringFromOneByte( Vector(source_, length)); + CHECK_NOT_EMPTY_HANDLE(isolate, result); stream_ = new GenericStringUtf16CharacterStream(result, 0, result->length()); break; -- 2.7.4