Revert of [debugger] do not predict step in target for liveedit. (patchset #2 id:20001 of https://codereview.chromium.org/1491743005/ )

Reason for revert:
[Sheriff] And it still breaks:
https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/3239

Please run chromium trybots on relands of CLs that broke chromium bots.

Original issue's description:
> [debugger] do not predict step in target for liveedit.
>
> R=verwaest@chromium.org
>
> Committed: https://crrev.com/8f87ff5d62e996b07ffbde7e735daa603c1d7290
> Cr-Commit-Position: refs/heads/master@{#32553}
>
> Committed: https://crrev.com/00559c4584fe3a4c3c1a8d3a5b5af0611b19c40a
> Cr-Commit-Position: refs/heads/master@{#32600}

TBR=verwaest@chromium.org,yangguo@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

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

Cr-Commit-Position: refs/heads/master@{#32607}
This commit is contained in:
machenbach 2015-12-04 02:43:00 -08:00 committed by Commit bot
parent 154a493cb7
commit 6f4d477f32
20 changed files with 175 additions and 55 deletions

View File

@ -652,10 +652,6 @@ class MacroAssembler: public Assembler {
const ParameterCount& actual, InvokeFlag flag,
const CallWrapper& call_wrapper);
void FloodFunctionIfStepping(Register fun, Register new_target,
const ParameterCount& expected,
const ParameterCount& actual);
// Invoke the JavaScript function in the given register. Changes the
// current context to the context in the function before invoking.
void InvokeFunction(Register function,
@ -1458,6 +1454,10 @@ class MacroAssembler: public Assembler {
InvokeFlag flag,
const CallWrapper& call_wrapper);
void FloodFunctionIfStepping(Register fun, Register new_target,
const ParameterCount& expected,
const ParameterCount& actual);
void InitializeNewString(Register string,
Register length,
Heap::RootListIndex map_index,

View File

@ -1417,6 +1417,14 @@ ExternalReference ExternalReference::debug_after_break_target_address(
}
ExternalReference
ExternalReference::debug_restarter_frame_function_pointer_address(
Isolate* isolate) {
return ExternalReference(
isolate->debug()->restarter_frame_function_pointer_address());
}
ExternalReference ExternalReference::virtual_handler_register(
Isolate* isolate) {
return ExternalReference(isolate->virtual_handler_register_address());

View File

@ -976,6 +976,8 @@ class ExternalReference BASE_EMBEDDED {
static ExternalReference debug_is_active_address(Isolate* isolate);
static ExternalReference debug_after_break_target_address(Isolate* isolate);
static ExternalReference debug_restarter_frame_function_pointer_address(
Isolate* isolate);
static ExternalReference is_profiling_address(Isolate* isolate);
static ExternalReference invoke_function_callback(Isolate* isolate);

View File

@ -2215,6 +2215,11 @@ static void Generate_Slot_DebugBreak(MacroAssembler* masm) {
}
static void Generate_PlainReturn_LiveEdit(MacroAssembler* masm) {
DebugCodegen::GeneratePlainReturnLiveEdit(masm);
}
static void Generate_FrameDropper_LiveEdit(MacroAssembler* masm) {
DebugCodegen::GenerateFrameDropperLiveEdit(masm);
}

View File

@ -197,6 +197,7 @@ inline bool operator&(BuiltinExtraArguments lhs, BuiltinExtraArguments rhs) {
#define BUILTIN_LIST_DEBUG_A(V) \
V(Return_DebugBreak, BUILTIN, DEBUG_STUB, kNoExtraICState) \
V(Slot_DebugBreak, BUILTIN, DEBUG_STUB, kNoExtraICState) \
V(PlainReturn_LiveEdit, BUILTIN, DEBUG_STUB, kNoExtraICState) \
V(FrameDropper_LiveEdit, BUILTIN, DEBUG_STUB, kNoExtraICState)

View File

@ -113,7 +113,19 @@ void DebugCodegen::GenerateDebugBreakStub(MacroAssembler* masm,
}
void DebugCodegen::GeneratePlainReturnLiveEdit(MacroAssembler* masm) {
__ Ret();
}
void DebugCodegen::GenerateFrameDropperLiveEdit(MacroAssembler* masm) {
ExternalReference restarter_frame_function_slot =
ExternalReference::debug_restarter_frame_function_pointer_address(
masm->isolate());
__ mov(ip, Operand(restarter_frame_function_slot));
__ mov(r1, Operand::Zero());
__ str(r1, MemOperand(ip, 0));
// Load the function pointer off of our current stack frame.
__ ldr(r1, MemOperand(fp,
StandardFrameConstants::kConstantPoolOffset - kPointerSize));
@ -122,9 +134,6 @@ void DebugCodegen::GenerateFrameDropperLiveEdit(MacroAssembler* masm) {
// FLAG_enable_embedded_constant_pool).
__ LeaveFrame(StackFrame::INTERNAL);
ParameterCount dummy(0);
__ FloodFunctionIfStepping(r1, no_reg, dummy, dummy);
{ ConstantPoolUnavailableScope constant_pool_unavailable(masm);
// Load context from the function.
__ ldr(cp, FieldMemOperand(r1, JSFunction::kContextOffset));

View File

@ -124,19 +124,27 @@ void DebugCodegen::GenerateDebugBreakStub(MacroAssembler* masm,
}
void DebugCodegen::GeneratePlainReturnLiveEdit(MacroAssembler* masm) {
__ Ret();
}
void DebugCodegen::GenerateFrameDropperLiveEdit(MacroAssembler* masm) {
ExternalReference restarter_frame_function_slot =
ExternalReference::debug_restarter_frame_function_pointer_address(
masm->isolate());
UseScratchRegisterScope temps(masm);
Register scratch = temps.AcquireX();
__ Mov(scratch, restarter_frame_function_slot);
__ Str(xzr, MemOperand(scratch));
// We do not know our frame height, but set sp based on fp.
__ Sub(masm->StackPointer(), fp, kPointerSize);
__ AssertStackConsistency();
__ Pop(x1, fp, lr); // Function, Frame, Return address.
ParameterCount dummy(0);
__ FloodFunctionIfStepping(x1, no_reg, dummy, dummy);
UseScratchRegisterScope temps(masm);
Register scratch = temps.AcquireX();
// Load context from the function.
__ Ldr(cp, FieldMemOperand(x1, JSFunction::kContextOffset));

View File

@ -336,6 +336,7 @@ void Debug::ThreadInit() {
// TODO(isolates): frames_are_dropped_?
base::NoBarrier_Store(&thread_local_.current_debug_scope_,
static_cast<base::AtomicWord>(0));
thread_local_.restarter_frame_function_pointer_ = NULL;
}
@ -930,6 +931,17 @@ void Debug::PrepareStep(StepAction step_action,
return;
}
STATIC_ASSERT(StepFrame > StepIn);
if (step_action >= StepIn) {
// If there's restarter frame on top of the stack, just get the pointer
// to function which is going to be restarted.
if (thread_local_.restarter_frame_function_pointer_ != NULL) {
Handle<JSFunction> restarted_function(
JSFunction::cast(*thread_local_.restarter_frame_function_pointer_));
FloodWithOneShot(restarted_function);
}
}
// Fill the current function with one-shot break points even for step in on
// a call target as the function called might be a native function for
// which step in will not stop. It also prepares for stepping in
@ -1510,11 +1522,14 @@ bool Debug::IsBreakAtReturn(JavaScriptFrame* frame) {
void Debug::FramesHaveBeenDropped(StackFrame::Id new_break_frame_id,
LiveEdit::FrameDropMode mode) {
LiveEdit::FrameDropMode mode,
Object** restarter_frame_function_pointer) {
if (mode != LiveEdit::CURRENTLY_SET_MODE) {
thread_local_.frame_drop_mode_ = mode;
}
thread_local_.break_frame_id_ = new_break_frame_id;
thread_local_.restarter_frame_function_pointer_ =
restarter_frame_function_pointer;
}

View File

@ -455,7 +455,8 @@ class Debug {
// Support for LiveEdit
void FramesHaveBeenDropped(StackFrame::Id new_break_frame_id,
LiveEdit::FrameDropMode mode);
LiveEdit::FrameDropMode mode,
Object** restarter_frame_function_pointer);
// Threading support.
char* ArchiveDebug(char* to);
@ -502,6 +503,11 @@ class Debug {
return reinterpret_cast<Address>(&after_break_target_);
}
Address restarter_frame_function_pointer_address() {
Object*** address = &thread_local_.restarter_frame_function_pointer_;
return reinterpret_cast<Address>(address);
}
Address step_in_enabled_address() {
return reinterpret_cast<Address>(&thread_local_.step_in_enabled_);
}
@ -650,6 +656,11 @@ class Debug {
// Stores the way how LiveEdit has patched the stack. It is used when
// debugger returns control back to user script.
LiveEdit::FrameDropMode frame_drop_mode_;
// When restarter frame is on stack, stores the address
// of the pointer to function being restarted. Otherwise (most of the time)
// stores NULL. This pointer is used with 'step in' implementation.
Object** restarter_frame_function_pointer_;
};
// Storage location for registers when handling debug break calls
@ -746,6 +757,8 @@ class DebugCodegen : public AllStatic {
static void GenerateDebugBreakStub(MacroAssembler* masm,
DebugBreakCallHelperMode mode);
static void GeneratePlainReturnLiveEdit(MacroAssembler* masm);
// FrameDropper is a code replacement for a JavaScript frame with possibly
// several frames above.
// There is no calling conventions here, because it never actually gets

View File

@ -105,16 +105,23 @@ void DebugCodegen::GenerateDebugBreakStub(MacroAssembler* masm,
}
void DebugCodegen::GeneratePlainReturnLiveEdit(MacroAssembler* masm) {
masm->ret(0);
}
void DebugCodegen::GenerateFrameDropperLiveEdit(MacroAssembler* masm) {
ExternalReference restarter_frame_function_slot =
ExternalReference::debug_restarter_frame_function_pointer_address(
masm->isolate());
__ mov(Operand::StaticVariable(restarter_frame_function_slot), Immediate(0));
// We do not know our frame height, but set esp based on ebp.
__ lea(esp, Operand(ebp, -1 * kPointerSize));
__ pop(edi); // Function.
__ pop(ebp);
ParameterCount dummy(0);
__ FloodFunctionIfStepping(edi, no_reg, dummy, dummy);
// Load context from the function.
__ mov(esi, FieldOperand(edi, JSFunction::kContextOffset));

View File

@ -811,6 +811,10 @@ bool LiveEdit::SetAfterBreakTarget(Debug* debug) {
switch (debug->thread_local_.frame_drop_mode_) {
case FRAMES_UNTOUCHED:
return false;
case FRAME_DROPPED_IN_IC_CALL:
// We must have been calling IC stub. Do not go there anymore.
code = isolate->builtins()->builtin(Builtins::kPlainReturn_LiveEdit);
break;
case FRAME_DROPPED_IN_DEBUG_SLOT_CALL:
// Debug break slot stub does not return normally, instead it manually
// cleans the stack and jumps. We should patch the jump address.
@ -1487,13 +1491,17 @@ static bool FixTryCatchHandler(StackFrame* top_frame,
// a. successful work of frame dropper code which eventually gets control,
// b. being compatible with regular stack structure for various stack
// iterators.
// Returns address of stack allocated pointer to restarted function,
// the value that is called 'restarter_frame_function_pointer'. The value
// at this address (possibly updated by GC) may be used later when preparing
// 'step in' operation.
// Frame structure (conforms InternalFrame structure):
// -- code
// -- SMI maker
// -- function (slot is called "context")
// -- frame base
static void SetUpFrameDropperFrame(StackFrame* bottom_js_frame,
Handle<Code> code) {
static Object** SetUpFrameDropperFrame(StackFrame* bottom_js_frame,
Handle<Code> code) {
DCHECK(bottom_js_frame->is_java_script());
Address fp = bottom_js_frame->fp();
@ -1505,6 +1513,9 @@ static void SetUpFrameDropperFrame(StackFrame* bottom_js_frame,
Memory::Object_at(fp + InternalFrameConstants::kCodeOffset) = *code;
Memory::Object_at(fp + StandardFrameConstants::kMarkerOffset) =
Smi::FromInt(StackFrame::INTERNAL);
return reinterpret_cast<Object**>(&Memory::Object_at(
fp + StandardFrameConstants::kContextOffset));
}
@ -1512,9 +1523,11 @@ static void SetUpFrameDropperFrame(StackFrame* bottom_js_frame,
// frames in range. Anyway the bottom frame is restarted rather than dropped,
// and therefore has to be a JavaScript frame.
// Returns error message or NULL.
static const char* DropFrames(Vector<StackFrame*> frames, int top_frame_index,
static const char* DropFrames(Vector<StackFrame*> frames,
int top_frame_index,
int bottom_js_frame_index,
LiveEdit::FrameDropMode* mode) {
LiveEdit::FrameDropMode* mode,
Object*** restarter_frame_function_pointer) {
if (!LiveEdit::kFrameDropperSupported) {
return "Stack manipulations are not supported in this architecture.";
}
@ -1529,8 +1542,12 @@ static const char* DropFrames(Vector<StackFrame*> frames, int top_frame_index,
Isolate* isolate = bottom_js_frame->isolate();
Code* pre_top_frame_code = pre_top_frame->LookupCode();
bool frame_has_padding = true;
if (pre_top_frame_code ==
isolate->builtins()->builtin(Builtins::kSlot_DebugBreak)) {
if (pre_top_frame_code->is_inline_cache_stub() &&
pre_top_frame_code->is_debug_stub()) {
// OK, we can drop inline cache calls.
*mode = LiveEdit::FRAME_DROPPED_IN_IC_CALL;
} else if (pre_top_frame_code ==
isolate->builtins()->builtin(Builtins::kSlot_DebugBreak)) {
// OK, we can drop debug break slot.
*mode = LiveEdit::FRAME_DROPPED_IN_DEBUG_SLOT_CALL;
} else if (pre_top_frame_code ==
@ -1624,7 +1641,10 @@ static const char* DropFrames(Vector<StackFrame*> frames, int top_frame_index,
*top_frame_pc_address = code->entry();
pre_top_frame->SetCallerFp(bottom_js_frame->fp());
SetUpFrameDropperFrame(bottom_js_frame, code);
*restarter_frame_function_pointer =
SetUpFrameDropperFrame(bottom_js_frame, code);
DCHECK((**restarter_frame_function_pointer)->IsJSFunction());
for (Address a = unused_stack_top;
a < unused_stack_bottom;
@ -1785,8 +1805,10 @@ static const char* DropActivationsInActiveThreadImpl(Isolate* isolate,
}
LiveEdit::FrameDropMode drop_mode = LiveEdit::FRAMES_UNTOUCHED;
const char* error_message =
DropFrames(frames, top_frame_index, bottom_js_frame_index, &drop_mode);
Object** restarter_frame_function_pointer = NULL;
const char* error_message = DropFrames(frames, top_frame_index,
bottom_js_frame_index, &drop_mode,
&restarter_frame_function_pointer);
if (error_message != NULL) {
return error_message;
@ -1800,7 +1822,8 @@ static const char* DropActivationsInActiveThreadImpl(Isolate* isolate,
break;
}
}
debug->FramesHaveBeenDropped(new_id, drop_mode);
debug->FramesHaveBeenDropped(
new_id, drop_mode, restarter_frame_function_pointer);
return NULL;
}

View File

@ -61,6 +61,8 @@ class LiveEdit : AllStatic {
enum FrameDropMode {
// No frame has been dropped.
FRAMES_UNTOUCHED,
// The top JS frame had been calling IC stub. IC stub mustn't be called now.
FRAME_DROPPED_IN_IC_CALL,
// The top JS frame had been calling debug break slot stub. Patch the
// address this stub jumps to in the end.
FRAME_DROPPED_IN_DEBUG_SLOT_CALL,

View File

@ -108,15 +108,23 @@ void DebugCodegen::GenerateDebugBreakStub(MacroAssembler* masm,
}
void DebugCodegen::GeneratePlainReturnLiveEdit(MacroAssembler* masm) {
__ Ret();
}
void DebugCodegen::GenerateFrameDropperLiveEdit(MacroAssembler* masm) {
ExternalReference restarter_frame_function_slot =
ExternalReference::debug_restarter_frame_function_pointer_address(
masm->isolate());
__ li(at, Operand(restarter_frame_function_slot));
__ sw(zero_reg, MemOperand(at, 0));
// We do not know our frame height, but set sp based on fp.
__ Subu(sp, fp, Operand(kPointerSize));
__ Pop(ra, fp, a1); // Return address, Frame, Function.
ParameterCount dummy(0);
__ FloodFunctionIfStepping(a1, no_reg, dummy, dummy);
// Load context from the function.
__ lw(cp, FieldMemOperand(a1, JSFunction::kContextOffset));

View File

@ -110,15 +110,23 @@ void DebugCodegen::GenerateDebugBreakStub(MacroAssembler* masm,
}
void DebugCodegen::GeneratePlainReturnLiveEdit(MacroAssembler* masm) {
__ Ret();
}
void DebugCodegen::GenerateFrameDropperLiveEdit(MacroAssembler* masm) {
ExternalReference restarter_frame_function_slot =
ExternalReference::debug_restarter_frame_function_pointer_address(
masm->isolate());
__ li(at, Operand(restarter_frame_function_slot));
__ sw(zero_reg, MemOperand(at, 0));
// We do not know our frame height, but set sp based on fp.
__ Dsubu(sp, fp, Operand(kPointerSize));
__ Pop(ra, fp, a1); // Return address, Frame, Function.
ParameterCount dummy(0);
__ FloodFunctionIfStepping(a1, no_reg, dummy, dummy);
// Load context from the function.
__ ld(cp, FieldMemOperand(a1, JSFunction::kContextOffset));

View File

@ -106,16 +106,24 @@ void DebugCodegen::GenerateDebugBreakStub(MacroAssembler* masm,
}
void DebugCodegen::GeneratePlainReturnLiveEdit(MacroAssembler* masm) {
masm->ret(0);
}
void DebugCodegen::GenerateFrameDropperLiveEdit(MacroAssembler* masm) {
ExternalReference restarter_frame_function_slot =
ExternalReference::debug_restarter_frame_function_pointer_address(
masm->isolate());
__ Move(rax, restarter_frame_function_slot);
__ movp(Operand(rax, 0), Immediate(0));
// We do not know our frame height, but set rsp based on rbp.
__ leap(rsp, Operand(rbp, -1 * kPointerSize));
__ Pop(rdi); // Function.
__ popq(rbp);
ParameterCount dummy(0);
__ FloodFunctionIfStepping(rdi, no_reg, dummy, dummy);
// Load context from the function.
__ movp(rsi, FieldOperand(rdi, JSFunction::kContextOffset));

View File

@ -315,10 +315,6 @@ class MacroAssembler: public Assembler {
const ParameterCount& actual, InvokeFlag flag,
const CallWrapper& call_wrapper);
void FloodFunctionIfStepping(Register fun, Register new_target,
const ParameterCount& expected,
const ParameterCount& actual);
// Invoke the JavaScript function in the given register. Changes the
// current context to the context in the function before invoking.
void InvokeFunction(Register function, Register new_target,
@ -901,6 +897,10 @@ class MacroAssembler: public Assembler {
Label::Distance done_distance,
const CallWrapper& call_wrapper);
void FloodFunctionIfStepping(Register fun, Register new_target,
const ParameterCount& expected,
const ParameterCount& actual);
void EnterExitFramePrologue();
void EnterExitFrameEpilogue(int argc, bool save_doubles);

View File

@ -979,10 +979,6 @@ class MacroAssembler: public Assembler {
const ParameterCount& actual, InvokeFlag flag,
const CallWrapper& call_wrapper);
void FloodFunctionIfStepping(Register fun, Register new_target,
const ParameterCount& expected,
const ParameterCount& actual);
// Invoke the JavaScript function in the given register. Changes the
// current context to the context in the function before invoking.
void InvokeFunction(Register function,
@ -1673,6 +1669,10 @@ const Operand& rt = Operand(zero_reg), BranchDelaySlot bd = PROTECT
InvokeFlag flag,
const CallWrapper& call_wrapper);
void FloodFunctionIfStepping(Register fun, Register new_target,
const ParameterCount& expected,
const ParameterCount& actual);
void InitializeNewString(Register string,
Register length,
Heap::RootListIndex map_index,

View File

@ -1034,10 +1034,6 @@ class MacroAssembler: public Assembler {
const ParameterCount& actual, InvokeFlag flag,
const CallWrapper& call_wrapper);
void FloodFunctionIfStepping(Register fun, Register new_target,
const ParameterCount& expected,
const ParameterCount& actual);
// Invoke the JavaScript function in the given register. Changes the
// current context to the context in the function before invoking.
void InvokeFunction(Register function,
@ -1769,6 +1765,10 @@ const Operand& rt = Operand(zero_reg), BranchDelaySlot bd = PROTECT
InvokeFlag flag,
const CallWrapper& call_wrapper);
void FloodFunctionIfStepping(Register fun, Register new_target,
const ParameterCount& expected,
const ParameterCount& actual);
void InitializeNewString(Register string,
Register length,
Heap::RootListIndex map_index,

View File

@ -60,6 +60,8 @@ ExternalReferenceTable::ExternalReferenceTable(Isolate* isolate) {
"Heap::NewSpaceAllocationLimitAddress()");
Add(ExternalReference::new_space_allocation_top_address(isolate).address(),
"Heap::NewSpaceAllocationTopAddress()");
Add(ExternalReference::debug_step_in_enabled_address(isolate).address(),
"Debug::step_in_enabled_address()");
Add(ExternalReference::mod_two_doubles_operation(isolate).address(),
"mod_two_doubles");
// Keyed lookup cache.
@ -137,10 +139,11 @@ ExternalReferenceTable::ExternalReferenceTable(Isolate* isolate) {
// Debug addresses
Add(ExternalReference::debug_after_break_target_address(isolate).address(),
"Debug::after_break_target_address()");
Add(ExternalReference::debug_restarter_frame_function_pointer_address(isolate)
.address(),
"Debug::restarter_frame_function_pointer_address()");
Add(ExternalReference::debug_is_active_address(isolate).address(),
"Debug::is_active_address()");
Add(ExternalReference::debug_step_in_enabled_address(isolate).address(),
"Debug::step_in_enabled_address()");
#ifndef V8_INTERPRETED_REGEXP
Add(ExternalReference::re_case_insensitive_compare_uc16(isolate).address(),

View File

@ -385,10 +385,6 @@ class MacroAssembler: public Assembler {
const ParameterCount& actual, InvokeFlag flag,
const CallWrapper& call_wrapper);
void FloodFunctionIfStepping(Register fun, Register new_target,
const ParameterCount& expected,
const ParameterCount& actual);
// Invoke the JavaScript function in the given register. Changes the
// current context to the context in the function before invoking.
void InvokeFunction(Register function,
@ -1608,6 +1604,10 @@ class MacroAssembler: public Assembler {
Label::Distance near_jump,
const CallWrapper& call_wrapper);
void FloodFunctionIfStepping(Register fun, Register new_target,
const ParameterCount& expected,
const ParameterCount& actual);
void EnterExitFramePrologue(bool save_rax);
// Allocates arg_stack_space * kPointerSize memory (not GCed) on the stack