Revert "Support for precise stepping in functions compiled before debugging was started (step 2)"

TBR=kmillikin@chromium.org

BUG=
TEST=

Review URL: http://codereview.chromium.org//8101011

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9502 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/compiler.cc

@@ -58,6 +58,7 @@ CompilationInfo::CompilationInfo(Handle<Script> script)
       script_(script),
       extension_(NULL),
       pre_parse_data_(NULL),
+      supports_deoptimization_(false),
       osr_ast_id_(AstNode::kNoNumber) {
   Initialize(NONOPT);
 }
@@ -72,6 +73,7 @@ CompilationInfo::CompilationInfo(Handle<SharedFunctionInfo> shared_info)
       script_(Handle<Script>(Script::cast(shared_info->script()))),
       extension_(NULL),
       pre_parse_data_(NULL),
+      supports_deoptimization_(false),
       osr_ast_id_(AstNode::kNoNumber) {
   Initialize(BASE);
 }
@@ -87,6 +89,7 @@ CompilationInfo::CompilationInfo(Handle<JSFunction> closure)
       script_(Handle<Script>(Script::cast(shared_info_->script()))),
       extension_(NULL),
       pre_parse_data_(NULL),
+      supports_deoptimization_(false),
       osr_ast_id_(AstNode::kNoNumber) {
   Initialize(BASE);
 }
