[debugger] fix step-next for tail calls.

R=ishell@chromium.org
BUG=v8:4698
LOG=N

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

Cr-Commit-Position: refs/heads/master@{#35230}
This commit is contained in:
yangguo 2016-04-04 02:38:41 -07:00 committed by Commit bot
parent cc8e11b785
commit 3d4f85ab9f
19 changed files with 115 additions and 23 deletions

View File

@ -2875,7 +2875,7 @@ void Assembler::RecordRelocInfo(RelocInfo::Mode rmode, intptr_t data) {
// We do not try to reuse pool constants.
RelocInfo rinfo(isolate(), reinterpret_cast<byte*>(pc_), rmode, data, NULL);
if (((rmode >= RelocInfo::COMMENT) &&
(rmode <= RelocInfo::DEBUG_BREAK_SLOT_AT_CALL)) ||
(rmode <= RelocInfo::DEBUG_BREAK_SLOT_AT_TAIL_CALL)) ||
(rmode == RelocInfo::INTERNAL_REFERENCE) ||
(rmode == RelocInfo::CONST_POOL) || (rmode == RelocInfo::VENEER_POOL) ||
(rmode == RelocInfo::DEOPT_REASON) ||

View File

@ -838,6 +838,8 @@ const char* RelocInfo::RelocModeName(RelocInfo::Mode rmode) {
return "debug break slot at return";
case DEBUG_BREAK_SLOT_AT_CALL:
return "debug break slot at call";
case DEBUG_BREAK_SLOT_AT_TAIL_CALL:
return "debug break slot at tail call";
case CODE_AGE_SEQUENCE:
return "code age sequence";
case GENERATOR_CONTINUATION:
@ -936,6 +938,7 @@ void RelocInfo::Verify(Isolate* isolate) {
case DEBUG_BREAK_SLOT_AT_POSITION:
case DEBUG_BREAK_SLOT_AT_RETURN:
case DEBUG_BREAK_SLOT_AT_CALL:
case DEBUG_BREAK_SLOT_AT_TAIL_CALL:
case GENERATOR_CONTINUATION:
case WASM_MEMORY_REFERENCE:
case NONE32:

View File

@ -398,6 +398,7 @@ class RelocInfo {
DEBUG_BREAK_SLOT_AT_POSITION,
DEBUG_BREAK_SLOT_AT_RETURN,
DEBUG_BREAK_SLOT_AT_CALL,
DEBUG_BREAK_SLOT_AT_TAIL_CALL,
EXTERNAL_REFERENCE, // The address of an external C++ function.
INTERNAL_REFERENCE, // An address inside the same function.
@ -491,7 +492,7 @@ class RelocInfo {
}
static inline bool IsDebugBreakSlot(Mode mode) {
return IsDebugBreakSlotAtPosition(mode) || IsDebugBreakSlotAtReturn(mode) ||
IsDebugBreakSlotAtCall(mode);
IsDebugBreakSlotAtCall(mode) || IsDebugBreakSlotAtTailCall(mode);
}
static inline bool IsDebugBreakSlotAtPosition(Mode mode) {
return mode == DEBUG_BREAK_SLOT_AT_POSITION;
@ -502,6 +503,9 @@ class RelocInfo {
static inline bool IsDebugBreakSlotAtCall(Mode mode) {
return mode == DEBUG_BREAK_SLOT_AT_CALL;
}
static inline bool IsDebugBreakSlotAtTailCall(Mode mode) {
return mode == DEBUG_BREAK_SLOT_AT_TAIL_CALL;
}
static inline bool IsDebuggerStatement(Mode mode) {
return mode == DEBUGGER_STATEMENT;
}

View File

@ -108,6 +108,9 @@ int BreakLocation::CodeIterator::GetModeMask(BreakLocatorType type) {
mask |= RelocInfo::ModeMask(RelocInfo::STATEMENT_POSITION);
mask |= RelocInfo::ModeMask(RelocInfo::DEBUG_BREAK_SLOT_AT_RETURN);
mask |= RelocInfo::ModeMask(RelocInfo::DEBUG_BREAK_SLOT_AT_CALL);
if (isolate()->is_tail_call_elimination_enabled()) {
mask |= RelocInfo::ModeMask(RelocInfo::DEBUG_BREAK_SLOT_AT_TAIL_CALL);
}
if (type == ALL_BREAK_LOCATIONS) {
mask |= RelocInfo::ModeMask(RelocInfo::DEBUG_BREAK_SLOT_AT_POSITION);
mask |= RelocInfo::ModeMask(RelocInfo::DEBUGGER_STATEMENT);
@ -161,6 +164,10 @@ BreakLocation BreakLocation::CodeIterator::GetBreakLocation() {
type = DEBUG_BREAK_SLOT_AT_RETURN;
} else if (RelocInfo::IsDebugBreakSlotAtCall(rmode())) {
type = DEBUG_BREAK_SLOT_AT_CALL;
} else if (RelocInfo::IsDebugBreakSlotAtTailCall(rmode())) {
type = isolate()->is_tail_call_elimination_enabled()
? DEBUG_BREAK_SLOT_AT_TAIL_CALL
: DEBUG_BREAK_SLOT_AT_CALL;
} else if (RelocInfo::IsDebuggerStatement(rmode())) {
type = DEBUGGER_STATEMENT;
} else if (RelocInfo::IsDebugBreakSlot(rmode())) {
@ -226,6 +233,10 @@ BreakLocation::BytecodeArrayIterator::GetDebugBreakType() {
return DEBUGGER_STATEMENT;
} else if (bytecode == interpreter::Bytecode::kReturn) {
return DEBUG_BREAK_SLOT_AT_RETURN;
} else if (bytecode == interpreter::Bytecode::kTailCall) {
return isolate()->is_tail_call_elimination_enabled()
? DEBUG_BREAK_SLOT_AT_TAIL_CALL
: DEBUG_BREAK_SLOT_AT_CALL;
} else if (interpreter::Bytecodes::IsCallOrNew(bytecode)) {
return DEBUG_BREAK_SLOT_AT_CALL;
} else if (source_position_iterator_.is_statement()) {
@ -604,22 +615,26 @@ void Debug::Break(JavaScriptFrame* frame) {
Address target_fp = thread_local_.target_fp_;
Address last_fp = thread_local_.last_fp_;
bool step_break = true;
bool step_break = false;
switch (step_action) {
case StepNone:
return;
case StepOut:
// Step out has not reached the target frame yet.
if (current_fp < target_fp) return;
step_break = true;
break;
case StepNext:
// Step next should not break in a deeper frame.
if (current_fp < target_fp) return;
// For step-next, a tail call is like a return and should break.
step_break = location.IsTailCall();
// Fall through.
case StepIn: {
FrameSummary summary = GetFirstFrameSummary(frame);
int offset = summary.code_offset();
step_break = location.IsReturn() || (current_fp != last_fp) ||
step_break = step_break || location.IsReturn() ||
(current_fp != last_fp) ||
(thread_local_.last_statement_position_ !=
location.abstract_code()->SourceStatementPosition(offset));
break;
@ -1015,8 +1030,10 @@ void Debug::PrepareStep(StepAction step_action) {
BreakLocation location =
BreakLocation::FromCodeOffset(debug_info, call_offset);
// At a return statement we will step out either way.
// Any step at a return is a step-out.
if (location.IsReturn()) step_action = StepOut;
// A step-next at a tail call is a step-out.
if (location.IsTailCall() && step_action == StepNext) step_action = StepOut;
thread_local_.last_statement_position_ =
debug_info->abstract_code()->SourceStatementPosition(
@ -1592,7 +1609,7 @@ bool Debug::IsBreakAtReturn(JavaScriptFrame* frame) {
Handle<DebugInfo> debug_info(shared->GetDebugInfo());
BreakLocation location =
BreakLocation::FromCodeOffset(debug_info, summary.code_offset());
return location.IsReturn();
return location.IsReturn() || location.IsTailCall();
}

View File

@ -81,6 +81,9 @@ class BreakLocation {
inline bool IsReturn() const { return type_ == DEBUG_BREAK_SLOT_AT_RETURN; }
inline bool IsCall() const { return type_ == DEBUG_BREAK_SLOT_AT_CALL; }
inline bool IsTailCall() const {
return type_ == DEBUG_BREAK_SLOT_AT_TAIL_CALL;
}
inline bool IsDebugBreakSlot() const { return type_ >= DEBUG_BREAK_SLOT; }
inline bool IsDebuggerStatement() const {
return type_ == DEBUGGER_STATEMENT;
@ -113,7 +116,8 @@ class BreakLocation {
DEBUGGER_STATEMENT,
DEBUG_BREAK_SLOT,
DEBUG_BREAK_SLOT_AT_CALL,
DEBUG_BREAK_SLOT_AT_RETURN
DEBUG_BREAK_SLOT_AT_RETURN,
DEBUG_BREAK_SLOT_AT_TAIL_CALL,
};
BreakLocation(Handle<DebugInfo> debug_info, DebugBreakType type,
@ -140,6 +144,8 @@ class BreakLocation {
explicit Iterator(Handle<DebugInfo> debug_info);
int ReturnPosition();
Isolate* isolate() { return debug_info_->GetIsolate(); }
Handle<DebugInfo> debug_info_;
int break_index_;
int position_;
@ -166,7 +172,7 @@ class BreakLocation {
}
private:
static int GetModeMask(BreakLocatorType type);
int GetModeMask(BreakLocatorType type);
RelocInfo::Mode rmode() { return reloc_iterator_.rinfo()->rmode(); }
RelocInfo* rinfo() { return reloc_iterator_.rinfo(); }

View File

@ -2638,7 +2638,7 @@ void FullCodeGenerator::EmitCall(Call* expr, ConvertReceiverMode mode) {
}
PrepareForBailoutForId(expr->CallId(), NO_REGISTERS);
SetCallPosition(expr);
SetCallPosition(expr, expr->tail_call_mode());
if (expr->tail_call_mode() == TailCallMode::kAllow) {
if (FLAG_trace) {
__ CallRuntime(Runtime::kTraceTailCall);

View File

@ -2436,7 +2436,7 @@ void FullCodeGenerator::EmitCall(Call* expr, ConvertReceiverMode mode) {
}
PrepareForBailoutForId(expr->CallId(), NO_REGISTERS);
SetCallPosition(expr);
SetCallPosition(expr, expr->tail_call_mode());
if (expr->tail_call_mode() == TailCallMode::kAllow) {
if (FLAG_trace) {
__ CallRuntime(Runtime::kTraceTailCall);

View File

@ -671,13 +671,16 @@ void FullCodeGenerator::SetExpressionAsStatementPosition(Expression* expr) {
}
}
void FullCodeGenerator::SetCallPosition(Expression* expr) {
void FullCodeGenerator::SetCallPosition(Expression* expr,
TailCallMode tail_call_mode) {
if (expr->position() == RelocInfo::kNoPosition) return;
RecordPosition(masm_, expr->position());
if (info_->is_debug()) {
RelocInfo::Mode mode = (tail_call_mode == TailCallMode::kAllow)
? RelocInfo::DEBUG_BREAK_SLOT_AT_TAIL_CALL
: RelocInfo::DEBUG_BREAK_SLOT_AT_CALL;
// Always emit a debug break slot before a call.
DebugCodegen::GenerateSlot(masm_, RelocInfo::DEBUG_BREAK_SLOT_AT_CALL);
DebugCodegen::GenerateSlot(masm_, mode);
}
}

View File

@ -677,7 +677,8 @@ class FullCodeGenerator: public AstVisitor {
// This is used in loop headers where we want to break for each iteration.
void SetExpressionAsStatementPosition(Expression* expr);
void SetCallPosition(Expression* expr);
void SetCallPosition(Expression* expr,
TailCallMode tail_call_mode = TailCallMode::kDisallow);
void SetConstructCallPosition(Expression* expr) {
// Currently call and construct calls are treated the same wrt debugging.

View File

@ -2523,7 +2523,7 @@ void FullCodeGenerator::EmitCall(Call* expr, ConvertReceiverMode mode) {
}
PrepareForBailoutForId(expr->CallId(), NO_REGISTERS);
SetCallPosition(expr);
SetCallPosition(expr, expr->tail_call_mode());
if (expr->tail_call_mode() == TailCallMode::kAllow) {
if (FLAG_trace) {
__ CallRuntime(Runtime::kTraceTailCall);

View File

@ -2635,7 +2635,7 @@ void FullCodeGenerator::EmitCall(Call* expr, ConvertReceiverMode mode) {
PrepareForBailoutForId(expr->CallId(), NO_REGISTERS);
// Record source position of the IC call.
SetCallPosition(expr);
SetCallPosition(expr, expr->tail_call_mode());
if (expr->tail_call_mode() == TailCallMode::kAllow) {
if (FLAG_trace) {
__ CallRuntime(Runtime::kTraceTailCall);

View File

@ -2636,7 +2636,7 @@ void FullCodeGenerator::EmitCall(Call* expr, ConvertReceiverMode mode) {
PrepareForBailoutForId(expr->CallId(), NO_REGISTERS);
// Record source position of the IC call.
SetCallPosition(expr);
SetCallPosition(expr, expr->tail_call_mode());
if (expr->tail_call_mode() == TailCallMode::kAllow) {
if (FLAG_trace) {
__ CallRuntime(Runtime::kTraceTailCall);

View File

@ -2639,7 +2639,7 @@ void FullCodeGenerator::EmitCall(Call* expr, ConvertReceiverMode mode) {
}
PrepareForBailoutForId(expr->CallId(), NO_REGISTERS);
SetCallPosition(expr);
SetCallPosition(expr, expr->tail_call_mode());
if (expr->tail_call_mode() == TailCallMode::kAllow) {
if (FLAG_trace) {
__ CallRuntime(Runtime::kTraceTailCall);

View File

@ -2574,7 +2574,7 @@ void FullCodeGenerator::EmitCall(Call* expr, ConvertReceiverMode mode) {
}
PrepareForBailoutForId(expr->CallId(), NO_REGISTERS);
SetCallPosition(expr);
SetCallPosition(expr, expr->tail_call_mode());
if (expr->tail_call_mode() == TailCallMode::kAllow) {
if (FLAG_trace) {
__ CallRuntime(Runtime::kTraceTailCall);

View File

@ -2513,7 +2513,7 @@ void FullCodeGenerator::EmitCall(Call* expr, ConvertReceiverMode mode) {
}
PrepareForBailoutForId(expr->CallId(), NO_REGISTERS);
SetCallPosition(expr);
SetCallPosition(expr, expr->tail_call_mode());
if (expr->tail_call_mode() == TailCallMode::kAllow) {
if (FLAG_trace) {
__ CallRuntime(Runtime::kTraceTailCall);

View File

@ -2515,7 +2515,7 @@ void FullCodeGenerator::EmitCall(Call* expr, ConvertReceiverMode mode) {
}
PrepareForBailoutForId(expr->CallId(), NO_REGISTERS);
SetCallPosition(expr);
SetCallPosition(expr, expr->tail_call_mode());
if (expr->tail_call_mode() == TailCallMode::kAllow) {
if (FLAG_trace) {
__ CallRuntime(Runtime::kTraceTailCall);

View File

@ -2966,7 +2966,7 @@ void Assembler::RecordRelocInfo(RelocInfo::Mode rmode, intptr_t data) {
// We do not try to reuse pool constants.
RelocInfo rinfo(isolate(), pc_, rmode, data, NULL);
if (rmode >= RelocInfo::COMMENT &&
rmode <= RelocInfo::DEBUG_BREAK_SLOT_AT_CALL) {
rmode <= RelocInfo::DEBUG_BREAK_SLOT_AT_TAIL_CALL) {
// Adjust code for new modes.
DCHECK(RelocInfo::IsDebugBreakSlot(rmode)
|| RelocInfo::IsComment(rmode)

View File

@ -3219,7 +3219,7 @@ void Assembler::RecordRelocInfo(RelocInfo::Mode rmode, intptr_t data) {
// We do not try to reuse pool constants.
RelocInfo rinfo(isolate(), pc_, rmode, data, NULL);
if (rmode >= RelocInfo::COMMENT &&
rmode <= RelocInfo::DEBUG_BREAK_SLOT_AT_CALL) {
rmode <= RelocInfo::DEBUG_BREAK_SLOT_AT_TAIL_CALL) {
// Adjust code for new modes.
DCHECK(RelocInfo::IsDebugBreakSlot(rmode)
|| RelocInfo::IsComment(rmode)

View File

@ -8127,3 +8127,61 @@ TEST(DisableTailCallElimination) {
ExpectInt32("h();", 2);
ExpectInt32("h(); %OptimizeFunctionOnNextCall(g); h();", 2);
}
TEST(DebugStepNextTailCallEliminiation) {
i::FLAG_allow_natives_syntax = true;
i::FLAG_harmony_tailcalls = true;
// TODO(ishell, 4698): Investigate why TurboFan in --always-opt mode makes
// stack[2].getFunctionName() return null.
i::FLAG_turbo_inlining = false;
DebugLocalContext env;
env.ExposeDebug();
v8::Isolate* isolate = env->GetIsolate();
v8::HandleScope scope(isolate);
CHECK(v8::Debug::IsTailCallEliminationEnabled(isolate));
const char* source =
"'use strict'; \n"
"var Debug = debug.Debug; \n"
"var exception = null; \n"
"var breaks = 0; \n"
"var log = []; \n"
"function f(x) { \n"
" if (x == 2) { \n"
" debugger; // Break a \n"
" } \n"
" if (x-- > 0) { // Break b \n"
" return f(x); // Break c \n"
" } \n"
"} // Break e \n"
"function listener(event, exec_state, event_data, data) {\n"
" if (event != Debug.DebugEvent.Break) return; \n"
" try { \n"
" var line = exec_state.frame(0).sourceLineText(); \n"
" var col = exec_state.frame(0).sourceColumn(); \n"
" var match = line.match(/\\/\\/ Break (\\w)/); \n"
" log.push(match[1] + col); \n"
" exec_state.prepareStep(Debug.StepAction.StepNext); \n"
" } catch (e) { \n"
" exception = e; \n"
" }; \n"
"}; \n"
"Debug.setListener(listener); \n"
"f(4); \n"
"Debug.setListener(null); // Break d \n";
CompileRun(source);
ExpectNull("exception");
ExpectString("JSON.stringify(log)", "[\"a4\",\"b2\",\"c4\",\"c11\",\"d0\"]");
v8::Debug::SetTailCallEliminationEnabled(isolate, false);
CompileRun(
"log = []; \n"
"Debug.setListener(listener); \n"
"f(5); \n"
"Debug.setListener(null); // Break f \n");
ExpectNull("exception");
ExpectString("JSON.stringify(log)",
"[\"a4\",\"b2\",\"c4\",\"e0\",\"e0\",\"e0\",\"e0\",\"f0\"]");
}