Only copy with, block and catch scopes in DebugEvaluate.

R=ulan@chromium.org
BUG=171715

Review URL: https://chromiumcodereview.appspot.com/13093003

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

diff --git a/src/runtime.cc b/src/runtime.cc
index d9dd12e..1ea9274 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -11481,6 +11481,13 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_ClearStepping) {
 }
 
 
+static bool IsBlockOrCatchOrWithScope(ScopeIterator::ScopeType type) {
+  return type == ScopeIterator::ScopeTypeBlock ||
+         type == ScopeIterator::ScopeTypeCatch ||
+         type == ScopeIterator::ScopeTypeWith;
+}
+
+
 // Creates a copy of the with context chain. The copy of the context chain is
 // is linked to the function context supplied.
 static Handle<Context> CopyNestedScopeContextChain(Isolate* isolate,
@@ -11495,8 +11502,7 @@ static Handle<Context> CopyNestedScopeContextChain(Isolate* isolate,
   ScopeIterator it(isolate, frame, inlined_jsframe_index);
   if (it.Failed()) return Handle<Context>::null();
 
-  for (; it.Type() != ScopeIterator::ScopeTypeGlobal &&
-         it.Type() != ScopeIterator::ScopeTypeLocal ; it.Next()) {
+  for ( ; IsBlockOrCatchOrWithScope(it.Type()); it.Next()) {
     ASSERT(!it.Done());
     scope_chain.Add(it.CurrentScopeInfo());
     context_chain.Add(it.CurrentContext());
@@ -11512,6 +11518,7 @@ static Handle<Context> CopyNestedScopeContextChain(Isolate* isolate,
     ASSERT(!(scope_info->HasContext() & current.is_null()));
 
     if (scope_info->Type() == CATCH_SCOPE) {
+      ASSERT(current->IsCatchContext());
       Handle<String> name(String::cast(current->extension()));
       Handle<Object> thrown_object(current->get(Context::THROWN_OBJECT_INDEX),
                                    isolate);
@@ -11522,6 +11529,7 @@ static Handle<Context> CopyNestedScopeContextChain(Isolate* isolate,
                                               thrown_object);
     } else if (scope_info->Type() == BLOCK_SCOPE) {
       // Materialize the contents of the block scope into a JSObject.
+      ASSERT(current->IsBlockContext());
       Handle<JSObject> block_scope_object =
           MaterializeBlockScope(isolate, current);
       CHECK(!block_scope_object.is_null());
@@ -11688,23 +11696,26 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluate) {
       isolate, frame, &frame_inspector);
   RETURN_IF_EMPTY_HANDLE(isolate, local_scope);
 
-  Handle<Context> frame_context(Context::cast(frame->context()));
-  Handle<Context> function_context;
-  Handle<ScopeInfo> scope_info(function->shared()->scope_info());
-  if (scope_info->HasContext()) {
-    function_context = Handle<Context>(frame_context->declaration_context());
-  }
-  Handle<Object> arguments = GetArgumentsObject(isolate,
-                                                frame,
-                                                &frame_inspector,
-                                                scope_info,
-                                                function_context);
-  SetProperty(isolate,
-              local_scope,
-              isolate->factory()->arguments_string(),
-              arguments,
-              ::NONE,
-              kNonStrictMode);
+  // Do not materialize the arguments object for eval or top-level code.
+  if (function->shared()->is_function()) {
+    Handle<Context> frame_context(Context::cast(frame->context()));
+    Handle<Context> function_context;
+    Handle<ScopeInfo> scope_info(function->shared()->scope_info());
+    if (scope_info->HasContext()) {
+      function_context = Handle<Context>(frame_context->declaration_context());
+    }
+    Handle<Object> arguments = GetArgumentsObject(isolate,
+                                                  frame,
+                                                  &frame_inspector,
+                                                  scope_info,
+                                                  function_context);
+    SetProperty(isolate,
+                local_scope,
+                isolate->factory()->arguments_string(),
+                arguments,
+                ::NONE,
+                kNonStrictMode);
+  }
 
   // Allocate a new context for the debug evaluation and set the extension
   // object build.

diff --git a/test/mjsunit/regress/regress-crbug-171715.js b/test/mjsunit/regress/regress-crbug-171715.js
new file mode 100644 (file)
index 0000000..040c381
--- /dev/null
+++ b/test/mjsunit/regress/regress-crbug-171715.js
@@ -0,0 +1,87 @@
+// Copyright 2013 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --expose-debug-as debug
+
+Debug = debug.Debug
+
+var error = null;
+var test = 0;
+
+function check_v(expected, exec_state, frame_id) {
+  assertEquals(expected, exec_state.frame(frame_id).evaluate('v').value());
+}
+
+function listener(event, exec_state, event_data, data) {
+  try {
+    if (event != Debug.DebugEvent.Break) return;
+    test++;
+    if (test == 1) {
+      check_v('inner0', exec_state, 0);
+      check_v('inner0', exec_state, 1);
+      check_v('outer',  exec_state, 2);
+      assertArrayEquals(["a", "b", "c"],
+                        exec_state.frame(0).evaluate('arguments').value());
+    } else if (test == 2) {
+      check_v('inner1', exec_state, 0);
+      check_v('inner1', exec_state, 1);
+      check_v('outer',  exec_state, 2);
+      assertArrayEquals(["a", "b", "c"],
+                        exec_state.frame(0).evaluate('arguments').value());
+    } else {
+      assertEquals(3, test);
+      check_v('inner2', exec_state, 0);
+      check_v('inner1', exec_state, 1);
+      check_v('inner1', exec_state, 2);
+      check_v('outer',  exec_state, 3);
+      assertArrayEquals(["x", "y", "z"],
+                        exec_state.frame(0).evaluate('arguments').value());
+      assertArrayEquals(["a", "b", "c"],
+                        exec_state.frame(1).evaluate('arguments').value());
+    }
+  } catch (e) {
+    error = e;
+  }
+};
+
+Debug.setListener(listener);
+
+var v = 'outer';
+(function() {  // Test 1 and 2
+  var v = 'inner0';
+  eval("debugger; var v = 'inner1'; debugger;");
+  assertEquals('inner1', v);  // Overwritten by local eval.
+})("a", "b", "c");
+assertNull(error);
+
+(function() {  // Test 3
+  var v = 'inner0';  // Local eval overwrites this value.
+  eval("var v = 'inner1'; " +
+       "(function() { var v = 'inner2'; debugger; })('x', 'y', 'z');");
+  assertEquals('inner1', v);  // Overwritten by local eval.
+})("a", "b", "c");
+assertNull(error);