From 89ac41aff9de202024b62fc0c28392311fd880b7 Mon Sep 17 00:00:00 2001 From: "erik.corry@gmail.com" Date: Thu, 9 Oct 2008 11:26:37 +0000 Subject: [PATCH] If an allocation is so huge that we cannot code the size needed in the failure object then we just return an out of memory failure object (instead of a retry after GC failure object). Not all places that checked for retry-after-GC were able to handle an immediate out of memory failure. This fixes http://code.google.com/p/v8/issues/detail?id=70 Review URL: http://codereview.chromium.org/6340 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@477 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/builtins.cc | 13 ++++--- src/factory.cc | 10 +++--- src/handles.cc | 2 +- src/heap-inl.h | 8 ++++- src/jsregexp.cc | 94 ++++++++++++++++++++----------------------------- src/objects-inl.h | 6 ++++ src/objects.h | 1 + test/cctest/test-api.cc | 27 ++++++++++++++ 8 files changed, 95 insertions(+), 66 deletions(-) diff --git a/src/builtins.cc b/src/builtins.cc index 43049f4..6169411 100644 --- a/src/builtins.cc +++ b/src/builtins.cc @@ -674,10 +674,15 @@ void Builtins::Setup(bool create_heap_objects) { masm.GetCode(&desc); Code::Flags flags = functions[i].flags; Object* code = Heap::CreateCode(desc, NULL, flags); - if (code->IsRetryAfterGC()) { - CHECK(Heap::CollectGarbage(Failure::cast(code)->requested(), - Failure::cast(code)->allocation_space())); - code = Heap::CreateCode(desc, NULL, flags); + if (code->IsFailure()) { + if (code->IsRetryAfterGC()) { + CHECK(Heap::CollectGarbage(Failure::cast(code)->requested(), + Failure::cast(code)->allocation_space())); + code = Heap::CreateCode(desc, NULL, flags); + } + if (code->IsFailure()) { + v8::internal::V8::FatalProcessOutOfMemory("CreateCode"); + } } // Add any unresolved jumps or calls to the fixup list in the // bootstrapper. diff --git a/src/factory.cc b/src/factory.cc index 3167a71..2ef40c45 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -467,10 +467,12 @@ Handle Factory::CopyAppendProxyDescriptor( GC_GREEDY_CHECK(); CallbacksDescriptor desc(*key, *value, attributes); Object* obj = array->CopyInsert(&desc, REMOVE_TRANSITIONS); - if (obj->IsRetryAfterGC()) { - CALL_GC(obj); - CallbacksDescriptor desc(*key, *value, attributes); - obj = array->CopyInsert(&desc, REMOVE_TRANSITIONS); + if (obj->IsFailure()) { + if (obj->IsRetryAfterGC()) { + CALL_GC(obj); + CallbacksDescriptor desc(*key, *value, attributes); + obj = array->CopyInsert(&desc, REMOVE_TRANSITIONS); + } if (obj->IsFailure()) { // TODO(1181417): Fix this. V8::FatalProcessOutOfMemory("CopyAppendProxyDescriptor"); diff --git a/src/handles.cc b/src/handles.cc index 5b6bc12..4f7c0d1 100644 --- a/src/handles.cc +++ b/src/handles.cc @@ -128,7 +128,7 @@ void TransformToFastProperties(Handle object, void FlattenString(Handle string) { if (string->IsFlat()) return; - CALL_HEAP_FUNCTION_VOID(String::cast(*string)->Flatten()); + CALL_HEAP_FUNCTION_VOID(string->Flatten()); ASSERT(string->IsFlat()); } diff --git a/src/heap-inl.h b/src/heap-inl.h index ea6c480..c841464 100644 --- a/src/heap-inl.h +++ b/src/heap-inl.h @@ -183,6 +183,9 @@ OldSpace* Heap::TargetSpace(HeapObject* object) { return Handle(); \ } \ } else { \ + if (__object__->IsOutOfMemoryFailure()) { \ + v8::internal::V8::FatalProcessOutOfMemory("CALL_HEAP_FUNCTION"); \ + } \ return Handle(); \ } \ } \ @@ -211,7 +214,10 @@ OldSpace* Heap::TargetSpace(HeapObject* object) { return; \ } \ } else { \ - return; \ + if (__object__->IsOutOfMemoryFailure()) { \ + V8::FatalProcessOutOfMemory("Handles"); \ + } \ + UNREACHABLE(); \ } \ } diff --git a/src/jsregexp.cc b/src/jsregexp.cc index 4385112..f6b6836 100644 --- a/src/jsregexp.cc +++ b/src/jsregexp.cc @@ -216,41 +216,14 @@ Handle RegExpImpl::JsreCompile(Handle re, unsigned number_of_captures; const char* error_message = NULL; - malloc_failure = Failure::Exception(); - JscreRegExp* code; + JscreRegExp* code = NULL; FlattenString(pattern); - if (pattern->IsAsciiRepresentation()) { - Vector contents = pattern->ToAsciiVector(); - code = jsRegExpCompile(contents.start(), - contents.length(), - case_option, - multiline_option, - &number_of_captures, - &error_message, - &JSREMalloc, - &JSREFree); - } else { - Vector contents = pattern->ToUC16Vector(); - code = jsRegExpCompile(contents.start(), - contents.length(), - case_option, - multiline_option, - &number_of_captures, - &error_message, - &JSREMalloc, - &JSREFree); - } + bool first_time = true; - if (code == NULL && malloc_failure->IsRetryAfterGC()) { - // Performs a GC, then retries. - if (!Heap::CollectGarbage(malloc_failure->requested(), - malloc_failure->allocation_space())) { - // TODO(1181417): Fix this. - V8::FatalProcessOutOfMemory("RegExpImpl::JsreCompile"); - } + while (true) { + first_time = false; malloc_failure = Failure::Exception(); - if (pattern->IsAsciiRepresentation()) { Vector contents = pattern->ToAsciiVector(); code = jsRegExpCompile(contents.start(), @@ -272,36 +245,45 @@ Handle RegExpImpl::JsreCompile(Handle re, &JSREMalloc, &JSREFree); } - - if (code == NULL && malloc_failure->IsRetryAfterGC()) { - // TODO(1181417): Fix this. - V8::FatalProcessOutOfMemory("RegExpImpl::JsreCompile"); + if (code == NULL) { + if (first_time && malloc_failure->IsRetryAfterGC()) { + if (!Heap::CollectGarbage(malloc_failure->requested(), + malloc_failure->allocation_space())) { + // TODO(1181417): Fix this. + V8::FatalProcessOutOfMemory("RegExpImpl::JsreCompile"); + } + continue; + } + if (malloc_failure->IsRetryAfterGC() || + malloc_failure->IsOutOfMemoryFailure()) { + // TODO(1181417): Fix this. + V8::FatalProcessOutOfMemory("RegExpImpl::JsreCompile"); + } else { + // Throw an exception. + Handle array = Factory::NewJSArray(2); + SetElement(array, 0, pattern); + SetElement(array, 1, Factory::NewStringFromUtf8(CStrVector( + (error_message == NULL) ? "Unknown regexp error" : error_message))); + Handle regexp_err = + Factory::NewSyntaxError("malformed_regexp", array); + return Handle(Top::Throw(*regexp_err)); + } } - } - - if (error_message != NULL) { - // Throw an exception. - Handle array = Factory::NewJSArray(2); - SetElement(array, 0, pattern); - SetElement(array, 1, Factory::NewStringFromUtf8(CStrVector(error_message))); - Handle regexp_err = - Factory::NewSyntaxError("malformed_regexp", array); - return Handle(Top::Throw(*regexp_err)); - } - ASSERT(code != NULL); + ASSERT(code != NULL); - // Convert the return address to a ByteArray pointer. - Handle internal( - ByteArray::FromDataStartAddress(reinterpret_cast
(code))); + // Convert the return address to a ByteArray pointer. + Handle internal( + ByteArray::FromDataStartAddress(reinterpret_cast
(code))); - Handle value = Factory::NewFixedArray(2); - value->set(CAPTURE_INDEX, Smi::FromInt(number_of_captures)); - value->set(INTERNAL_INDEX, *internal); - re->set_type_tag(JSRegExp::JSCRE); - re->set_data(*value); + Handle value = Factory::NewFixedArray(2); + value->set(CAPTURE_INDEX, Smi::FromInt(number_of_captures)); + value->set(INTERNAL_INDEX, *internal); + re->set_type_tag(JSRegExp::JSCRE); + re->set_data(*value); - return re; + return re; + } } diff --git a/src/objects-inl.h b/src/objects-inl.h index c8bd70a..5f45c2f 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -211,6 +211,12 @@ bool Object::IsRetryAfterGC() { } +bool Object::IsOutOfMemoryFailure() { + return HAS_FAILURE_TAG(this) + && Failure::cast(this)->IsOutOfMemoryException(); +} + + bool Object::IsException() { return this == Failure::Exception(); } diff --git a/src/objects.h b/src/objects.h index d1da915..564601f 100644 --- a/src/objects.h +++ b/src/objects.h @@ -601,6 +601,7 @@ class Object BASE_EMBEDDED { inline bool IsByteArray(); inline bool IsFailure(); inline bool IsRetryAfterGC(); + inline bool IsOutOfMemoryFailure(); inline bool IsException(); inline bool IsJSObject(); inline bool IsMap(); diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index c45ccf8..7bc60bd 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -2341,6 +2341,33 @@ TEST(ErrorReporting) { } +static const char* js_code_causing_huge_string_flattening = + "var str = 'X';" + "for (var i = 0; i < 29; i++) {" + " str = str + str;" + "}" + "str.match(/X/);"; + + +void OOMCallback(const char* location, const char* message) { + exit(0); +} + + +TEST(RegexpOutOfMemory) { + // Execute a script that causes out of memory when flattening a string. + v8::HandleScope scope; + v8::V8::SetFatalErrorHandler(OOMCallback); + LocalContext context; + Local