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

@@ -739,7 +739,7 @@ static bool InstallFullCode(CompilationInfo* info) {
   Handle<ScopeInfo> 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());

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;

src/mark-compact.h

@@ -434,6 +434,7 @@ class CodeFlusher {
     }
   }
 
+  void EvictCandidate(SharedFunctionInfo* shared_info);
   void EvictCandidate(JSFunction* function);
 
   void ProcessCandidates() {

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<ScopeInfo*>(READ_FIELD(this, kScopeInfoOffset));
 }

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());
 }

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.

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(

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<SharedFunctionInfo> shared1;
+  {
+    HandleScope inner_scope;
+    CompileRun("function f() { return 'foobar'; }"
+               "function g(x) { if (x) f(); }"
+               "f();"
+               "g(false);"
+               "g(false);");
+
+    Handle<JSFunction> f =
+        v8::Utils::OpenHandle(
+            *v8::Handle<v8::Function>::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<MarkingParity>(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<SharedFunctionInfo> shared2;
+  {
+    HandleScope inner_scope;
+    CompileRun("function flushMe() { return 0; }"
+               "flushMe(1);");
+
+    Handle<JSFunction> f =
+        v8::Utils::OpenHandle(
+            *v8::Handle<v8::Function>::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<MarkingParity>(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);
+}