X87: Debugger: use debug break slots to break at function exit.

port fc9c5275 (r29672).

original commit message:

    Debugger: use debug break slots to break at function exit.

    By not having to patch the return sequence (we patch the debug
    break slot right before it), we don't overwrite it and therefore
    don't have to keep the original copy of the code around.

BUG=

Review URL: https://codereview.chromium.org/1236023007

Cr-Commit-Position: refs/heads/master@{#29694}

src/x87/assembler-x87-inl.h

@@ -53,35 +53,25 @@ static const int kNoCodeAgeSequenceLength = 5;
 
 
 // The modes possibly affected by apply must be in kApplyMask.
-void RelocInfo::apply(intptr_t delta, ICacheFlushMode icache_flush_mode) {
-  bool flush_icache = icache_flush_mode != SKIP_ICACHE_FLUSH;
+void RelocInfo::apply(intptr_t delta) {
   if (IsRuntimeEntry(rmode_) || IsCodeTarget(rmode_)) {
     int32_t* p = reinterpret_cast<int32_t*>(pc_);
     *p -= delta;  // Relocate entry.
-    if (flush_icache) CpuFeatures::FlushICache(p, sizeof(uint32_t));
   } else if (IsCodeAgeSequence(rmode_)) {
     if (*pc_ == kCallOpcode) {
       int32_t* p = reinterpret_cast<int32_t*>(pc_ + 1);
       *p -= delta;  // Relocate entry.
-      if (flush_icache) CpuFeatures::FlushICache(p, sizeof(uint32_t));
     }
-  } else if (IsJSReturn(rmode_) && IsPatchedReturnSequence()) {
-    // Special handling of js_return when a break point is set (call
-    // instruction has been inserted).
-    int32_t* p = reinterpret_cast<int32_t*>(pc_ + 1);
-    *p -= delta;  // Relocate entry.
-    if (flush_icache) CpuFeatures::FlushICache(p, sizeof(uint32_t));
   } else if (IsDebugBreakSlot(rmode_) && IsPatchedDebugBreakSlotSequence()) {
     // Special handling of a debug break slot when a break point is set (call
     // instruction has been inserted).
-    int32_t* p = reinterpret_cast<int32_t*>(pc_ + 1);
+    int32_t* p = reinterpret_cast<int32_t*>(
+        pc_ + Assembler::kPatchDebugBreakSlotAddressOffset);
     *p -= delta;  // Relocate entry.
-    if (flush_icache) CpuFeatures::FlushICache(p, sizeof(uint32_t));
   } else if (IsInternalReference(rmode_)) {
     // absolute code pointer inside code object moves with the code object.
     int32_t* p = reinterpret_cast<int32_t*>(pc_);
     *p += delta;  // Relocate entry.
-    if (flush_icache) CpuFeatures::FlushICache(p, sizeof(uint32_t));
   }
 }
 
@@ -245,17 +235,17 @@ void RelocInfo::set_code_age_stub(Code* stub,
 }
 
 
-Address RelocInfo::call_address() {
-  DCHECK((IsJSReturn(rmode()) && IsPatchedReturnSequence()) ||
-         (IsDebugBreakSlot(rmode()) && IsPatchedDebugBreakSlotSequence()));
-  return Assembler::target_address_at(pc_ + 1, host_);
+Address RelocInfo::debug_call_address() {
+  DCHECK(IsDebugBreakSlot(rmode()) && IsPatchedDebugBreakSlotSequence());
+  Address location = pc_ + Assembler::kPatchDebugBreakSlotAddressOffset;
+  return Assembler::target_address_at(location, host_);
 }
 
 
-void RelocInfo::set_call_address(Address target) {
-  DCHECK((IsJSReturn(rmode()) && IsPatchedReturnSequence()) ||
-         (IsDebugBreakSlot(rmode()) && IsPatchedDebugBreakSlotSequence()));
-  Assembler::set_target_address_at(pc_ + 1, host_, target);
+void RelocInfo::set_debug_call_address(Address target) {
+  DCHECK(IsDebugBreakSlot(rmode()) && IsPatchedDebugBreakSlotSequence());
+  Address location = pc_ + Assembler::kPatchDebugBreakSlotAddressOffset;
+  Assembler::set_target_address_at(location, host_, target);
   if (host() != NULL) {
     Object* target_code = Code::GetCodeFromTargetAddress(target);
     host()->GetHeap()->incremental_marking()->RecordWriteIntoCode(
@@ -264,23 +254,6 @@ void RelocInfo::set_call_address(Address target) {
 }
 
 
-Object* RelocInfo::call_object() {
-  return *call_object_address();
-}
-
-
-void RelocInfo::set_call_object(Object* target) {
-  *call_object_address() = target;
-}
-
-
-Object** RelocInfo::call_object_address() {
-  DCHECK((IsJSReturn(rmode()) && IsPatchedReturnSequence()) ||
-         (IsDebugBreakSlot(rmode()) && IsPatchedDebugBreakSlotSequence()));
-  return reinterpret_cast<Object**>(pc_ + 1);
-}
-
-
 void RelocInfo::WipeOut() {
   if (IsEmbeddedObject(rmode_) || IsExternalReference(rmode_) ||
       IsInternalReference(rmode_)) {
@@ -319,10 +292,8 @@ void RelocInfo::Visit(Isolate* isolate, ObjectVisitor* visitor) {
     visitor->VisitInternalReference(this);
   } else if (RelocInfo::IsCodeAgeSequence(mode)) {
     visitor->VisitCodeAgeSequence(this);
-  } else if (((RelocInfo::IsJSReturn(mode) &&
-              IsPatchedReturnSequence()) ||
-             (RelocInfo::IsDebugBreakSlot(mode) &&
-              IsPatchedDebugBreakSlotSequence())) &&
+  } else if (RelocInfo::IsDebugBreakSlot(mode) &&
+             IsPatchedDebugBreakSlotSequence() &&
              isolate->debug()->has_break_points()) {
     visitor->VisitDebugTarget(this);
   } else if (IsRuntimeEntry(mode)) {
@@ -348,10 +319,8 @@ void RelocInfo::Visit(Heap* heap) {
   } else if (RelocInfo::IsCodeAgeSequence(mode)) {
     StaticVisitor::VisitCodeAgeSequence(heap, this);
   } else if (heap->isolate()->debug()->has_break_points() &&
-             ((RelocInfo::IsJSReturn(mode) &&
-              IsPatchedReturnSequence()) ||
-             (RelocInfo::IsDebugBreakSlot(mode) &&
-              IsPatchedDebugBreakSlotSequence()))) {
+             RelocInfo::IsDebugBreakSlot(mode) &&
+             IsPatchedDebugBreakSlotSequence()) {
     StaticVisitor::VisitDebugTarget(heap, this);
   } else if (IsRuntimeEntry(mode)) {
     StaticVisitor::VisitRuntimeEntry(this);
@@ -503,11 +472,6 @@ Address Assembler::target_address_from_return_address(Address pc) {
 }
 
 
-Address Assembler::break_address_from_return_address(Address pc) {
-  return pc - Assembler::kPatchDebugBreakSlotReturnOffset;
-}
-
-
 Displacement Assembler::disp_at(Label* L) {
   return Displacement(long_at(L->pos()));
 }

src/x87/assembler-x87.cc

@@ -83,8 +83,8 @@ void Displacement::init(Label* L, Type type) {
 
 const int RelocInfo::kApplyMask =
     RelocInfo::kCodeTargetMask | 1 << RelocInfo::RUNTIME_ENTRY |
-    1 << RelocInfo::JS_RETURN | 1 << RelocInfo::INTERNAL_REFERENCE |
-    1 << RelocInfo::CODE_AGE_SEQUENCE | RelocInfo::kDebugBreakSlotMask;
+    1 << RelocInfo::INTERNAL_REFERENCE | 1 << RelocInfo::CODE_AGE_SEQUENCE |
+    RelocInfo::kDebugBreakSlotMask;
 
 
 bool RelocInfo::IsCodedSpecially() {

src/x87/assembler-x87.h

@@ -524,9 +524,6 @@ class Assembler : public AssemblerBase {
   // of that call in the instruction stream.
   inline static Address target_address_from_return_address(Address pc);
 
-  // Return the code target address of the patch debug break slot
-  inline static Address break_address_from_return_address(Address pc);
-
   // This sets the branch destination (which is in the instruction on x86).
   // This is for calls and branches within generated code.
   inline static void deserialization_set_special_target_at(
@@ -544,21 +541,16 @@ class Assembler : public AssemblerBase {
   // Distance between the address of the code target in the call instruction
   // and the return address
   static const int kCallTargetAddressOffset = kPointerSize;
-  // Distance between start of patched return sequence and the emitted address
-  // to jump to.
-  static const int kPatchReturnSequenceAddressOffset = 1;  // JMP imm32.
-
-  // Distance between start of patched debug break slot and the emitted address
-  // to jump to.
-  static const int kPatchDebugBreakSlotAddressOffset = 1;  // JMP imm32.
 
   static const int kCallInstructionLength = 5;
-  static const int kPatchDebugBreakSlotReturnOffset = kPointerSize;
-  static const int kJSReturnSequenceLength = 6;
 
   // The debug break slot must be able to contain a call instruction.
   static const int kDebugBreakSlotLength = kCallInstructionLength;
 
+  // Distance between start of patched debug break slot and the emitted address
+  // to jump to.
+  static const int kPatchDebugBreakSlotAddressOffset = 1;  // JMP imm32.
+
   // One byte opcode for test al, 0xXX.
   static const byte kTestAlByte = 0xA8;
   // One byte opcode for nop.
@@ -951,16 +943,11 @@ class Assembler : public AssemblerBase {
     return pc_offset() - label->pos();
   }
 
-  // Mark address of the ExitJSFrame code.
-  void RecordJSReturn();
-
   // Mark generator continuation.
   void RecordGeneratorContinuation();
 
   // Mark address of a debug break slot.
-  void RecordDebugBreakSlot();
-  void RecordDebugBreakSlotForCall(int argc);
-  void RecordDebugBreakSlotForConstructCall();
+  void RecordDebugBreakSlot(RelocInfo::Mode mode, int argc = 0);
 
   // Record a comment relocation entry that can be used by a disassembler.
   // Use --code-comments to enable.

src/x87/debug-x87.cc

@@ -13,64 +13,50 @@
 namespace v8 {
 namespace internal {
 
-// Patch the code at the current PC with a call to the target address.
-// Additional guard int3 instructions can be added if required.
-void PatchCodeWithCall(Address pc, Address target, int guard_bytes) {
-  // Call instruction takes up 5 bytes and int3 takes up one byte.
-  static const int kCallCodeSize = 5;
-  int code_size = kCallCodeSize + guard_bytes;
-
-  // Create a code patcher.
-  CodePatcher patcher(pc, code_size);
-
-// Add a label for checking the size of the code used for returning.
-#ifdef DEBUG
-  Label check_codesize;
-  patcher.masm()->bind(&check_codesize);
-#endif
+#define __ ACCESS_MASM(masm)
 
-  // Patch the code.
-  patcher.masm()->call(target, RelocInfo::NONE32);
 
-  // Check that the size of the code generated is as expected.
-  DCHECK_EQ(kCallCodeSize,
-            patcher.masm()->SizeOfCodeGeneratedSince(&check_codesize));
+void EmitDebugBreakSlot(MacroAssembler* masm) {
+  Label check_codesize;
+  __ bind(&check_codesize);
+  __ Nop(Assembler::kDebugBreakSlotLength);
+  DCHECK_EQ(Assembler::kDebugBreakSlotLength,
+            masm->SizeOfCodeGeneratedSince(&check_codesize));
+}
 
-  // Add the requested number of int3 instructions after the call.
-  DCHECK_GE(guard_bytes, 0);
-  for (int i = 0; i < guard_bytes; i++) {
-    patcher.masm()->int3();
-  }
 
-  CpuFeatures::FlushICache(pc, code_size);
+void DebugCodegen::GenerateSlot(MacroAssembler* masm, RelocInfo::Mode mode,
+                                int call_argc) {
+  // Generate enough nop's to make space for a call instruction.
+  masm->RecordDebugBreakSlot(mode, call_argc);
+  EmitDebugBreakSlot(masm);
 }
 
 
-// Patch the JS frame exit code with a debug break call. See
-// CodeGenerator::VisitReturnStatement and VirtualFrame::Exit in codegen-x87.cc
-// for the precise return instructions sequence.
-void BreakLocation::SetDebugBreakAtReturn() {
-  DCHECK(Assembler::kJSReturnSequenceLength >=
-         Assembler::kCallInstructionLength);
-  PatchCodeWithCall(
-      pc(), debug_info_->GetIsolate()->builtins()->Return_DebugBreak()->entry(),
-      Assembler::kJSReturnSequenceLength - Assembler::kCallInstructionLength);
+void DebugCodegen::ClearDebugBreakSlot(Address pc) {
+  CodePatcher patcher(pc, Assembler::kDebugBreakSlotLength);
+  EmitDebugBreakSlot(patcher.masm());
 }
 
 
-void BreakLocation::SetDebugBreakAtSlot() {
-  DCHECK(IsDebugBreakSlot());
-  Isolate* isolate = debug_info_->GetIsolate();
-  PatchCodeWithCall(
-      pc(), isolate->builtins()->Slot_DebugBreak()->entry(),
-      Assembler::kDebugBreakSlotLength - Assembler::kCallInstructionLength);
+void DebugCodegen::PatchDebugBreakSlot(Address pc, Handle<Code> code) {
+  DCHECK_EQ(Code::BUILTIN, code->kind());
+  static const int kSize = Assembler::kDebugBreakSlotLength;
+  CodePatcher patcher(pc, kSize);
+
+  // Add a label for checking the size of the code used for returning.
+  Label check_codesize;
+  patcher.masm()->bind(&check_codesize);
+  patcher.masm()->call(code->entry(), RelocInfo::NONE32);
+  // Check that the size of the code generated is as expected.
+  DCHECK_EQ(kSize, patcher.masm()->SizeOfCodeGeneratedSince(&check_codesize));
 }
 
 
-#define __ ACCESS_MASM(masm)
+void DebugCodegen::GenerateDebugBreakStub(MacroAssembler* masm,
+                                          DebugBreakCallHelperMode mode) {
+  __ RecordComment("Debug break");
 
-static void Generate_DebugBreakCallHelper(MacroAssembler* masm,
-                                          RegList object_regs) {
   // Enter an internal frame.
   {
     FrameScope scope(masm, StackFrame::INTERNAL);
@@ -81,56 +67,27 @@ static void Generate_DebugBreakCallHelper(MacroAssembler* masm,
     }
     __ push(Immediate(Smi::FromInt(LiveEdit::kFramePaddingInitialSize)));
 
-    // Store the registers containing live values on the expression stack to
-    // make sure that these are correctly updated during GC. Non object values
-    // are stored as a smi causing it to be untouched by GC.
-    DCHECK((object_regs & ~kJSCallerSaved) == 0);
-    for (int i = 0; i < kNumJSCallerSaved; i++) {
-      int r = JSCallerSavedCode(i);
-      Register reg = { r };
-      if ((object_regs & (1 << r)) != 0) {
-        __ push(reg);
-      }
-    }
+    if (mode == SAVE_RESULT_REGISTER) __ push(eax);
 
-#ifdef DEBUG
-    __ RecordComment("// Calling from debug break to runtime - come in - over");
-#endif
     __ Move(eax, Immediate(0));  // No arguments.
     __ mov(ebx, Immediate(ExternalReference::debug_break(masm->isolate())));
 
     CEntryStub ceb(masm->isolate(), 1);
     __ CallStub(&ceb);
 
-    // Automatically find register that could be used after register restore.
-    // We need one register for padding skip instructions.
-    Register unused_reg = { -1 };
-
-    // Restore the register values containing object pointers from the
-    // expression stack.
-    for (int i = kNumJSCallerSaved; --i >= 0;) {
-      int r = JSCallerSavedCode(i);
-      Register reg = { r };
-      if (FLAG_debug_code) {
+    if (FLAG_debug_code) {
+      for (int i = 0; i < kNumJSCallerSaved; ++i) {
+        Register reg = {JSCallerSavedCode(i)};
         __ Move(reg, Immediate(kDebugZapValue));
       }
-      bool taken = reg.code() == esi.code();
-      if ((object_regs & (1 << r)) != 0) {
-        __ pop(reg);
-        taken = true;
-      }
-      if (!taken) {
-        unused_reg = reg;
-      }
     }
 
-    DCHECK(unused_reg.code() != -1);
+    if (mode == SAVE_RESULT_REGISTER) __ pop(eax);
 
-    // Read current padding counter and skip corresponding number of words.
-    __ pop(unused_reg);
+    __ pop(ebx);
     // We divide stored value by 2 (untagging) and multiply it by word's size.
     STATIC_ASSERT(kSmiTagSize == 1 && kSmiShiftSize == 0);
-    __ lea(esp, Operand(esp, unused_reg, times_half_pointer_size, 0));
+    __ lea(esp, Operand(esp, ebx, times_half_pointer_size, 0));
 
     // Get rid of the internal frame.
   }
@@ -148,33 +105,6 @@ static void Generate_DebugBreakCallHelper(MacroAssembler* masm,
 }
 
 
-void DebugCodegen::GenerateReturnDebugBreak(MacroAssembler* masm) {
-  // Register state just before return from JS function (from codegen-x87.cc).
-  // ----------- S t a t e -------------
-  //  -- eax: return value
-  // -----------------------------------
-  Generate_DebugBreakCallHelper(masm, eax.bit());
-}
-
-
-void DebugCodegen::GenerateSlot(MacroAssembler* masm,
-                                DebugCodegen::SlotLocation location,
-                                int call_argc) {
-  // Generate enough nop's to make space for a call instruction.
-  Label check_codesize;
-  __ bind(&check_codesize);
-  RecordRelocInfo(masm, location, call_argc);
-  __ Nop(Assembler::kDebugBreakSlotLength);
-  DCHECK_EQ(Assembler::kDebugBreakSlotLength,
-            masm->SizeOfCodeGeneratedSince(&check_codesize));
-}
-
-
-void DebugCodegen::GenerateSlotDebugBreak(MacroAssembler* masm) {
-  Generate_DebugBreakCallHelper(masm, 0);
-}
-
-
 void DebugCodegen::GeneratePlainReturnLiveEdit(MacroAssembler* masm) {
   masm->ret(0);
 }

src/x87/full-codegen-x87.cc

@@ -461,26 +461,14 @@ void FullCodeGenerator::EmitReturnSequence() {
     __ pop(eax);
     EmitProfilingCounterReset();
     __ bind(&ok);
-#ifdef DEBUG
-    // Add a label for checking the size of the code used for returning.
-    Label check_exit_codesize;
-    masm_->bind(&check_exit_codesize);
-#endif
+
     SetReturnPosition(function());
-    __ RecordJSReturn();
-    // Do not use the leave instruction here because it is too short to
-    // patch with the code required by the debugger.
-    __ mov(esp, ebp);
     int no_frame_start = masm_->pc_offset();
-    __ pop(ebp);
+    __ leave();
 
     int arg_count = info_->scope()->num_parameters() + 1;
     int arguments_bytes = arg_count * kPointerSize;
     __ Ret(arguments_bytes, ecx);
-    // Check that the size of the code used for returning is large enough
-    // for the debugger's requirements.
-    DCHECK(Assembler::kJSReturnSequenceLength <=
-           masm_->SizeOfCodeGeneratedSince(&check_exit_codesize));
     info_->AddNoFrameRange(no_frame_start, masm_->pc_offset());
   }
 }

src/x87/macro-assembler-x87.h

@@ -979,7 +979,7 @@ class MacroAssembler: public Assembler {
 class CodePatcher {
  public:
   CodePatcher(byte* address, int size);
-  virtual ~CodePatcher();
+  ~CodePatcher();
 
   // Macro assembler to emit code.
   MacroAssembler* masm() { return &masm_; }