@@ -305,9 +308,9 @@ static bool MakeCrankshaftCode(CompilationInfo* info) {
 
 
 static bool GenerateCode(CompilationInfo* info) {
-  return info->IsCompilingForDebugging() || !V8::UseCrankshaft() ?
-      FullCodeGenerator::MakeCode(info) :
-      MakeCrankshaftCode(info);
+  return V8::UseCrankshaft() ?
+    MakeCrankshaftCode(info) :
+    FullCodeGenerator::MakeCode(info);
 }
 
 

src/compiler.h

@@ -120,19 +120,6 @@ class CompilationInfo BASE_EMBEDDED {
     ASSERT(IsOptimizing());
     osr_ast_id_ = osr_ast_id;
   }
-  void MarkCompilingForDebugging(Handle<Code> current_code) {
-    ASSERT(mode_ != OPTIMIZE);
-    ASSERT(current_code->kind() == Code::FUNCTION);
-    flags_ |= IsCompilingForDebugging::encode(true);
-    if (current_code->is_compiled_optimizable()) {
-      EnableDeoptimizationSupport();
-    } else {
-      mode_ = CompilationInfo::NONOPT;
-    }
-  }
-  bool IsCompilingForDebugging() {
-    return IsCompilingForDebugging::decode(flags_);
-  }
 
   bool has_global_object() const {
     return !closure().is_null() && (closure()->context()->global() != NULL);
@@ -152,12 +139,10 @@ class CompilationInfo BASE_EMBEDDED {
   void DisableOptimization();
 
   // Deoptimization support.
-  bool HasDeoptimizationSupport() const {
-    return SupportsDeoptimization::decode(flags_);
-  }
+  bool HasDeoptimizationSupport() const { return supports_deoptimization_; }
   void EnableDeoptimizationSupport() {
     ASSERT(IsOptimizable());
-    flags_ |= SupportsDeoptimization::encode(true);
+    supports_deoptimization_ = true;
   }
 
   // Determine whether or not we can adaptively optimize.
@@ -218,11 +203,6 @@ class CompilationInfo BASE_EMBEDDED {
   class IsNativesSyntaxAllowed: public BitField<bool, 5, 1> {};
   // Is this a function from our natives.
   class IsNative: public BitField<bool, 6, 1> {};
-  // Is this code being compiled with support for deoptimization..
-  class SupportsDeoptimization: public BitField<bool, 7, 1> {};
-  // If compiling for debugging produce just full code matching the
-  // initial mode setting.
-  class IsCompilingForDebugging: public BitField<bool, 8, 1> {};
 
 
   unsigned flags_;
@@ -251,6 +231,7 @@ class CompilationInfo BASE_EMBEDDED {
 
   // Compilation mode flag and whether deoptimization is allowed.
   Mode mode_;
+  bool supports_deoptimization_;
   int osr_ast_id_;
 
   DISALLOW_COPY_AND_ASSIGN(CompilationInfo);

src/debug.cc

@@ -1727,79 +1727,32 @@ void Debug::ClearStepNext() {
 }
 
 
-// Helper function to compile full code for debugging. This code will
-// have debug break slots and deoptimization
-// information. Deoptimization information is required in case that an
-// optimized version of this function is still activated on the
-// stack. It will also make sure that the full code is compiled with
-// the same flags as the previous version - that is flags which can
-// change the code generated. The current method of mapping from
-// already compiled full code without debug break slots to full code
-// with debug break slots depends on the generated code is otherwise
-// exactly the same.
-static bool CompileFullCodeForDebugging(Handle<SharedFunctionInfo> shared,
-                                        Handle<Code> current_code) {
-  ASSERT(!current_code->has_debug_break_slots());
-
-  CompilationInfo info(shared);
-  info.MarkCompilingForDebugging(current_code);
-  ASSERT(!info.shared_info()->is_compiled());
-  ASSERT(!info.isolate()->has_pending_exception());
-
-  // Use compile lazy which will end up compiling the full code in the
-  // configuration configured above.
-  bool result = Compiler::CompileLazy(&info);
-  ASSERT(result != Isolate::Current()->has_pending_exception());
-  info.isolate()->clear_pending_exception();
-#if DEBUG
-  if (result) {
-    Handle<Code> new_code(shared->code());
-    ASSERT(new_code->has_debug_break_slots());
-    ASSERT(current_code->is_compiled_optimizable() ==
-           new_code->is_compiled_optimizable());
-    ASSERT(current_code->instruction_size() <= new_code->instruction_size());
-  }
-#endif
-  return result;
-}
-
-
 void Debug::PrepareForBreakPoints() {
   // If preparing for the first break point make sure to deoptimize all
   // functions as debugging does not work with optimized code.
   if (!has_break_points_) {
     Deoptimizer::DeoptimizeAll();
 
-    Handle<Code> lazy_compile =
-        Handle<Code>(isolate_->builtins()->builtin(Builtins::kLazyCompile));
-
-    // Keep the list of activated functions in a handlified list as it
-    // is used both in GC and non-GC code.
-    List<Handle<JSFunction> > active_functions(100);
-
-    {
     // We are going to iterate heap to find all functions without
     // 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.
     AssertNoAllocation no_allocation;
+    Builtins* builtins = isolate_->builtins();
+    Code* lazy_compile = builtins->builtin(Builtins::kLazyCompile);
 
     // Find all non-optimized code functions with activation frames on
     // the stack.
+    List<JSFunction*> active_functions(100);
     for (JavaScriptFrameIterator it(isolate_); !it.done(); it.Advance()) {
       JavaScriptFrame* frame = it.frame();
       if (frame->function()->IsJSFunction()) {
         JSFunction* function = JSFunction::cast(frame->function());
-          if (function->code()->kind() == Code::FUNCTION &&
-              !function->code()->has_debug_break_slots())
-            active_functions.Add(Handle<JSFunction>(function));
+        if (function->code()->kind() == Code::FUNCTION)
+          active_functions.Add(function);
       }
     }
-      // Sort the functions on the object pointer value to prepare for
-      // the binary search below.
-      active_functions.Sort(HandleObjectPointerCompare<JSFunction>);
+    active_functions.Sort();
 
     // Scan the heap for all non-optimized functions which has no
     // debug break slots.
@@ -1813,118 +1766,12 @@ void Debug::PrepareForBreakPoints() {
             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;
+              SortedListBSearch<JSFunction*>(active_functions, function) != -1;
           if (!has_activation) {
-              function->set_code(*lazy_compile);
-              function->shared()->set_code(*lazy_compile);
-            }
-          }
-        }
+            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.
-
-    // 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];
-      Handle<SharedFunctionInfo> shared(function->shared());
-      // If recompilation is not possible just skip it.
-      if (shared->is_toplevel() ||
-          !shared->allows_lazy_compilation() ||
-          shared->code()->kind() == Code::BUILTIN) {
-        continue;
-      }
-
-      // Make sure that the shared full code is compiled with debug
-      // break slots.
-      Handle<Code> current_code(function->code());
-      if (shared->code()->has_debug_break_slots()) {
-        // if the code is already recompiled to have break slots skip
-        // recompilation.
-        ASSERT(!function->code()->has_debug_break_slots());
-      } else {
-        // Try to compile the full code with debug break slots. If it
-        // fails just keep the current code.
-        ASSERT(shared->code() == *current_code);
-        ZoneScope zone_scope(isolate_, DELETE_ON_EXIT);
-        shared->set_code(*lazy_compile);
-        bool prev_force_debugger_active =
-            isolate_->debugger()->force_debugger_active();
-        isolate_->debugger()->set_force_debugger_active(true);
-        CompileFullCodeForDebugging(shared, current_code);
-        isolate_->debugger()->set_force_debugger_active(
-            prev_force_debugger_active);
-        if (!shared->is_compiled()) {
-          shared->set_code(*current_code);
-          continue;
-        }
-      }
-      Handle<Code> new_code(shared->code());
-
-      // Find the function and patch return address.
-      for (JavaScriptFrameIterator it(isolate_); !it.done(); it.Advance()) {
-        JavaScriptFrame* frame = it.frame();
-        // If the current frame is for this function in its
-        // non-optimized form rewrite the return address to continue
-        // in the newly compiled full code with debug break slots.
-        if (frame->function()->IsJSFunction() &&
-            frame->function() == *function &&
-            frame->LookupCode()->kind() == Code::FUNCTION) {
-          intptr_t delta = frame->pc() - current_code->instruction_start();
-          int debug_break_slot_count = 0;
-          int mask = RelocInfo::ModeMask(RelocInfo::DEBUG_BREAK_SLOT);
-          for (RelocIterator it(*new_code, mask); !it.done(); it.next()) {
-            // Check if the pc in the new code with debug break
-            // slots is before this slot.
-            RelocInfo* info = it.rinfo();
-            int debug_break_slot_bytes =
-                debug_break_slot_count * Assembler::kDebugBreakSlotLength;
-            intptr_t new_delta =
-                info->pc() -
-                new_code->instruction_start() -
-                debug_break_slot_bytes;
-            if (new_delta > delta) {
-              break;
-            }
-
-            // Passed a debug break slot in the full code with debug
-            // break slots.
-            debug_break_slot_count++;
-          }
-          int debug_break_slot_bytes =
-              debug_break_slot_count * Assembler::kDebugBreakSlotLength;
-          if (FLAG_trace_deopt) {
-            PrintF("Replacing code %08" V8PRIxPTR " - %08" V8PRIxPTR " (%d) "
-                   "with %08" V8PRIxPTR " - %08" V8PRIxPTR " (%d) "
-                   "for debugging, "
-                   "changing pc from %08" V8PRIxPTR " to %08" V8PRIxPTR "\n",
-                   reinterpret_cast<intptr_t>(
-                       current_code->instruction_start()),
-                   reinterpret_cast<intptr_t>(
-                       current_code->instruction_start()) +
-                       current_code->instruction_size(),
-                   current_code->instruction_size(),
-                   reinterpret_cast<intptr_t>(new_code->instruction_start()),
-                   reinterpret_cast<intptr_t>(new_code->instruction_start()) +
-                       new_code->instruction_size(),
-                   new_code->instruction_size(),
-                   reinterpret_cast<intptr_t>(frame->pc()),
-                   reinterpret_cast<intptr_t>(new_code->instruction_start()) +
-                       delta + debug_break_slot_bytes);
-          }
-
-          // Patch the return address to return into the code with
-          // debug break slots.
-          frame->set_pc(
-              new_code->instruction_start() + delta + debug_break_slot_bytes);
-        }
       }
     }
   }
@@ -2994,9 +2841,7 @@ void Debugger::EnqueueDebugCommand(v8::Debug::ClientData* client_data) {
 bool Debugger::IsDebuggerActive() {
   ScopedLock with(debugger_access_);
 
-  return message_handler_ != NULL ||
-      !event_listener_.is_null() ||
-      force_debugger_active_;
+  return message_handler_ != NULL || !event_listener_.is_null();
 }
 
 

src/debug.h

@@ -809,15 +809,11 @@ class Debugger {
   }
 
   void set_compiling_natives(bool compiling_natives) {
-    compiling_natives_ = compiling_natives;
+    Debugger::compiling_natives_ = compiling_natives;
   }
   bool compiling_natives() const { return compiling_natives_; }
   void set_loading_debugger(bool v) { is_loading_debugger_ = v; }
   bool is_loading_debugger() const { return is_loading_debugger_; }
-  void set_force_debugger_active(bool force_debugger_active) {
-    force_debugger_active_ = force_debugger_active;
-  }
-  bool force_debugger_active() const { return force_debugger_active_; }
 
   bool IsDebuggerActive();
 
@@ -843,7 +839,6 @@ class Debugger {
   bool compiling_natives_;  // Are we compiling natives?
   bool is_loading_debugger_;  // Are we loading the debugger?
   bool never_unload_debugger_;  // Can we unload the debugger?
-  bool force_debugger_active_;  // Activate debugger without event listeners.
   v8::Debug::MessageHandler2 message_handler_;
   bool debugger_unload_pending_;  // Was message handler cleared?
   v8::Debug::HostDispatchHandler host_dispatch_handler_;

src/full-codegen.cc

@@ -289,12 +289,11 @@ bool FullCodeGenerator::MakeCode(CompilationInfo* info) {
 #ifdef ENABLE_DEBUGGER_SUPPORT
   code->set_has_debug_break_slots(
       info->isolate()->debugger()->IsDebuggerActive());
-  code->set_compiled_optimizable(info->IsOptimizable());
 #endif  // ENABLE_DEBUGGER_SUPPORT
   code->set_allow_osr_at_loop_nesting_level(0);
   code->set_stack_check_table_offset(table_offset);
   CodeGenerator::PrintCode(code, info);
-  info->SetCode(code);  // May be an empty handle.
+  info->SetCode(code);  // may be an empty handle.
 #ifdef ENABLE_GDB_JIT_INTERFACE
   if (FLAG_gdbjit && !code.is_null()) {
     GDBJITLineInfo* lineinfo =

src/list-inl.h

@@ -216,11 +216,11 @@ int SortedListBSearch(
     int mid = (low + high) / 2;
     T mid_elem = list[mid];
 
-    if (cmp(&mid_elem, &elem) > 0) {
+    if (mid_elem > elem) {
       high = mid - 1;
       continue;
     }
-    if (cmp(&mid_elem, &elem) < 0) {
+    if (mid_elem < elem) {
       low = mid + 1;
       continue;
     }

src/objects-inl.h

@@ -2996,21 +2996,6 @@ void Code::set_has_debug_break_slots(bool value) {
 }
 
 
-bool Code::is_compiled_optimizable() {
-  ASSERT(kind() == FUNCTION);
-  byte flags = READ_BYTE_FIELD(this, kFullCodeFlags);
-  return FullCodeFlagsIsCompiledOptimizable::decode(flags);
-}
-
-
-void Code::set_compiled_optimizable(bool value) {
-  ASSERT(kind() == FUNCTION);
-  byte flags = READ_BYTE_FIELD(this, kFullCodeFlags);
-  flags = FullCodeFlagsIsCompiledOptimizable::update(flags, value);
-  WRITE_BYTE_FIELD(this, kFullCodeFlags, flags);
-}
-
-
 int Code::allow_osr_at_loop_nesting_level() {
   ASSERT(kind() == FUNCTION);
   return READ_BYTE_FIELD(this, kAllowOSRAtLoopNestingLevelOffset);

src/objects.h

@@ -3677,11 +3677,6 @@ class Code: public HeapObject {
   inline bool has_debug_break_slots();
   inline void set_has_debug_break_slots(bool value);
 
-  // [compiled_with_optimizing]: For FUNCTION kind, tells if it has
-  // been compiled with IsOptimizing set to true.
-  inline bool is_compiled_optimizable();
-  inline void set_compiled_optimizable(bool value);
-
   // [allow_osr_at_loop_nesting_level]: For FUNCTION kind, tells for
   // how long the function has been marked for OSR and therefore which
   // level of loop nesting we are willing to do on-stack replacement
@@ -3877,7 +3872,6 @@ class Code: public HeapObject {
   class FullCodeFlagsHasDeoptimizationSupportField:
       public BitField<bool, 0, 1> {};  // NOLINT
   class FullCodeFlagsHasDebugBreakSlotsField: public BitField<bool, 1, 1> {};
-  class FullCodeFlagsIsCompiledOptimizable: public BitField<bool, 2, 1> {};
 
   static const int kBinaryOpReturnTypeOffset = kBinaryOpTypeOffset + 1;
 

src/utils.h

@@ -143,16 +143,6 @@ static int PointerValueCompare(const T* a, const T* b) {
 }
 
 
-// Compare function to compare the object pointer value of two
-// handlified objects. The handles are passed as pointers to the
-// handles.
-template<typename T> class Handle;  // Forward declaration.
-template <typename T>
-static int HandleObjectPointerCompare(const Handle<T>* a, const Handle<T>* b) {
-  return Compare<T*>(*(*a), *(*b));
-}
-
-
 // Returns the smallest power of two which is >= x. If you pass in a
 // number that is already a power of two, it is returned as is.
 // Implementation is from "Hacker's Delight" by Henry S. Warren, Jr.,

test/mjsunit/debug-step-3.js

-// Copyright 2011 the V8 project authors. All rights reserved.
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are
-// met:
-//
-//     * Redistributions of source code must retain the above copyright
-//       notice, this list of conditions and the following disclaimer.
-//     * Redistributions in binary form must reproduce the above
-//       copyright notice, this list of conditions and the following
-//       disclaimer in the documentation and/or other materials provided
-//       with the distribution.
-//     * Neither the name of Google Inc. nor the names of its
-//       contributors may be used to endorse or promote products derived
-//       from this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-// Flags: --expose-debug-as debug
-
-// This test tests that full code compiled without debug break slots
-// is recompiled with debug break slots when debugging is started.
-
-// Get the Debug object exposed from the debug context global object.
-Debug = debug.Debug
-
-var bp;
-var done = false;
-var step_count = 0;
-var set_bp = false
-
-// Debug event listener which steps until the global variable done is true.
-function listener(event, exec_state, event_data, data) {
-  if (event == Debug.DebugEvent.Break) {
-    if (!done) exec_state.prepareStep(Debug.StepAction.StepNext);
-    step_count++;
-  }
-};
-
-// Set the global variables state to prpare the stepping test.
-function prepare_step_test() {
-  done = false;
-  step_count = 0;
-}
-
-// Test function to step through.
-function f() {
-  var a = 0;
-  if (set_bp) { bp = Debug.setBreakPoint(f, 3); }
-  var i = 1;
-  var j = 2;
-  done = true;
-};
-
-prepare_step_test();
-f();
-
-// Add the debug event listener.
-Debug.setListener(listener);
-
-// Make f set a breakpoint with an activation on the stack.
-prepare_step_test();
-set_bp = true;
-f();
-assertEquals(4, step_count);
-Debug.clearBreakPoint(bp);
-
-// Set a breakpoint on the first var statement (line 1).
-set_bp = false;
-bp = Debug.setBreakPoint(f, 3);
-
-// Step through the function ensuring that the var statements are hit as well.
-prepare_step_test();
-f();
-assertEquals(4, step_count);
-
-// Clear the breakpoint and check that no stepping happens.
-Debug.clearBreakPoint(bp);
-prepare_step_test();
-f();
-assertEquals(0, step_count);
-
-// Get rid of the debug event listener.
-Debug.setListener(null);