Avoid exposing compiler-allocated temporaries to the debugger

R=yangguo@chromium.org
BUG=

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20899 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/objects.h b/src/objects.h
index a190f8d..71eeaa2 100644 (file)
--- 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

diff --git a/src/runtime.cc b/src/runtime.cc
index 1fa6da8..c508b4a 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -11200,33 +11200,44 @@ RUNTIME_FUNCTION(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++;
     }
   }
 
@@ -11287,7 +11298,7 @@ RUNTIME_FUNCTION(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);
 
@@ -11302,7 +11313,7 @@ RUNTIME_FUNCTION(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) {
@@ -11353,7 +11364,7 @@ RUNTIME_FUNCTION(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));
   }
 
@@ -11434,6 +11445,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;
@@ -11478,6 +11490,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(

diff --git a/src/scopeinfo.cc b/src/scopeinfo.cc
index 157cdfa..564565f 100644 (file)
--- 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),

diff --git a/test/mjsunit/bugs/harmony/debug-blockscopes.js b/test/mjsunit/bugs/harmony/debug-blockscopes.js
index fda32eb..9ef8efb 100644 (file)
--- 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:');

diff --git a/test/mjsunit/debug-scopes.js b/test/mjsunit/debug-scopes.js
index f5b5ec9..ce37d24 100644 (file)
--- 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--;

diff --git a/test/mjsunit/harmony/debug-blockscopes.js b/test/mjsunit/harmony/debug-blockscopes.js
index ca2ab9e..f56a306 100644 (file)
--- 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();