From 2f2670088531a1d87542743947ed0dd06ba820c0 Mon Sep 17 00:00:00 2001 From: "ulan@chromium.org" Date: Thu, 13 Mar 2014 14:09:18 +0000 Subject: [PATCH] Fix memory leak caused by treating Code::next_code_link as strong in marker. BUG= TEST=test/cctest/NextCodeLinkIsWeak R=mstarzinger@chromium.org Review URL: https://codereview.chromium.org/181833004 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19897 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/heap.cc | 1 + 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, 120 insertions(+), 17 deletions(-) diff --git a/src/heap.cc b/src/heap.cc index 25f712158e..9476d33a79 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -4085,6 +4085,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 810f25d439..9a0a1eafcb 100644 --- a/src/mark-compact.cc +++ b/src/mark-compact.cc @@ -1860,6 +1860,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 49fd9d4816..947eb66961 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -1392,6 +1392,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); } @@ -5685,6 +5690,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() { @@ -5713,20 +5719,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 db84664d42..8771564181 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 41e5fd6fd3..de8ca6d055 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 c7725788e8..c503e07ade 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1893,6 +1893,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); @@ -5229,7 +5231,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( @@ -5557,8 +5558,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; @@ -10703,6 +10704,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 07828a9a48..60e70fc0f6 100644 --- a/test/cctest/test-heap.cc +++ b/test/cctest/test-heap.cc @@ -3698,6 +3698,102 @@ TEST(ObjectsInOptimizedCodeAreWeak) { } + +static Handle OptimizeDummyFunction(const char* name) { + char source[256]; + snprintf(source, sizeof(source), + "function %s() { return 0; }" + "%s(); %s();" + "%%OptimizeFunctionOnNextCall(%s);" + "%s();", name, name, name, name, name); + CompileRun(source); + 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;