When preparing heap for breakpoints make sure not to recompile inlined functions.

src/debug.cc

Index: src/debug.cc
diff --git a/src/debug.cc b/src/debug.cc
index ffba7821cac966f4895b19571af370bd1b6739d7..47f105604689bae9888e08aa76ca509b3de55a2b 100644
--- a/src/debug.cc
+++ b/src/debug.cc
@@ -1776,10 +1776,12 @@ void Debug::PrepareForBreakPoints() {
       // debug break slots.
       isolate_->heap()->CollectAllGarbage(Heap::kMakeHeapIterableMask);
 
-      // Ensure no GC in this scope as we are comparing raw pointer
-      // values and performing a heap iteration.
+      // Ensure no GC in this scope as we are going to use gc_metadata
+      // field in the Code object to mark active functions.
       AssertNoAllocation no_allocation;
 
+      Object* active_code_marker = isolate_->heap()->the_hole_value();

[Erik Corry, 2012/01/25 14:05:28]

It seems we only go through the stack for the current thread, leaving the other
threads to crash, but lets fix that in a different change.

+
       // Find all non-optimized code functions with activation frames
       // on the stack. This includes functions which have optimized
       // activations (including inlined functions) on the stack as the
@@ -1790,57 +1792,56 @@ void Debug::PrepareForBreakPoints() {
           List<JSFunction*> functions(Compiler::kMaxInliningLevels + 1);
           frame->GetFunctions(&functions);
           for (int i = 0; i < functions.length(); i++) {
-            if (!functions[i]->shared()->code()->has_debug_break_slots()) {
-              active_functions.Add(Handle<JSFunction>(functions[i]));
-            }
+            JSFunction* function = functions[i];
+            active_functions.Add(Handle<JSFunction>(function));
+            function->shared()->code()->set_gc_metadata(active_code_marker);
           }
         } else if (frame->function()->IsJSFunction()) {
           JSFunction* function = JSFunction::cast(frame->function());
           ASSERT(frame->LookupCode()->kind() == Code::FUNCTION);
-          if (!frame->LookupCode()->has_debug_break_slots() ||
-              !function->shared()->code()->has_debug_break_slots()) {
-            active_functions.Add(Handle<JSFunction>(function));
-          }
+          active_functions.Add(Handle<JSFunction>(function));
+          function->shared()->code()->set_gc_metadata(active_code_marker);
         }
       }
 
-      // Sort the functions on the object pointer value to prepare for
-      // the binary search below.
-      active_functions.Sort(HandleObjectPointerCompare<JSFunction>);
-
       // Scan the heap for all non-optimized functions which has no

[Erik Corry, 2012/01/25 14:05:28]

has -> have

-      // debug break slots.
+      // debug break slots and are not active or inlined into an active
+      // function and mark them for lazy compilation.
       HeapIterator iterator;
       HeapObject* obj = NULL;
       while (((obj = iterator.next()) != NULL)) {
         if (obj->IsJSFunction()) {
           JSFunction* function = JSFunction::cast(obj);
-          if (function->shared()->allows_lazy_compilation() &&
-              function->shared()->script()->IsScript() &&
+          SharedFunctionInfo* shared = function->shared();
+          if (shared->allows_lazy_compilation() &&
+              shared->script()->IsScript() &&
               function->code()->kind() == Code::FUNCTION &&
-              !function->code()->has_debug_break_slots()) {
-            bool has_activation =
-                SortedListBSearch<Handle<JSFunction> >(
-                    active_functions,
-                    Handle<JSFunction>(function),
-                    HandleObjectPointerCompare<JSFunction>) != -1;
-            if (!has_activation) {
-              function->set_code(*lazy_compile);
-              function->shared()->set_code(*lazy_compile);
-            }
+              !function->code()->has_debug_break_slots() &&
+              shared->code()->gc_metadata() != active_code_marker) {
+            function->set_code(*lazy_compile);
+            function->shared()->set_code(*lazy_compile);
           }
         }
       }
-    }
 
-    // Now the non-GC scope is left, and the sorting of the functions
-    // in active_function is not ensured any more. The code below does
-    // not rely on it.
+      // Clear gc_metadata field.
+      for (int i = 0; i < active_functions.length(); i++) {
+        Handle<JSFunction> function = active_functions[i];
+        function->shared()->code()->set_gc_metadata(Smi::FromInt(0));
+      }
+    }
 
     // Now recompile all functions with activation frames and and
     // patch the return address to run in the new compiled code.
     for (int i = 0; i < active_functions.length(); i++) {
       Handle<JSFunction> function = active_functions[i];
+
+      if (function->code()->kind() == Code::FUNCTION &&
+          function->code()->has_debug_break_slots()) {
+        // Nothing to do. Function code already had debug break slots.
+        continue;
+      }
+
       Handle<SharedFunctionInfo> shared(function->shared());
       // If recompilation is not possible just skip it.
       if (shared->is_toplevel() ||
@@ -1851,9 +1852,6 @@ void Debug::PrepareForBreakPoints() {
 
       // Make sure that the shared full code is compiled with debug
       // break slots.
-      if (function->code() == *lazy_compile) {
-        function->set_code(shared->code());
-      }
       if (!shared->code()->has_debug_break_slots()) {
         // Try to compile the full code with debug break slots. If it
         // fails just keep the current code.
@@ -1872,6 +1870,10 @@ void Debug::PrepareForBreakPoints() {
           continue;
         }
       }
+
+      // Keep function code in sync with shared function info.
+      function->set_code(shared->code());
+
       Handle<Code> new_code(shared->code());
 
       // Find the function and patch the return address.