From a7d1b658fc4f88094fe8c4f1f1407502e9115f6b Mon Sep 17 00:00:00 2001 From: "ulan@chromium.org" Date: Wed, 26 Mar 2014 15:14:51 +0000 Subject: [PATCH] Reland r19897 "Fix memory leak caused by treating Code::next_code_link as strong in marker. R=mstarzinger@chromium.org Review URL: https://codereview.chromium.org/212553003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20282 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/heap.cc | 19 ++++++++- src/mark-compact.cc | 4 ++ src/objects-inl.h | 20 +++------- src/objects-visiting-inl.h | 4 ++ src/objects-visiting.h | 2 + src/objects.h | 10 +++-- test/cctest/test-heap.cc | 96 ++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 137 insertions(+), 18 deletions(-) diff --git a/src/heap.cc b/src/heap.cc index 8d321ad..6374433 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -1775,6 +1775,18 @@ static Object* VisitWeakList(Heap* heap, } +template +static void ClearWeakList(Heap* heap, + Object* list) { + Object* undefined = heap->undefined_value(); + while (list != undefined) { + T* candidate = reinterpret_cast(list); + list = WeakListVisitor::WeakNext(candidate); + WeakListVisitor::SetWeakNext(candidate, undefined); + } +} + + template<> struct WeakListVisitor { static void SetWeakNext(JSFunction* function, Object* next) { @@ -1868,7 +1880,11 @@ struct WeakListVisitor { } } - static void VisitPhantomObject(Heap*, Context*) { + static void VisitPhantomObject(Heap* heap, Context* context) { + ClearWeakList(heap, + context->get(Context::OPTIMIZED_FUNCTIONS_LIST)); + ClearWeakList(heap, context->get(Context::OPTIMIZED_CODE_LIST)); + ClearWeakList(heap, context->get(Context::DEOPTIMIZED_CODE_LIST)); } static int WeakNextOffset() { @@ -4159,6 +4175,7 @@ MaybeObject* Heap::CreateCode(const CodeDesc& desc, code->set_is_crankshafted(crankshafted); code->set_deoptimization_data(empty_fixed_array(), SKIP_WRITE_BARRIER); code->set_raw_type_feedback_info(undefined_value()); + code->set_next_code_link(undefined_value()); code->set_handler_table(empty_fixed_array(), SKIP_WRITE_BARRIER); code->set_gc_metadata(Smi::FromInt(0)); code->set_ic_age(global_ic_age_); diff --git a/src/mark-compact.cc b/src/mark-compact.cc index 4eb178b..f04a8bc 100644 --- a/src/mark-compact.cc +++ b/src/mark-compact.cc @@ -1863,6 +1863,10 @@ class RootMarkingVisitor : public ObjectVisitor { for (Object** p = start; p < end; p++) MarkObjectByPointer(p); } + // Skip the weak next code link in a code object, which is visited in + // ProcessTopOptimizedFrame. + void VisitNextCodeLink(Object** p) { } + private: void MarkObjectByPointer(Object** p) { if (!(*p)->IsHeapObject()) return; diff --git a/src/objects-inl.h b/src/objects-inl.h index cad7b63..acfbb43 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -1393,6 +1393,11 @@ void HeapObject::IteratePointer(ObjectVisitor* v, int offset) { } +void HeapObject::IterateNextCodeLink(ObjectVisitor* v, int offset) { + v->VisitNextCodeLink(reinterpret_cast(FIELD_ADDR(this, offset))); +} + + double HeapNumber::value() { return READ_DOUBLE_FIELD(this, kValueOffset); } @@ -5787,6 +5792,7 @@ ACCESSORS(Code, relocation_info, ByteArray, kRelocationInfoOffset) ACCESSORS(Code, handler_table, FixedArray, kHandlerTableOffset) ACCESSORS(Code, deoptimization_data, FixedArray, kDeoptimizationDataOffset) ACCESSORS(Code, raw_type_feedback_info, Object, kTypeFeedbackInfoOffset) +ACCESSORS(Code, next_code_link, Object, kNextCodeLinkOffset) void Code::WipeOutHeader() { @@ -5815,20 +5821,6 @@ void Code::set_type_feedback_info(Object* value, WriteBarrierMode mode) { } -Object* Code::next_code_link() { - CHECK(kind() == OPTIMIZED_FUNCTION); - return raw_type_feedback_info(); -} - - -void Code::set_next_code_link(Object* value, WriteBarrierMode mode) { - CHECK(kind() == OPTIMIZED_FUNCTION); - set_raw_type_feedback_info(value); - CONDITIONAL_WRITE_BARRIER(GetHeap(), this, kTypeFeedbackInfoOffset, - value, mode); -} - - int Code::stub_info() { ASSERT(kind() == COMPARE_IC || kind() == COMPARE_NIL_IC || kind() == BINARY_OP_IC || kind() == LOAD_IC); diff --git a/src/objects-visiting-inl.h b/src/objects-visiting-inl.h index 4f14988..31117bb 100644 --- a/src/objects-visiting-inl.h +++ b/src/objects-visiting-inl.h @@ -899,6 +899,7 @@ void Code::CodeIterateBody(ObjectVisitor* v) { IteratePointer(v, kHandlerTableOffset); IteratePointer(v, kDeoptimizationDataOffset); IteratePointer(v, kTypeFeedbackInfoOffset); + IterateNextCodeLink(v, kNextCodeLinkOffset); IteratePointer(v, kConstantPoolOffset); RelocIterator it(this, mode_mask); @@ -933,6 +934,9 @@ void Code::CodeIterateBody(Heap* heap) { StaticVisitor::VisitPointer( heap, reinterpret_cast(this->address() + kTypeFeedbackInfoOffset)); + StaticVisitor::VisitNextCodeLink( + heap, + reinterpret_cast(this->address() + kNextCodeLinkOffset)); StaticVisitor::VisitPointer( heap, reinterpret_cast(this->address() + kConstantPoolOffset)); diff --git a/src/objects-visiting.h b/src/objects-visiting.h index 41e5fd6..de8ca6d 100644 --- a/src/objects-visiting.h +++ b/src/objects-visiting.h @@ -414,6 +414,8 @@ class StaticMarkingVisitor : public StaticVisitorBase { INLINE(static void VisitCodeAgeSequence(Heap* heap, RelocInfo* rinfo)); INLINE(static void VisitExternalReference(RelocInfo* rinfo)) { } INLINE(static void VisitRuntimeEntry(RelocInfo* rinfo)) { } + // Skip the weak next code link in a code object. + INLINE(static void VisitNextCodeLink(Heap* heap, Object** slot)) { } // TODO(mstarzinger): This should be made protected once refactoring is done. // Mark non-optimize code for functions inlined into the given optimized diff --git a/src/objects.h b/src/objects.h index 35fed0a..0e352a2 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1898,6 +1898,8 @@ class HeapObject: public Object { inline void IteratePointers(ObjectVisitor* v, int start, int end); // as above, for the single element at "offset" inline void IteratePointer(ObjectVisitor* v, int offset); + // as above, for the next code link of a code object. + inline void IterateNextCodeLink(ObjectVisitor* v, int offset); private: DISALLOW_IMPLICIT_CONSTRUCTORS(HeapObject); @@ -5249,7 +5251,6 @@ class Code: public HeapObject { // the kind of the code object. // FUNCTION => type feedback information. // STUB => various things, e.g. a SMI - // OPTIMIZED_FUNCTION => the next_code_link for optimized code list. DECL_ACCESSORS(raw_type_feedback_info, Object) inline Object* type_feedback_info(); inline void set_type_feedback_info( @@ -5582,8 +5583,8 @@ class Code: public HeapObject { kHandlerTableOffset + kPointerSize; static const int kTypeFeedbackInfoOffset = kDeoptimizationDataOffset + kPointerSize; - static const int kNextCodeLinkOffset = kTypeFeedbackInfoOffset; // Shared. - static const int kGCMetadataOffset = kTypeFeedbackInfoOffset + kPointerSize; + static const int kNextCodeLinkOffset = kTypeFeedbackInfoOffset + kPointerSize; + static const int kGCMetadataOffset = kNextCodeLinkOffset + kPointerSize; static const int kICAgeOffset = kGCMetadataOffset + kPointerSize; static const int kFlagsOffset = kICAgeOffset + kIntSize; @@ -10734,6 +10735,9 @@ class ObjectVisitor BASE_EMBEDDED { // Handy shorthand for visiting a single pointer. virtual void VisitPointer(Object** p) { VisitPointers(p, p + 1); } + // Visit weak next_code_link in Code object. + virtual void VisitNextCodeLink(Object** p) { VisitPointers(p, p + 1); } + // To allow lazy clearing of inline caches the visitor has // a rich interface for iterating over Code objects.. diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc index d67f18d..55bb466 100644 --- a/test/cctest/test-heap.cc +++ b/test/cctest/test-heap.cc @@ -3752,6 +3752,102 @@ TEST(ObjectsInOptimizedCodeAreWeak) { } + +static Handle OptimizeDummyFunction(const char* name) { + EmbeddedVector source; + OS::SNPrintF(source, + "function %s() { return 0; }" + "%s(); %s();" + "%%OptimizeFunctionOnNextCall(%s);" + "%s();", name, name, name, name, name); + CompileRun(source.start()); + Handle fun = + v8::Utils::OpenHandle( + *v8::Handle::Cast( + CcTest::global()->Get(v8_str(name)))); + return fun; +} + + +static int GetCodeChainLength(Code* code) { + int result = 0; + while (code->next_code_link()->IsCode()) { + result++; + code = Code::cast(code->next_code_link()); + } + return result; +} + + +TEST(NextCodeLinkIsWeak) { + i::FLAG_allow_natives_syntax = true; + CcTest::InitializeVM(); + Isolate* isolate = CcTest::i_isolate(); + v8::internal::Heap* heap = CcTest::heap(); + + if (!isolate->use_crankshaft()) return; + HandleScope outer_scope(heap->isolate()); + Handle code; + heap->CollectAllAvailableGarbage(); + int code_chain_length_before, code_chain_length_after; + { + HandleScope scope(heap->isolate()); + Handle mortal = OptimizeDummyFunction("mortal"); + Handle immortal = OptimizeDummyFunction("immortal"); + CHECK_EQ(immortal->code()->next_code_link(), mortal->code()); + code_chain_length_before = GetCodeChainLength(immortal->code()); + // Keep the immortal code and let the mortal code die. + code = scope.CloseAndEscape(Handle(immortal->code())); + CompileRun("mortal = null; immortal = null;"); + } + heap->CollectAllAvailableGarbage(); + // Now mortal code should be dead. + code_chain_length_after = GetCodeChainLength(*code); + CHECK_EQ(code_chain_length_before - 1, code_chain_length_after); +} + + +static Handle DummyOptimizedCode(Isolate* isolate) { + i::byte buffer[i::Assembler::kMinimalBufferSize]; + MacroAssembler masm(isolate, buffer, sizeof(buffer)); + CodeDesc desc; + masm.Prologue(BUILD_FUNCTION_FRAME); + masm.GetCode(&desc); + Handle undefined(isolate->heap()->undefined_value(), isolate); + Handle code = isolate->factory()->NewCode( + desc, Code::ComputeFlags(Code::OPTIMIZED_FUNCTION), undefined); + CHECK(code->IsCode()); + return code; +} + + +TEST(NextCodeLinkIsWeak2) { + i::FLAG_allow_natives_syntax = true; + CcTest::InitializeVM(); + Isolate* isolate = CcTest::i_isolate(); + v8::internal::Heap* heap = CcTest::heap(); + + if (!isolate->use_crankshaft()) return; + HandleScope outer_scope(heap->isolate()); + heap->CollectAllAvailableGarbage(); + Handle context(Context::cast(heap->native_contexts_list()), isolate); + Handle new_head; + Handle old_head(context->get(Context::OPTIMIZED_CODE_LIST), isolate); + { + HandleScope scope(heap->isolate()); + Handle immortal = DummyOptimizedCode(isolate); + Handle mortal = DummyOptimizedCode(isolate); + mortal->set_next_code_link(*old_head); + immortal->set_next_code_link(*mortal); + context->set(Context::OPTIMIZED_CODE_LIST, *immortal); + new_head = scope.CloseAndEscape(immortal); + } + heap->CollectAllAvailableGarbage(); + // Now mortal code should be dead. + CHECK_EQ(*old_head, new_head->next_code_link()); +} + + #ifdef DEBUG TEST(AddInstructionChangesNewSpacePromotion) { i::FLAG_allow_natives_syntax = true; -- 2.7.4