Skip to content

Commit 01902e4

Browse files
hashseedCommit bot
hashseed
authored and
Commit bot
committed
Debugger: use FrameInspector in ScopeIterator to find context.
In optimized code, it's not guaranteed that the current context is stored in its frame slot. R=bmeurer@chromium.org BUG=v8:4309 LOG=N Committed: https://crrev.com/3a0ee39cbde6a9778cfc4e2a6a0a8ff68933ff38 Cr-Commit-Position: refs/heads/master@{#29697} Review URL: https://codereview.chromium.org/1239033002 Cr-Commit-Position: refs/heads/master@{#29744}
1 parent cc66a1c commit 01902e4

6 files changed

Lines changed: 186 additions & 72 deletions

File tree

‎src/debug.cc‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2735,19 +2735,8 @@ void Debug::HandleDebugBreak() {
27352735
bool debug_command_only = isolate_->stack_guard()->CheckDebugCommand() &&
27362736
!isolate_->stack_guard()->CheckDebugBreak();
27372737

2738-
bool is_debugger_statement = !isolate_->stack_guard()->CheckDebugCommand() &&
2739-
!isolate_->stack_guard()->CheckDebugBreak();
2740-
27412738
isolate_->stack_guard()->ClearDebugBreak();
27422739

2743-
if (is_debugger_statement) {
2744-
// If we have been called via 'debugger' Javascript statement,
2745-
// we might not be prepared for breakpoints.
2746-
// TODO(dslomov,yangguo): CheckDebugBreak may race with RequestDebugBreak.
2747-
// Revisit this to clean-up.
2748-
HandleScope handle_scope(isolate_);
2749-
PrepareForBreakPoints();
2750-
}
27512740
ProcessDebugMessages(debug_command_only);
27522741
}
27532742

‎src/runtime/runtime-debug.cc‎

