Cleaned up some debugger stuff on ia32 and x64.

Got rid of the debug break on return entry code which did not add anything. It just jumped directly to the debug break on return code.

Removed the CodePatcher class on x64 as it was not implemented.

Added instruction cache flush to where the return sequence was patched on x64.

Added some missing ENABLE_DEBUGGER_SUPPORT #ifdef/#endif.
Review URL: http://codereview.chromium.org/193057

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

src/builtins.cc

@@ -609,11 +609,6 @@ static void Generate_Return_DebugBreak(MacroAssembler* masm) {
 }
 
 
-static void Generate_Return_DebugBreakEntry(MacroAssembler* masm) {
-  Debug::GenerateReturnDebugBreakEntry(masm);
-}
-
-
 static void Generate_StubNoRegisters_DebugBreak(MacroAssembler* masm) {
   Debug::GenerateStubNoRegistersDebugBreak(masm);
 }

src/builtins.h

@@ -90,7 +90,6 @@ namespace internal {
 // Define list of builtins used by the debugger implemented in assembly.
 #define BUILTIN_LIST_DEBUG_A(V)                                \
   V(Return_DebugBreak,          BUILTIN, DEBUG_BREAK)          \
-  V(Return_DebugBreakEntry,     BUILTIN, DEBUG_BREAK)          \
   V(ConstructCall_DebugBreak,   BUILTIN, DEBUG_BREAK)          \
   V(StubNoRegisters_DebugBreak, BUILTIN, DEBUG_BREAK)          \
   V(LoadIC_DebugBreak,          LOAD_IC, DEBUG_BREAK)          \

src/debug.cc

@@ -563,7 +563,6 @@ bool Debug::break_on_exception_ = false;
 bool Debug::break_on_uncaught_exception_ = true;
 
 Handle<Context> Debug::debug_context_ = Handle<Context>();
-Code* Debug::debug_break_return_entry_ = NULL;
 Code* Debug::debug_break_return_ = NULL;
 
 
@@ -644,11 +643,6 @@ void ScriptCache::HandleWeakScript(v8::Persistent<v8::Value> obj, void* data) {
 void Debug::Setup(bool create_heap_objects) {
   ThreadInit();
   if (create_heap_objects) {
-    // Get code to handle entry to debug break on return.
-    debug_break_return_entry_ =
-        Builtins::builtin(Builtins::Return_DebugBreakEntry);
-    ASSERT(debug_break_return_entry_->IsCode());
-
     // Get code to handle debug break on return.
     debug_break_return_ =
         Builtins::builtin(Builtins::Return_DebugBreak);
@@ -810,7 +804,6 @@ void Debug::PreemptionWhileInDebugger() {
 
 
 void Debug::Iterate(ObjectVisitor* v) {
-  v->VisitPointer(bit_cast<Object**, Code**>(&(debug_break_return_entry_)));
   v->VisitPointer(bit_cast<Object**, Code**>(&(debug_break_return_)));
 }
 
@@ -1614,26 +1607,25 @@ void Debug::SetAfterBreakTarget(JavaScriptFrame* frame) {
   Address addr = frame->pc() - Assembler::kPatchReturnSequenceLength;
 
   // Check if the location is at JS exit.
-  bool at_js_exit = false;
+  bool at_js_return = false;
+  bool break_at_js_return_active = false;
   RelocIterator it(debug_info->code());
   while (!it.done()) {
     if (RelocInfo::IsJSReturn(it.rinfo()->rmode())) {
-      at_js_exit = (it.rinfo()->pc() ==
-                        addr - Assembler::kPatchReturnSequenceAddressOffset);
+      at_js_return = (it.rinfo()->pc() ==
+          addr - Assembler::kPatchReturnSequenceAddressOffset);
+      break_at_js_return_active = it.rinfo()->IsCallInstruction();
     }
     it.next();
   }
 
   // Handle the jump to continue execution after break point depending on the
   // break location.
-  if (at_js_exit) {
-    // First check if the call in the code is still the debug break return
-    // entry code. If it is the break point is still active. If not the break
-    // point was removed during break point processing.
-    if (Assembler::target_address_at(addr) ==
-        debug_break_return_entry()->entry()) {
-      // Break point still active. Jump to the corresponding place in the
-      // original code.
+  if (at_js_return) {
+    // If the break point as return is still active jump to the corresponding
+    // place in the original code. If not the break point was removed during
+    // break point processing.
+    if (break_at_js_return_active) {
       addr +=  original_code->instruction_start() - code->instruction_start();
     }
 

src/debug.h

@@ -335,10 +335,8 @@ class Debug {
     return &registers_[r];
   }
 
-  // Address of the debug break return entry code.
-  static Code* debug_break_return_entry() { return debug_break_return_entry_; }
-
-  // Support for getting the address of the debug break on return code.
+  // Access to the debug break on return code.
+  static Code* debug_break_return() { return debug_break_return_; }
   static Code** debug_break_return_address() {
     return &debug_break_return_;
   }
@@ -385,7 +383,6 @@ class Debug {
   static void GenerateKeyedStoreICDebugBreak(MacroAssembler* masm);
   static void GenerateConstructCallDebugBreak(MacroAssembler* masm);
   static void GenerateReturnDebugBreak(MacroAssembler* masm);
-  static void GenerateReturnDebugBreakEntry(MacroAssembler* masm);
   static void GenerateStubNoRegistersDebugBreak(MacroAssembler* masm);
 
   // Called from stub-cache.cc.
@@ -469,9 +466,6 @@ class Debug {
   static ThreadLocal thread_local_;
   static void ThreadInit();
 
-  // Code object for debug break return entry code.
-  static Code* debug_break_return_entry_;
-
   // Code to call for handling debug break on return.
   static Code* debug_break_return_;
 

src/ia32/debug-ia32.cc

@@ -36,9 +36,7 @@ namespace internal {
 
 #ifdef ENABLE_DEBUGGER_SUPPORT
 
-// A debug break in the frame exit code is identified by a call instruction.
 bool BreakLocationIterator::IsDebugBreakAtReturn() {
-  // Opcode E8 is call.
   return Debug::IsDebugBreakAtReturn(rinfo());
 }
 
@@ -49,7 +47,7 @@ bool BreakLocationIterator::IsDebugBreakAtReturn() {
 void BreakLocationIterator::SetDebugBreakAtReturn() {
   ASSERT(Debug::kIa32JSReturnSequenceLength >=
          Debug::kIa32CallInstructionLength);
-  rinfo()->PatchCodeWithCall(Debug::debug_break_return_entry()->entry(),
+  rinfo()->PatchCodeWithCall(Debug::debug_break_return()->entry(),
       Debug::kIa32JSReturnSequenceLength - Debug::kIa32CallInstructionLength);
 }
 
@@ -61,11 +59,11 @@ void BreakLocationIterator::ClearDebugBreakAtReturn() {
 }
 
 
-// Check whether the JS frame exit code has been patched with a debug break.
+// A debug break in the frame exit code is identified by the JS frame exit code
+// having been patched with a call instruction.
 bool Debug::IsDebugBreakAtReturn(RelocInfo* rinfo) {
   ASSERT(RelocInfo::IsJSReturn(rinfo->rmode()));
-  // Opcode E8 is call.
-  return (*(rinfo->pc()) == 0xE8);
+  return rinfo->IsCallInstruction();
 }
 
 
@@ -194,17 +192,6 @@ void Debug::GenerateReturnDebugBreak(MacroAssembler* masm) {
 }
 
 
-void Debug::GenerateReturnDebugBreakEntry(MacroAssembler* masm) {
-  // OK to clobber ebx as we are returning from a JS function through the code
-  // generated by CodeGenerator::GenerateReturnSequence()
-  ExternalReference debug_break_return =
-      ExternalReference(Debug_Address::DebugBreakReturn());
-  __ mov(ebx, Operand::StaticVariable(debug_break_return));
-  __ add(Operand(ebx), Immediate(Code::kHeaderSize - kHeapObjectTag));
-  __ jmp(Operand(ebx));
-}
-
-
 void Debug::GenerateStubNoRegistersDebugBreak(MacroAssembler* masm) {
   // Register state for stub CallFunction (from CallFunctionStub in ic-ia32.cc).
   // ----------- S t a t e -------------

src/ia32/macro-assembler-ia32.cc

@@ -1170,8 +1170,9 @@ void MacroAssembler::Abort(const char* msg) {
 }
 
 
+#ifdef ENABLE_DEBUGGER_SUPPORT
 CodePatcher::CodePatcher(byte* address, int size)
-  : address_(address), size_(size), masm_(address, size + Assembler::kGap) {
+    : address_(address), size_(size), masm_(address, size + Assembler::kGap) {
   // Create a new macro assembler pointing to the address of the code to patch.
   // The size is adjusted with kGap on order for the assembler to generate size
   // bytes of instructions without failing with buffer size constraints.
@@ -1187,6 +1188,7 @@ CodePatcher::~CodePatcher() {
   ASSERT(masm_.pc_ == address_ + size_);
   ASSERT(masm_.reloc_info_writer.pos() == address_ + size_ + Assembler::kGap);
 }
+#endif  // ENABLE_DEBUGGER_SUPPORT
 
 
 } }  // namespace v8::internal

src/ia32/macro-assembler-ia32.h

@@ -338,6 +338,7 @@ class MacroAssembler: public Assembler {
 };
 
 
+#ifdef ENABLE_DEBUGGER_SUPPORT
 // The code patcher is used to patch (typically) small parts of code e.g. for
 // debugging and other types of instrumentation. When using the code patcher
 // the exact number of bytes specified must be emitted. Is not legal to emit
@@ -356,6 +357,7 @@ class CodePatcher {
   int size_;  // Number of bytes of the expected patch size.
   MacroAssembler masm_;  // Macro assembler used to generate the code.
 };
+#endif  // ENABLE_DEBUGGER_SUPPORT
 
 
 // -----------------------------------------------------------------------------

src/x64/assembler-x64.cc

@@ -174,6 +174,7 @@ void CpuFeatures::Probe()  {
 // Additional guard int3 instructions can be added if required.
 void RelocInfo::PatchCodeWithCall(Address target, int guard_bytes) {
   // Call instruction takes up 13 bytes and int3 takes up one byte.
+  static const int kCallInstructionSize = 13;
   Address patch_site = pc_;
   Memory::uint16_at(patch_site) = 0xBA49u;  // movq r10, imm64
   // Write "0x00, call r10" starting at last byte of address.  We overwrite
@@ -183,8 +184,11 @@ void RelocInfo::PatchCodeWithCall(Address target, int guard_bytes) {
 
   // Add the requested number of int3 instructions after the call.
   for (int i = 0; i < guard_bytes; i++) {
-    *(patch_site + 13 + i) = 0xCC;  // int3
+    *(patch_site + kCallInstructionSize + i) = 0xCC;  // int3
   }
+
+  // Indicate that code has changed.
+  CPU::FlushICache(patch_site, kCallInstructionSize + guard_bytes);
 }
 
 
@@ -275,7 +279,7 @@ Assembler::Assembler(void* buffer, int buffer_size) {
 
   // Clear the buffer in debug mode unless it was provided by the
   // caller in which case we can't be sure it's okay to overwrite
-  // existing code in it; see CodePatcher::CodePatcher(...).
+  // existing code in it.
 #ifdef DEBUG
   if (own_buffer_) {
     memset(buffer_, 0xCC, buffer_size);  // int3

src/x64/debug-x64.cc

@@ -160,18 +160,6 @@ void Debug::GenerateReturnDebugBreak(MacroAssembler* masm) {
 }
 
 
-void Debug::GenerateReturnDebugBreakEntry(MacroAssembler* masm) {
-  // OK to clobber rbx as we are returning from a JS function through the code
-  // generated by CodeGenerator::GenerateReturnSequence()
-  ExternalReference debug_break_return =
-      ExternalReference(Debug_Address::DebugBreakReturn());
-  __ movq(rbx, debug_break_return);
-  __ movq(rbx, Operand(rbx, 0));
-  __ addq(rbx, Immediate(Code::kHeaderSize - kHeapObjectTag));
-  __ jmp(rbx);
-}
-
-
 void Debug::GenerateStoreICDebugBreak(MacroAssembler* masm) {
   // REgister state for IC store call (from ic-x64.cc).
   // ----------- S t a t e -------------
@@ -207,7 +195,7 @@ bool BreakLocationIterator::IsDebugBreakAtReturn()  {
 
 void BreakLocationIterator::SetDebugBreakAtReturn()  {
   ASSERT(Debug::kX64JSReturnSequenceLength >= Debug::kX64CallInstructionLength);
-  rinfo()->PatchCodeWithCall(Debug::debug_break_return_entry()->entry(),
+  rinfo()->PatchCodeWithCall(Debug::debug_break_return()->entry(),
       Debug::kX64JSReturnSequenceLength - Debug::kX64CallInstructionLength);
 }
 

src/x64/macro-assembler-x64.h

@@ -380,26 +380,6 @@ class MacroAssembler: public Assembler {
 };
 
 
-// The code patcher is used to patch (typically) small parts of code e.g. for
-// debugging and other types of instrumentation. When using the code patcher
-// the exact number of bytes specified must be emitted. Is not legal to emit
-// relocation information. If any of these constraints are violated it causes
-// an assertion.
-class CodePatcher {
- public:
-  CodePatcher(byte* address, int size);
-  virtual ~CodePatcher();
-
-  // Macro assembler to emit code.
-  MacroAssembler* masm() { return &masm_; }
-
- private:
-  byte* address_;  // The address of the code being patched.
-  int size_;  // Number of bytes of the expected patch size.
-  MacroAssembler masm_;  // Macro assembler used to generate the code.
-};
-
-
 // -----------------------------------------------------------------------------
 // Static helper functions.