Reviewers: Weiliang,

Message:
PTAL.

Description:
X87: Debugger: use debug break slot to break on call.

port 8965b683ce39bc3c24ed2466d189323d81a70852 (r29561)

original commit message:

    Break point at calls are currently set via IC. To change this, we
    need to set debug break slots instead. We also need to distinguish
    those debug break slots as calls to support step-in.

    To implement this, we add a data field to debug break reloc info to
    indicate non-call debug breaks or in case of call debug breaks, the
    number of arguments. We can later use this to find the callee on the
    evaluation stack in Debug::PrepareStep.

BUG=

Please review this at https://codereview.chromium.org/1233823002/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+14, -81 lines):
  M src/x87/assembler-x87.h
  M src/x87/debug-x87.cc
  M src/x87/full-codegen-x87.cc


Index: src/x87/assembler-x87.h
diff --git a/src/x87/assembler-x87.h b/src/x87/assembler-x87.h
index d8c86abf7a06e46b377284789e48de23c2e9c28f..5498b8eaedf0acea222f3cadfdb17d42193f9a08 100644
--- a/src/x87/assembler-x87.h
+++ b/src/x87/assembler-x87.h
@@ -956,6 +956,8 @@ class Assembler : public AssemblerBase {

   // Mark address of a debug break slot.
   void RecordDebugBreakSlot();
+  void RecordDebugBreakSlotForCall(int argc);
+  void RecordDebugBreakSlotForConstructCall();

   // Record a comment relocation entry that can be used by a disassembler.
   // Use --code-comments to enable.
Index: src/x87/debug-x87.cc
diff --git a/src/x87/debug-x87.cc b/src/x87/debug-x87.cc
index d0fcc82eaa0f8af1f7c667d0d486335d6a20412e..c7ef01ee6fa00281b9c8f34c2f413fedbd11a7f9 100644
--- a/src/x87/debug-x87.cc
+++ b/src/x87/debug-x87.cc
@@ -70,9 +70,7 @@ void BreakLocation::SetDebugBreakAtSlot() {
 #define __ ACCESS_MASM(masm)

 static void Generate_DebugBreakCallHelper(MacroAssembler* masm,
-                                          RegList object_regs,
-                                          RegList non_object_regs,
-                                          bool convert_call_to_jmp) {
+                                          RegList object_regs) {
   // Enter an internal frame.
   {
     FrameScope scope(masm, StackFrame::INTERNAL);
@@ -87,22 +85,12 @@ static void Generate_DebugBreakCallHelper(MacroAssembler* masm, // 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);
-    DCHECK((non_object_regs & ~kJSCallerSaved) == 0);
-    DCHECK((object_regs & non_object_regs) == 0);
     for (int i = 0; i < kNumJSCallerSaved; i++) {
       int r = JSCallerSavedCode(i);
       Register reg = { r };
       if ((object_regs & (1 << r)) != 0) {
         __ push(reg);
       }
-      if ((non_object_regs & (1 << r)) != 0) {
-        if (FLAG_debug_code) {
-          __ test(reg, Immediate(0xc0000000));
-          __ Assert(zero, kUnableToEncodeValueAsSmi);
-        }
-        __ SmiTag(reg);
-        __ push(reg);
-      }
     }

 #ifdef DEBUG
@@ -131,11 +119,6 @@ static void Generate_DebugBreakCallHelper(MacroAssembler* masm,
         __ pop(reg);
         taken = true;
       }
-      if ((non_object_regs & (1 << r)) != 0) {
-        __ pop(reg);
-        __ SmiUntag(reg);
-        taken = true;
-      }
       if (!taken) {
         unused_reg = reg;
       }
@@ -152,11 +135,9 @@ static void Generate_DebugBreakCallHelper(MacroAssembler* masm,
     // Get rid of the internal frame.
   }

- // If this call did not replace a call but patched other code then there will - // be an unwanted return address left on the stack. Here we get rid of that.
-  if (convert_call_to_jmp) {
-    __ add(esp, Immediate(kPointerSize));
-  }
+  // This call did not replace a call , so there will be an unwanted
+  // return address left on the stack. Here we get rid of that.
+  __ add(esp, Immediate(kPointerSize));

   // Now that the break point has been handled, resume normal execution by
   // jumping to the target address intended by the caller and that was
@@ -167,62 +148,12 @@ static void Generate_DebugBreakCallHelper(MacroAssembler* masm,
 }


-void DebugCodegen::GenerateCallICStubDebugBreak(MacroAssembler* masm) {
-  // Register state for CallICStub
-  // ----------- S t a t e -------------
-  //  -- edx    : type feedback slot (smi)
-  //  -- edi    : function
-  // -----------------------------------
-  Generate_DebugBreakCallHelper(masm, edx.bit() | edi.bit(),
-                                0, false);
-}
-
-
 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(), 0, true);
-}
-
-
-void DebugCodegen::GenerateCallFunctionStubDebugBreak(MacroAssembler* masm) {
-  // Register state for CallFunctionStub (from code-stubs-x87.cc).
-  // ----------- S t a t e -------------
-  //  -- edi: function
-  // -----------------------------------
-  Generate_DebugBreakCallHelper(masm, edi.bit(), 0, false);
-}
-
-
-void DebugCodegen::GenerateCallConstructStubDebugBreak(MacroAssembler* masm) {
-  // Register state for CallConstructStub (from code-stubs-x87.cc).
-  // eax is the actual number of arguments not encoded as a smi see comment
-  // above IC call.
-  // ----------- S t a t e -------------
-  //  -- eax: number of arguments (not smi)
-  //  -- edi: constructor function
-  // -----------------------------------
-  // The number of arguments in eax is not smi encoded.
-  Generate_DebugBreakCallHelper(masm, edi.bit(), eax.bit(), false);
-}
-
-
-void DebugCodegen::GenerateCallConstructStubRecordDebugBreak(
-    MacroAssembler* masm) {
-  // Register state for CallConstructStub (from code-stubs-x87.cc).
-  // eax is the actual number of arguments not encoded as a smi see comment
-  // above IC call.
-  // ----------- S t a t e -------------
-  //  -- eax: number of arguments (not smi)
-  //  -- ebx: feedback array
-  //  -- edx: feedback slot (smi)
-  //  -- edi: constructor function
-  // -----------------------------------
-  // The number of arguments in eax is not smi encoded.
-  Generate_DebugBreakCallHelper(masm, ebx.bit() | edx.bit() | edi.bit(),
-                                eax.bit(), false);
+  Generate_DebugBreakCallHelper(masm, eax.bit());
 }


@@ -230,7 +161,6 @@ void DebugCodegen::GenerateSlot(MacroAssembler* masm) {
   // Generate enough nop's to make space for a call instruction.
   Label check_codesize;
   __ bind(&check_codesize);
-  __ RecordDebugBreakSlot();
   __ Nop(Assembler::kDebugBreakSlotLength);
   DCHECK_EQ(Assembler::kDebugBreakSlotLength,
             masm->SizeOfCodeGeneratedSince(&check_codesize));
@@ -238,7 +168,7 @@ void DebugCodegen::GenerateSlot(MacroAssembler* masm) {


 void DebugCodegen::GenerateSlotDebugBreak(MacroAssembler* masm) {
-  Generate_DebugBreakCallHelper(masm, 0, 0, true);
+  Generate_DebugBreakCallHelper(masm, 0);
 }


Index: src/x87/full-codegen-x87.cc
diff --git a/src/x87/full-codegen-x87.cc b/src/x87/full-codegen-x87.cc
index 7b1a35dcd72d1dae92c8d8f5827dcebcd9957c52..f60ba1d9b517f2c4471a1da807e7707aca7fcb9b 100644
--- a/src/x87/full-codegen-x87.cc
+++ b/src/x87/full-codegen-x87.cc
@@ -2185,6 +2185,7 @@ void FullCodeGenerator::VisitYield(Yield* expr) {
       CallIC(ic, TypeFeedbackId::None());
       __ mov(edi, eax);
       __ mov(Operand(esp, 2 * kPointerSize), edi);
+      SetCallPosition(expr, 1);
       CallFunctionStub stub(isolate(), 1, CALL_AS_METHOD);
       __ CallStub(&stub);

@@ -2986,7 +2987,7 @@ void FullCodeGenerator::EmitCall(Call* expr, CallICState::CallType call_type) {
     VisitForStackValue(args->at(i));
   }

-  SetExpressionPosition(expr);
+  SetCallPosition(expr, arg_count);
Handle<Code> ic = CodeFactory::CallIC(isolate(), arg_count, call_type).code();
   __ Move(edx, Immediate(SmiFromSlot(expr->CallFeedbackICSlot())));
   __ mov(edi, Operand(esp, (arg_count + 1) * kPointerSize));
@@ -3117,7 +3118,7 @@ void FullCodeGenerator::VisitCall(Call* expr) {

     PrepareForBailoutForId(expr->EvalId(), NO_REGISTERS);

-    SetExpressionPosition(expr);
+    SetCallPosition(expr, arg_count);
     CallFunctionStub stub(isolate(), arg_count, NO_CALL_FUNCTION_FLAGS);
     __ mov(edi, Operand(esp, (arg_count + 1) * kPointerSize));
     __ CallStub(&stub);
@@ -3188,7 +3189,7 @@ void FullCodeGenerator::VisitCallNew(CallNew* expr) {

   // Call the construct call builtin that handles allocation and
   // constructor invocation.
-  SetExpressionPosition(expr);
+  SetConstructCallPosition(expr);

   // Load function and argument count into edi and eax.
   __ Move(eax, Immediate(arg_count));
@@ -3231,7 +3232,7 @@ void FullCodeGenerator::EmitSuperConstructorCall(Call* expr) {

   // Call the construct call builtin that handles allocation and
   // constructor invocation.
-  SetExpressionPosition(expr);
+  SetConstructCallPosition(expr);

   // Load function and argument count into edi and eax.
   __ Move(eax, Immediate(arg_count));
@@ -4653,7 +4654,7 @@ void FullCodeGenerator::EmitCallJSRuntimeFunction(CallRuntime* expr) {
   ZoneList<Expression*>* args = expr->arguments();
   int arg_count = args->length();

-  SetExpressionPosition(expr);
+  SetCallPosition(expr, arg_count);
   CallFunctionStub stub(isolate(), arg_count, NO_CALL_FUNCTION_FLAGS);
   __ mov(edi, Operand(esp, (arg_count + 1) * kPointerSize));
   __ CallStub(&stub);


--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to