Reviewers: Yang,
Message:
PTAL :) "LocalIsSynthetic" might not be the right name.
Description:
Avoid exposing compiler-allocated temporaries to the debugger
R=
BUG=
Please review this at https://codereview.chromium.org/245963006/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+52, -41 lines):
M src/objects.h
M src/runtime.cc
M src/scopeinfo.cc
M test/mjsunit/bugs/harmony/debug-blockscopes.js
M test/mjsunit/debug-scopes.js
M test/mjsunit/harmony/debug-blockscopes.js
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
a0b4982588bc9878745fae97083365995c5f2edf..5b1b77c2fa41b142793e396b76c97cf799541291
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -4663,6 +4663,10 @@ class ScopeInfo : public FixedArray {
// Return the initialization flag of the given context local.
InitializationFlag ContextLocalInitFlag(int var);
+ // Return true if this local was introduced by the compiler, and should
not be
+ // exposed to the user in a debugger.
+ bool LocalIsSynthetic(int var);
+
// Lookup support for serialized scope info. Returns the
// the stack slot index for a given slot name if the slot is
// present; otherwise returns a value < 0. The name must be an
internalized
Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index
2d4ab93015e4f8a749fc8f4b61fbfe3976933869..57d204fda40f89f3d191f0ff643bbd2328ad002a
100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -11199,33 +11199,44 @@ RUNTIME_FUNCTION(MaybeObject*,
Runtime_GetFrameDetails) {
ASSERT(*scope_info != ScopeInfo::Empty(isolate));
// Get the locals names and values into a temporary array.
- //
- // TODO(1240907): Hide compiler-introduced stack variables
- // (e.g. .result)? For users of the debugger, they will probably be
- // confusing.
+ int local_count = scope_info->LocalCount();
+ for (int slot = 0; slot < scope_info->LocalCount(); ++slot) {
+ // Hide compiler-introduced temporary variables, whether on the stack
or on
+ // the context.
+ if (scope_info->LocalIsSynthetic(slot))
+ local_count--;
+ }
+
Handle<FixedArray> locals =
- isolate->factory()->NewFixedArray(scope_info->LocalCount() * 2);
+ isolate->factory()->NewFixedArray(local_count * 2);
// Fill in the values of the locals.
+ int local = 0;
int i = 0;
for (; i < scope_info->StackLocalCount(); ++i) {
// Use the value from the stack.
- locals->set(i * 2, scope_info->LocalName(i));
- locals->set(i * 2 + 1, frame_inspector.GetExpression(i));
+ if (scope_info->LocalIsSynthetic(i))
+ continue;
+ locals->set(local * 2, scope_info->LocalName(i));
+ locals->set(local * 2 + 1, frame_inspector.GetExpression(i));
+ local++;
}
- if (i < scope_info->LocalCount()) {
+ if (local < local_count) {
// Get the context containing declarations.
Handle<Context> context(
Context::cast(it.frame()->context())->declaration_context());
for (; i < scope_info->LocalCount(); ++i) {
+ if (scope_info->LocalIsSynthetic(i))
+ continue;
Handle<String> name(scope_info->LocalName(i));
VariableMode mode;
InitializationFlag init_flag;
- locals->set(i * 2, *name);
+ locals->set(local * 2, *name);
int context_slot_index =
scope_info->ContextSlotIndex(*name, &mode, &init_flag);
Object* value = context->get(context_slot_index);
- locals->set(i * 2 + 1, value);
+ locals->set(local * 2 + 1, value);
+ local++;
}
}
@@ -11286,7 +11297,7 @@ RUNTIME_FUNCTION(MaybeObject*,
Runtime_GetFrameDetails) {
// Calculate the size of the result.
int details_size = kFrameDetailsFirstDynamicIndex +
- 2 * (argument_count + scope_info->LocalCount()) +
+ 2 * (argument_count + local_count) +
(at_return ? 1 : 0);
Handle<FixedArray> details =
isolate->factory()->NewFixedArray(details_size);
@@ -11301,7 +11312,7 @@ RUNTIME_FUNCTION(MaybeObject*,
Runtime_GetFrameDetails) {
// Add the locals count
details->set(kFrameDetailsLocalCountIndex,
- Smi::FromInt(scope_info->LocalCount()));
+ Smi::FromInt(local_count));
// Add the source position.
if (position != RelocInfo::kNoPosition) {
@@ -11352,7 +11363,7 @@ RUNTIME_FUNCTION(MaybeObject*,
Runtime_GetFrameDetails) {
}
// Add locals name and value from the temporary copy from the function
frame.
- for (int i = 0; i < scope_info->LocalCount() * 2; i++) {
+ for (int i = 0; i < local_count * 2; i++) {
details->set(details_index++, locals->get(i));
}
@@ -11433,6 +11444,7 @@ static MaybeHandle<JSObject>
MaterializeStackLocalsWithFrameInspector(
// Second fill all stack locals.
for (int i = 0; i < scope_info->StackLocalCount(); ++i) {
+ if (scope_info->LocalIsSynthetic(i)) continue;
Handle<String> name(scope_info->StackLocalName(i));
Handle<Object> value(frame_inspector->GetExpression(i), isolate);
if (value->IsTheHole()) continue;
@@ -11477,6 +11489,7 @@ static void
UpdateStackLocalsFromMaterializedObject(Isolate* isolate,
// Stack locals.
for (int i = 0; i < scope_info->StackLocalCount(); ++i) {
+ if (scope_info->LocalIsSynthetic(i)) continue;
if (frame->GetExpression(i)->IsTheHole()) continue;
HandleScope scope(isolate);
Handle<Object> value = Object::GetPropertyOrElement(
Index: src/scopeinfo.cc
diff --git a/src/scopeinfo.cc b/src/scopeinfo.cc
index
157cdfa4b6c54fd17cc503ce0c6dd80e467e8b59..564565ff32a3cdaf09d39ffabe47b9a157741fd7
100644
--- a/src/scopeinfo.cc
+++ b/src/scopeinfo.cc
@@ -278,6 +278,17 @@ InitializationFlag ScopeInfo::ContextLocalInitFlag(int
var) {
}
+bool ScopeInfo::LocalIsSynthetic(int var) {
+ ASSERT(0 <= var && var < LocalCount());
+ // There's currently no flag stored on the ScopeInfo to indicate that a
+ // variable is a compiler-introduced temporary. However, to avoid
conflict
+ // with user declarations, the current temporaries
like .generator_object and
+ // .result start with a dot, so we can use that as a flag. It's a hack!
+ Handle<String> name(LocalName(var));
+ return name->length() > 0 && name->Get(0) == '.';
+}
+
+
int ScopeInfo::StackSlotIndex(String* name) {
ASSERT(name->IsInternalizedString());
if (length() > 0) {
@@ -368,16 +379,17 @@ bool
ScopeInfo::CopyContextLocalsToScopeObject(Handle<ScopeInfo> scope_info,
int local_count = scope_info->ContextLocalCount();
if (local_count == 0) return true;
// Fill all context locals to the context extension.
+ int first_context_var = scope_info->StackLocalCount();
int start = scope_info->ContextLocalNameEntriesIndex();
- int end = start + local_count;
- for (int i = start; i < end; ++i) {
- int context_index = Context::MIN_CONTEXT_SLOTS + i - start;
+ for (int i = 0; i < local_count; ++i) {
+ if (scope_info->LocalIsSynthetic(first_context_var + i)) continue;
+ int context_index = Context::MIN_CONTEXT_SLOTS + i;
RETURN_ON_EXCEPTION_VALUE(
isolate,
Runtime::SetObjectProperty(
isolate,
scope_object,
- Handle<String>(String::cast(scope_info->get(i))),
+ Handle<String>(String::cast(scope_info->get(i + start))),
Handle<Object>(context->get(context_index), isolate),
::NONE,
SLOPPY),
Index: test/mjsunit/bugs/harmony/debug-blockscopes.js
diff --git a/test/mjsunit/bugs/harmony/debug-blockscopes.js
b/test/mjsunit/bugs/harmony/debug-blockscopes.js
index
fda32eb3d30acfc98a8839714450afa2834cf92c..9ef8efbc0ce513abea2cc292078a1c5fcc1a08dd
100644
--- a/test/mjsunit/bugs/harmony/debug-blockscopes.js
+++ b/test/mjsunit/bugs/harmony/debug-blockscopes.js
@@ -144,18 +144,10 @@ function CheckScopeContent(content, number,
exec_state) {
if (!scope.scopeObject().property('arguments').isUndefined()) {
scope_size--;
}
- // Also ignore synthetic variable from catch block.
- if (!scope.scopeObject().property('.catch-var').isUndefined()) {
- scope_size--;
- }
// Skip property with empty name.
if (!scope.scopeObject().property('').isUndefined()) {
scope_size--;
}
- // Also ignore synthetic variable from block scopes.
- if (!scope.scopeObject().property('.block').isUndefined()) {
- scope_size--;
- }
if (count != scope_size) {
print('Names found in scope:');
Index: test/mjsunit/debug-scopes.js
diff --git a/test/mjsunit/debug-scopes.js b/test/mjsunit/debug-scopes.js
index
f5b5ec913ed6634dacface1cc90d176646d92513..ce37d2402305f5200023794475f1e74a770eb450
100644
--- a/test/mjsunit/debug-scopes.js
+++ b/test/mjsunit/debug-scopes.js
@@ -166,10 +166,6 @@ function CheckScopeContent(content, number,
exec_state) {
if (!scope.scopeObject().property('arguments').isUndefined()) {
scope_size--;
}
- // Also ignore synthetic variable from catch block.
- if (!scope.scopeObject().property('.catch-var').isUndefined()) {
- scope_size--;
- }
// Skip property with empty name.
if (!scope.scopeObject().property('').isUndefined()) {
scope_size--;
Index: test/mjsunit/harmony/debug-blockscopes.js
diff --git a/test/mjsunit/harmony/debug-blockscopes.js
b/test/mjsunit/harmony/debug-blockscopes.js
index
ca2ab9e5a68d90d362e6eb37b02a1cfc7493bb3c..f56a306b6faace323ac8249132adbf1a6e386d4d
100644
--- a/test/mjsunit/harmony/debug-blockscopes.js
+++ b/test/mjsunit/harmony/debug-blockscopes.js
@@ -147,18 +147,10 @@ function CheckScopeContent(content, number,
exec_state) {
if (!scope.scopeObject().property('arguments').isUndefined()) {
scope_size--;
}
- // Also ignore synthetic variable from catch block.
- if (!scope.scopeObject().property('.catch-var').isUndefined()) {
- scope_size--;
- }
// Skip property with empty name.
if (!scope.scopeObject().property('').isUndefined()) {
scope_size--;
}
- // Also ignore synthetic variable from block scopes.
- if (!scope.scopeObject().property('.block').isUndefined()) {
- scope_size--;
- }
if (count != scope_size) {
print('Names found in scope:');
@@ -375,8 +367,9 @@ listener_delegate = function(exec_state) {
debug.ScopeType.Local,
debug.ScopeType.Global], exec_state);
CheckScopeContent({x:'y'}, 0, exec_state);
- // The function scope contains a temporary iteration variable.
- CheckScopeContent({'.for.x':'y'}, 1, exec_state);
+ // The function scope contains a temporary iteration variable, but it is
+ // hidden to the debugger.
+ CheckScopeContent({}, 1, exec_state);
};
for_loop_1();
EndTest();
@@ -400,8 +393,9 @@ listener_delegate = function(exec_state) {
debug.ScopeType.Global], exec_state);
CheckScopeContent({x:3}, 0, exec_state);
CheckScopeContent({x:'y'}, 1, exec_state);
- // The function scope contains a temporary iteration variable.
- CheckScopeContent({'.for.x':'y'}, 2, exec_state);
+ // The function scope contains a temporary iteration variable, hidden to
the
+ // debugger.
+ CheckScopeContent({}, 2, exec_state);
};
for_loop_2();
EndTest();
--
--
v8-dev mailing list
[email protected]
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 [email protected].
For more options, visit https://groups.google.com/d/optout.