Lines changed: 63 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,7 @@ class FrameInspector {
529529
Object* GetContext() {
530530
return is_optimized_ ? deoptimized_frame_->GetContext() : frame_->context();
531531
}
532+
JavaScriptFrame* GetArgumentsFrame() { return frame_; }
532533

533534
// To inspect all the provided arguments the frame might need to be
534535
// replaced with the arguments frame.
@@ -959,9 +960,7 @@ static void UpdateStackLocalsFromMaterializedObject(
959960
Isolate* isolate, Handle<JSObject> target, Handle<ScopeInfo> scope_info,
960961
JavaScriptFrame* frame, int inlined_jsframe_index) {
961962
if (inlined_jsframe_index != 0 || frame->is_optimized()) {
962-
// Optimized frames are not supported.
963-
// TODO(yangguo): make sure all code deoptimized when debugger is active
964-
// and assert that this cannot happen.
963+
// Optimized frames are not supported. Simply give up.
965964
return;
966965
}
967966

@@ -994,15 +993,14 @@ static void UpdateStackLocalsFromMaterializedObject(
994993

995994
MUST_USE_RESULT static MaybeHandle<JSObject> MaterializeLocalContext(
996995
Isolate* isolate, Handle<JSObject> target, Handle<JSFunction> function,
997-
JavaScriptFrame* frame) {
996+
Handle<Context> frame_context) {
998997
HandleScope scope(isolate);
999998
Handle<SharedFunctionInfo> shared(function->shared());
1000999
Handle<ScopeInfo> scope_info(shared->scope_info());
10011000

10021001
if (!scope_info->HasContext()) return target;
10031002

10041003
// Third fill all context locals.
1005-
Handle<Context> frame_context(Context::cast(frame->context()));
10061004
Handle<Context> function_context(frame_context->declaration_context());
10071005
ScopeInfo::CopyContextLocalsToScopeObject(scope_info, function_context,
10081006
target);
@@ -1058,16 +1056,17 @@ MUST_USE_RESULT static MaybeHandle<JSObject> MaterializeScriptScope(
10581056

10591057

10601058
MUST_USE_RESULT static MaybeHandle<JSObject> MaterializeLocalScope(
1061-
Isolate* isolate, JavaScriptFrame* frame, int inlined_jsframe_index) {
1062-
FrameInspector frame_inspector(frame, inlined_jsframe_index, isolate);
1063-
Handle<JSFunction> function(JSFunction::cast(frame_inspector.GetFunction()));
1059+
Isolate* isolate, FrameInspector* frame_inspector) {
1060+
Handle<JSFunction> function(JSFunction::cast(frame_inspector->GetFunction()));
10641061

10651062
Handle<JSObject> local_scope =
10661063
isolate->factory()->NewJSObject(isolate->object_function());
10671064
MaterializeStackLocalsWithFrameInspector(isolate, local_scope, function,
1068-
&frame_inspector);
1065+
frame_inspector);
1066+
1067+
Handle<Context> frame_context(Context::cast(frame_inspector->GetContext()));
10691068

1070-
return MaterializeLocalContext(isolate, local_scope, function, frame);
1069+
return MaterializeLocalContext(isolate, local_scope, function, frame_context);
10711070
}
10721071

10731072

@@ -1096,13 +1095,10 @@ static bool SetContextLocalValue(Isolate* isolate, Handle<ScopeInfo> scope_info,
10961095

10971096

10981097
static bool SetLocalVariableValue(Isolate* isolate, JavaScriptFrame* frame,
1099-
int inlined_jsframe_index,
11001098
Handle<String> variable_name,
11011099
Handle<Object> new_value) {
1102-
if (inlined_jsframe_index != 0 || frame->is_optimized()) {
1103-
// Optimized frames are not supported.
1104-
return false;
1105-
}
1100+
// Optimized frames are not supported.
1101+
if (frame->is_optimized()) return false;
11061102

11071103
Handle<JSFunction> function(frame->function());
11081104
Handle<SharedFunctionInfo> shared(function->shared());
@@ -1311,15 +1307,13 @@ static bool SetCatchVariableValue(Isolate* isolate, Handle<Context> context,
13111307
static Handle<JSObject> MaterializeBlockScope(Isolate* isolate,
13121308
Handle<ScopeInfo> scope_info,
13131309
Handle<Context> context,
1314-
JavaScriptFrame* frame,
1315-
int inlined_jsframe_index) {
1310+
FrameInspector* frame_inspector) {
13161311
Handle<JSObject> block_scope =
13171312
isolate->factory()->NewJSObject(isolate->object_function());
13181313

1319-
if (frame != nullptr) {
1320-
FrameInspector frame_inspector(frame, inlined_jsframe_index, isolate);
1314+
if (frame_inspector != nullptr) {
13211315
MaterializeStackLocalsWithFrameInspector(isolate, block_scope, scope_info,
1322-
&frame_inspector);
1316+
frame_inspector);
13231317
}
13241318

13251319
if (!context.is_null()) {
@@ -1370,21 +1364,26 @@ class ScopeIterator {
13701364
ScopeTypeModule
13711365
};
13721366

1373-
ScopeIterator(Isolate* isolate, JavaScriptFrame* frame,
1374-
int inlined_jsframe_index, bool ignore_nested_scopes = false)
1367+
ScopeIterator(Isolate* isolate, FrameInspector* frame_inspector,
1368+
bool ignore_nested_scopes = false)
13751369
: isolate_(isolate),
1376-
frame_(frame),
1377-
inlined_jsframe_index_(inlined_jsframe_index),
1378-
function_(frame->function()),
1379-
context_(Context::cast(frame->context())),
1370+
frame_inspector_(frame_inspector),
13801371
nested_scope_chain_(4),
13811372
seen_script_scope_(false),
13821373
failed_(false) {
1374+
if (!frame_inspector->GetContext()->IsContext() ||
1375+
!frame_inspector->GetFunction()->IsJSFunction()) {
1376+
// Optimized frame, context or function cannot be materialized. Give up.
1377+
return;
1378+
}
1379+
1380+
context_ = Handle<Context>(Context::cast(frame_inspector->GetContext()));
1381+
13831382
// Catch the case when the debugger stops in an internal function.
1384-
Handle<SharedFunctionInfo> shared_info(function_->shared());
1383+
Handle<SharedFunctionInfo> shared_info(function()->shared());
13851384
Handle<ScopeInfo> scope_info(shared_info->scope_info());
13861385
if (shared_info->script() == isolate->heap()->undefined_value()) {
1387-
while (context_->closure() == *function_) {
1386+
while (context_->closure() == function()) {
13881387
context_ = Handle<Context>(context_->previous(), isolate_);
13891388
}
13901389
return;
@@ -1408,7 +1407,7 @@ class ScopeIterator {
14081407

14091408
// PC points to the instruction after the current one, possibly a break
14101409
// location as well. So the "- 1" to exclude it from the search.
1411-
Address call_pc = frame->pc() - 1;
1410+
Address call_pc = frame()->pc() - 1;
14121411

14131412
// Find the break point where execution has stopped.
14141413
BreakLocation location =
@@ -1421,7 +1420,7 @@ class ScopeIterator {
14211420
if (scope_info->HasContext()) {
14221421
context_ = Handle<Context>(context_->declaration_context(), isolate_);
14231422
} else {
1424-
while (context_->closure() == *function_) {
1423+
while (context_->closure() == function()) {
14251424
context_ = Handle<Context>(context_->previous(), isolate_);
14261425
}
14271426
}
@@ -1446,15 +1445,15 @@ class ScopeIterator {
14461445
} else {
14471446
DCHECK(scope_info->scope_type() == EVAL_SCOPE);
14481447
info.set_eval();
1449-
info.set_context(Handle<Context>(function_->context()));
1448+
info.set_context(Handle<Context>(function()->context()));
14501449
}
14511450
if (Parser::ParseStatic(&info) && Scope::Analyze(&info)) {
14521451
scope = info.function()->scope();
14531452
}
14541453
RetrieveScopeChain(scope, shared_info);
14551454
} else {
14561455
// Function code
1457-
ParseInfo info(&zone, function_);
1456+
ParseInfo info(&zone, Handle<JSFunction>(function()));
14581457
if (Parser::ParseStatic(&info) && Scope::Analyze(&info)) {
14591458
scope = info.function()->scope();
14601459
}
@@ -1465,15 +1464,11 @@ class ScopeIterator {
14651464

14661465
ScopeIterator(Isolate* isolate, Handle<JSFunction> function)
14671466
: isolate_(isolate),
1468-
frame_(NULL),
1469-
inlined_jsframe_index_(0),
1470-
function_(function),
1467+
frame_inspector_(NULL),
14711468
context_(function->context()),
14721469
seen_script_scope_(false),
14731470
failed_(false) {
1474-
if (function->IsBuiltin()) {
1475-
context_ = Handle<Context>();
1476-
}
1471+
if (function->IsBuiltin()) context_ = Handle<Context>();
14771472
}
14781473

14791474
// More scopes?
@@ -1584,7 +1579,7 @@ class ScopeIterator {
15841579
case ScopeIterator::ScopeTypeLocal:
15851580
// Materialize the content of the local scope into a JSObject.
15861581
DCHECK(nested_scope_chain_.length() == 1);
1587-
return MaterializeLocalScope(isolate_, frame_, inlined_jsframe_index_);
1582+
return MaterializeLocalScope(isolate_, frame_inspector_);
15881583
case ScopeIterator::ScopeTypeWith:
15891584
// Return the with object.
15901585
return Handle<JSObject>(JSObject::cast(CurrentContext()->extension()));
@@ -1600,11 +1595,11 @@ class ScopeIterator {
16001595
Handle<Context> context = scope_info->HasContext()
16011596
? CurrentContext()
16021597
: Handle<Context>::null();
1603-
return MaterializeBlockScope(isolate_, scope_info, context, frame_,
1604-
inlined_jsframe_index_);
1598+
return MaterializeBlockScope(isolate_, scope_info, context,
1599+
frame_inspector_);
16051600
} else {
16061601
return MaterializeBlockScope(isolate_, Handle<ScopeInfo>::null(),
1607-
CurrentContext(), nullptr, 0);
1602+
CurrentContext(), nullptr);
16081603
}
16091604
}
16101605
case ScopeIterator::ScopeTypeModule:
@@ -1631,8 +1626,8 @@ class ScopeIterator {
16311626
case ScopeIterator::ScopeTypeGlobal:
16321627
break;
16331628
case ScopeIterator::ScopeTypeLocal:
1634-
return SetLocalVariableValue(isolate_, frame_, inlined_jsframe_index_,
1635-
variable_name, new_value);
1629+
return SetLocalVariableValue(isolate_, frame(), variable_name,
1630+
new_value);
16361631
case ScopeIterator::ScopeTypeWith:
16371632
break;
16381633
case ScopeIterator::ScopeTypeCatch:
@@ -1647,7 +1642,7 @@ class ScopeIterator {
16471642
case ScopeIterator::ScopeTypeBlock:
16481643
return SetBlockVariableValue(
16491644
isolate_, HasContext() ? CurrentContext() : Handle<Context>::null(),
1650-
CurrentScopeInfo(), frame_, variable_name, new_value);
1645+
CurrentScopeInfo(), frame(), variable_name, new_value);
16511646
case ScopeIterator::ScopeTypeModule:
16521647
// TODO(2399): should we implement it?
16531648
break;
@@ -1694,7 +1689,7 @@ class ScopeIterator {
16941689

16951690
case ScopeIterator::ScopeTypeLocal: {
16961691
os << "Local:\n";
1697-
function_->shared()->scope_info()->Print();
1692+
function()->shared()->scope_info()->Print();
16981693
if (!CurrentContext().is_null()) {
16991694
CurrentContext()->Print(os);
17001695
if (CurrentContext()->has_extension()) {
@@ -1747,18 +1742,24 @@ class ScopeIterator {
17471742

17481743
private:
17491744
Isolate* isolate_;
1750-
JavaScriptFrame* frame_;
1751-
int inlined_jsframe_index_;
1752-
Handle<JSFunction> function_;
1745+
FrameInspector* const frame_inspector_;
17531746
Handle<Context> context_;
17541747
List<Handle<ScopeInfo> > nested_scope_chain_;
17551748
bool seen_script_scope_;
17561749
bool failed_;
17571750

1751+
inline JavaScriptFrame* frame() {
1752+
return frame_inspector_->GetArgumentsFrame();
1753+
}
1754+
1755+
inline JSFunction* function() {
1756+
return JSFunction::cast(frame_inspector_->GetFunction());
1757+
}
1758+
17581759
void RetrieveScopeChain(Scope* scope,
17591760
Handle<SharedFunctionInfo> shared_info) {
17601761
if (scope != NULL) {
1761-
int source_position = shared_info->code()->SourcePosition(frame_->pc());
1762+
int source_position = frame_inspector_->GetSourcePosition();
17621763
scope->GetNestedScopeChain(isolate_, &nested_scope_chain_,
17631764
source_position);
17641765
} else {
@@ -1789,10 +1790,11 @@ RUNTIME_FUNCTION(Runtime_GetScopeCount) {
17891790
StackFrame::Id id = UnwrapFrameId(wrapped_id);
17901791
JavaScriptFrameIterator it(isolate, id);
17911792
JavaScriptFrame* frame = it.frame();
1793+
FrameInspector frame_inspector(frame, 0, isolate);
17921794

17931795
// Count the visible scopes.
17941796
int n = 0;
1795-
for (ScopeIterator it(isolate, frame, 0); !it.Done(); it.Next()) {
1797+
for (ScopeIterator it(isolate, &frame_inspector); !it.Done(); it.Next()) {
17961798
n++;
17971799
}
17981800

@@ -1917,10 +1919,11 @@ RUNTIME_FUNCTION(Runtime_GetScopeDetails) {
19171919
StackFrame::Id id = UnwrapFrameId(wrapped_id);
19181920
JavaScriptFrameIterator frame_it(isolate, id);
19191921
JavaScriptFrame* frame = frame_it.frame();
1922+
FrameInspector frame_inspector(frame, inlined_jsframe_index, isolate);
19201923

19211924
// Find the requested scope.
19221925
int n = 0;
1923-
ScopeIterator it(isolate, frame, inlined_jsframe_index);
1926+
ScopeIterator it(isolate, &frame_inspector);
19241927
for (; !it.Done() && n < index; it.Next()) {
19251928
n++;
19261929
}
@@ -1962,9 +1965,10 @@ RUNTIME_FUNCTION(Runtime_GetAllScopesDetails) {
19621965
StackFrame::Id id = UnwrapFrameId(wrapped_id);
19631966
JavaScriptFrameIterator frame_it(isolate, id);
19641967
JavaScriptFrame* frame = frame_it.frame();
1968+
FrameInspector frame_inspector(frame, inlined_jsframe_index, isolate);
19651969

19661970
List<Handle<JSObject> > result(4);
1967-
ScopeIterator it(isolate, frame, inlined_jsframe_index, ignore_nested_scopes);
1971+
ScopeIterator it(isolate, &frame_inspector, ignore_nested_scopes);
19681972
for (; !it.Done(); it.Next()) {
19691973
Handle<JSObject> details;
19701974
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, details,
@@ -2065,8 +2069,9 @@ RUNTIME_FUNCTION(Runtime_SetScopeVariableValue) {
20652069
StackFrame::Id id = UnwrapFrameId(wrapped_id);
20662070
JavaScriptFrameIterator frame_it(isolate, id);
20672071
JavaScriptFrame* frame = frame_it.frame();
2072+
FrameInspector frame_inspector(frame, inlined_jsframe_index, isolate);
20682073

2069-
ScopeIterator it(isolate, frame, inlined_jsframe_index);
2074+
ScopeIterator it(isolate, &frame_inspector);
20702075
res = SetScopeVariableValue(&it, index, variable_name, new_value);
20712076
} else {
20722077
CONVERT_ARG_HANDLE_CHECKED(JSFunction, fun, 0);
@@ -2086,7 +2091,9 @@ RUNTIME_FUNCTION(Runtime_DebugPrintScopes) {
20862091
// Print the scopes for the top frame.
20872092
StackFrameLocator locator(isolate);
20882093
JavaScriptFrame* frame = locator.FindJavaScriptFrame(0);
2089-
for (ScopeIterator it(isolate, frame, 0); !it.Done(); it.Next()) {
2094+
FrameInspector frame_inspector(frame, 0, isolate);
2095+
2096+
for (ScopeIterator it(isolate, &frame_inspector); !it.Done(); it.Next()) {
20902097
it.DebugPrint();
20912098
}
20922099
#endif
@@ -2479,7 +2486,7 @@ class EvaluationContextBuilder {
24792486
Handle<Context> inner_context;
24802487

24812488
bool stop = false;
2482-
for (ScopeIterator it(isolate, frame, inlined_jsframe_index);
2489+
for (ScopeIterator it(isolate, &frame_inspector);
24832490
!it.Failed() && !it.Done() && !stop; it.Next()) {
24842491
ScopeIterator::ScopeType scope_type = it.Type();
24852492

0 commit comments

Comments
 (0)