From b93b2b98b89509aa9039ffe600425d38e35c60bd Mon Sep 17 00:00:00 2001 From: "mstarzinger@chromium.org" Date: Fri, 11 Jan 2013 13:13:11 +0000 Subject: [PATCH] Fix shared function info code replacement. This fixes a corner case when the unoptimized code for a shared function info is replaced while the function is enqueued as a flushing candidate. Since the link field is stored within the code object, the candidates list got destroyed. R=hpayer@chromium.org BUG=v8:169209 TEST=cctest/test-heap/Regress169209 Review URL: https://codereview.chromium.org/11818052 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13361 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/compiler.cc | 2 +- src/mark-compact.cc | 33 +++++++++++++++++-- src/mark-compact.h | 1 + src/objects-inl.h | 13 ++++++++ src/objects.cc | 2 +- src/objects.h | 1 + src/runtime.cc | 2 +- test/cctest/test-heap.cc | 71 ++++++++++++++++++++++++++++++++++++++++ 8 files changed, 120 insertions(+), 5 deletions(-) diff --git a/src/compiler.cc b/src/compiler.cc index c4c8f9941b..daeca1e14d 100644 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -739,7 +739,7 @@ static bool InstallFullCode(CompilationInfo* info) { Handle scope_info = ScopeInfo::Create(info->scope(), info->zone()); shared->set_scope_info(*scope_info); - shared->set_code(*code); + shared->ReplaceCode(*code); if (!function.is_null()) { function->ReplaceCode(*code); ASSERT(!function->IsOptimized()); diff --git a/src/mark-compact.cc b/src/mark-compact.cc index 3ebdb1e6b7..d867197d7c 100644 --- a/src/mark-compact.cc +++ b/src/mark-compact.cc @@ -885,8 +885,8 @@ void CodeFlusher::ProcessJSFunctionCandidates() { if (!code_mark.Get()) { shared->set_code(lazy_compile); candidate->set_code(lazy_compile); - } else if (code == lazy_compile) { - candidate->set_code(lazy_compile); + } else { + candidate->set_code(code); } // We are in the middle of a GC cycle so the write barrier in the code @@ -935,6 +935,34 @@ void CodeFlusher::ProcessSharedFunctionInfoCandidates() { } +void CodeFlusher::EvictCandidate(SharedFunctionInfo* shared_info) { + // The function is no longer a candidate, make sure it gets visited + // again so that previous flushing decisions are revisited. + isolate_->heap()->incremental_marking()->RecordWrites(shared_info); + + SharedFunctionInfo* candidate = shared_function_info_candidates_head_; + SharedFunctionInfo* next_candidate; + if (candidate == shared_info) { + next_candidate = GetNextCandidate(shared_info); + shared_function_info_candidates_head_ = next_candidate; + ClearNextCandidate(shared_info); + } else { + while (candidate != NULL) { + next_candidate = GetNextCandidate(candidate); + + if (next_candidate == shared_info) { + next_candidate = GetNextCandidate(shared_info); + SetNextCandidate(candidate, next_candidate); + ClearNextCandidate(shared_info); + break; + } + + candidate = next_candidate; + } + } +} + + void CodeFlusher::EvictCandidate(JSFunction* function) { ASSERT(!function->next_function_link()->IsUndefined()); Object* undefined = isolate_->heap()->undefined_value(); @@ -957,6 +985,7 @@ void CodeFlusher::EvictCandidate(JSFunction* function) { next_candidate = GetNextCandidate(function); SetNextCandidate(candidate, next_candidate); ClearNextCandidate(function, undefined); + break; } candidate = next_candidate; diff --git a/src/mark-compact.h b/src/mark-compact.h index 9a0b014d63..8821c3df30 100644 --- a/src/mark-compact.h +++ b/src/mark-compact.h @@ -434,6 +434,7 @@ class CodeFlusher { } } + void EvictCandidate(SharedFunctionInfo* shared_info); void EvictCandidate(JSFunction* function); void ProcessCandidates() { diff --git a/src/objects-inl.h b/src/objects-inl.h index f21355ec4b..34983cd7b3 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -4380,6 +4380,19 @@ void SharedFunctionInfo::set_code(Code* value, WriteBarrierMode mode) { } +void SharedFunctionInfo::ReplaceCode(Code* value) { + // If the GC metadata field is already used then the function was + // enqueued as a code flushing candidate and we remove it now. + if (code()->gc_metadata() != NULL) { + CodeFlusher* flusher = GetHeap()->mark_compact_collector()->code_flusher(); + flusher->EvictCandidate(this); + } + + ASSERT(code()->gc_metadata() == NULL && value->gc_metadata() == NULL); + set_code(value); +} + + ScopeInfo* SharedFunctionInfo::scope_info() { return reinterpret_cast(READ_FIELD(this, kScopeInfoOffset)); } diff --git a/src/objects.cc b/src/objects.cc index 1d7ceb06d4..2f2bd64348 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -8292,7 +8292,7 @@ void SharedFunctionInfo::EnableDeoptimizationSupport(Code* recompiled) { // old code, we have to replace it. We should try to avoid this // altogether because it flushes valuable type feedback by // effectively resetting all IC state. - set_code(recompiled); + ReplaceCode(recompiled); } ASSERT(has_deoptimization_support()); } diff --git a/src/objects.h b/src/objects.h index 6b137cdd1c..8b3bbd4517 100644 --- a/src/objects.h +++ b/src/objects.h @@ -5393,6 +5393,7 @@ class SharedFunctionInfo: public HeapObject { // [code]: Function code. DECL_ACCESSORS(code, Code) + inline void ReplaceCode(Code* code); // [optimized_code_map]: Map from native context to optimized code // and a shared literals array or Smi 0 if none. diff --git a/src/runtime.cc b/src/runtime.cc index 0b630563b1..b508ad3e99 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -2147,7 +2147,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_SetCode) { // target function to undefined. SetCode is only used for built-in // constructors like String, Array, and Object, and some web code // doesn't like seeing source code for constructors. - target_shared->set_code(source_shared->code()); + target_shared->ReplaceCode(source_shared->code()); target_shared->set_scope_info(source_shared->scope_info()); target_shared->set_length(source_shared->length()); target_shared->set_formal_parameter_count( diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc index 58ff4606b8..834e75b1c3 100644 --- a/test/cctest/test-heap.cc +++ b/test/cctest/test-heap.cc @@ -2627,3 +2627,74 @@ TEST(Regress165495) { // Unoptimized code is missing and the deoptimizer will go ballistic. CompileRun("var g = mkClosure(); g('bozo');"); } + + +TEST(Regress169209) { + i::FLAG_allow_natives_syntax = true; + i::FLAG_flush_code_incrementally = true; + InitializeVM(); + v8::HandleScope scope; + + // Perform one initial GC to enable code flushing. + HEAP->CollectAllGarbage(Heap::kAbortIncrementalMarkingMask); + + // Prepare a shared function info eligible for code flushing for which + // the unoptimized code will be replaced during optimization. + Handle shared1; + { + HandleScope inner_scope; + CompileRun("function f() { return 'foobar'; }" + "function g(x) { if (x) f(); }" + "f();" + "g(false);" + "g(false);"); + + Handle f = + v8::Utils::OpenHandle( + *v8::Handle::Cast( + v8::Context::GetCurrent()->Global()->Get(v8_str("f")))); + CHECK(f->is_compiled()); + const int kAgingThreshold = 6; + for (int i = 0; i < kAgingThreshold; i++) { + f->shared()->code()->MakeOlder(static_cast(i % 2)); + } + + shared1 = inner_scope.CloseAndEscape(handle(f->shared(), ISOLATE)); + } + + // Prepare a shared function info eligible for code flushing that will + // represent the dangling tail of the candidate list. + Handle shared2; + { + HandleScope inner_scope; + CompileRun("function flushMe() { return 0; }" + "flushMe(1);"); + + Handle f = + v8::Utils::OpenHandle( + *v8::Handle::Cast( + v8::Context::GetCurrent()->Global()->Get(v8_str("flushMe")))); + CHECK(f->is_compiled()); + const int kAgingThreshold = 6; + for (int i = 0; i < kAgingThreshold; i++) { + f->shared()->code()->MakeOlder(static_cast(i % 2)); + } + + shared2 = inner_scope.CloseAndEscape(handle(f->shared(), ISOLATE)); + } + + // Simulate incremental marking and collect code flushing candidates. + SimulateIncrementalMarking(); + CHECK(shared1->code()->gc_metadata() != NULL); + + // Optimize function and make sure the unoptimized code is replaced. +#ifdef DEBUG + FLAG_stop_at = "f"; +#endif + CompileRun("%OptimizeFunctionOnNextCall(g);" + "g(false);"); + + // Finish garbage collection cycle. + HEAP->CollectAllGarbage(Heap::kNoGCFlags); + CHECK(shared1->code()->gc_metadata() == NULL); +}