Do not materialize context-allocated values for debug-evaluate.

BUG=259300
R=ulan@chromium.org, verwaest@chromium.org

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

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

src/mirror-debugger.js

@@ -1699,30 +1699,12 @@ FrameMirror.prototype.stepInPositions = function() {
 
 FrameMirror.prototype.evaluate = function(source, disable_break,
                                           opt_context_object) {
-  var result_array = %DebugEvaluate(this.break_id_,
-                                    this.details_.frameId(),
-                                    this.details_.inlinedFrameIndex(),
-                                    source,
-                                    Boolean(disable_break),
-                                    opt_context_object);
-  // Silently ignore local variables changes if the frame is optimized.
-  if (!this.isOptimizedFrame()) {
-    var local_scope_on_stack = result_array[1];
-    var local_scope_modifed = result_array[2];
-    for (var n in local_scope_modifed) {
-      var value_on_stack = local_scope_on_stack[n];
-      var value_modifed = local_scope_modifed[n];
-      if (value_on_stack !== value_modifed) {
-        %SetScopeVariableValue(this.break_id_,
-                               this.details_.frameId(),
-                               this.details_.inlinedFrameIndex(),
-                               0,
-                               n,
-                               value_modifed);
-      }
-    }
-  }
-  return MakeMirror(result_array[0]);
+  return MakeMirror(%DebugEvaluate(this.break_id_,
+                                   this.details_.frameId(),
+                                   this.details_.inlinedFrameIndex(),
+                                   source,
+                                   Boolean(disable_break),
+                                   opt_context_object));
 };
 
 

src/scopes.cc

@@ -970,6 +970,13 @@ Variable* Scope::LookupRecursive(Handle<String> name,
                                  BindingKind* binding_kind,
                                  AstNodeFactory<AstNullVisitor>* factory) {
   ASSERT(binding_kind != NULL);
+  if (already_resolved() && is_with_scope()) {
+    // Short-cut: if the scope is deserialized from a scope info, variable
+    // allocation is already fixed.  We can simply return with dynamic lookup.
+    *binding_kind = DYNAMIC_LOOKUP;
+    return NULL;
+  }
+
   // Try to find the variable in this scope.
   Variable* var = LocalLookup(name);
 
@@ -998,6 +1005,7 @@ Variable* Scope::LookupRecursive(Handle<String> name,
   }
 
   if (is_with_scope()) {
+    ASSERT(!already_resolved());
     // The current scope is a with scope, so the variable binding can not be
     // statically resolved. However, note that it was necessary to do a lookup
     // in the outer scope anyway, because if a binding exists in an outer scope,

test/mjsunit/debug-evaluate-closure.js

+// 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 --allow-natives-syntax
+
+Debug = debug.Debug;
+var listened = false;
+
+function listener(event, exec_state, event_data, data) {
+  if (event != Debug.DebugEvent.Break) return;
+  try {
+    assertEquals("goo", exec_state.frame(0).evaluate("goo").value());
+    exec_state.frame(0).evaluate("goo = 'goo foo'");
+    assertEquals("bar return", exec_state.frame(0).evaluate("bar()").value());
+    assertEquals("inner bar", exec_state.frame(0).evaluate("inner").value());
+    assertEquals("outer bar", exec_state.frame(0).evaluate("outer").value());
+    assertEquals("baz inner", exec_state.frame(0).evaluate("baz").value());
+    assertEquals("baz outer", exec_state.frame(1).evaluate("baz").value());
+    exec_state.frame(0).evaluate("w = 'w foo'");
+    exec_state.frame(0).evaluate("inner = 'inner foo'");
+    exec_state.frame(0).evaluate("outer = 'outer foo'");
+    exec_state.frame(0).evaluate("baz = 'baz inner foo'");
+    exec_state.frame(1).evaluate("baz = 'baz outer foo'");
+    listened = true;
+  } catch (e) {
+    print(e);
+    print(e.stack);
+  }
+}
+
+Debug.setListener(listener);
+
+var outer = "outer";
+var baz = "baz outer";
+
+function foo() {
+  var inner = "inner";
+  var baz = "baz inner";
+  var goo = "goo";
+  var withw = { w: "w" };
+  var withv = { v: "v" };
+
+  with (withv) {
+    var bar = function bar() {
+      assertEquals("goo foo", goo);
+      inner = "inner bar";
+      outer = "outer bar";
+      v = "v bar";
+      return "bar return";
+    };
+  }
+
+  with (withw) {
+    debugger;
+  }
+
+  assertEquals("inner foo", inner);
+  assertEquals("baz inner foo", baz);
+  assertEquals("w foo", withw.w);
+  assertEquals("v bar", withv.v);
+}
+
+foo();
+assertEquals("outer foo", outer);
+assertEquals("baz outer foo", baz);
+assertTrue(listened);
+Debug.setListener(null);

test/mjsunit/regress/regress-crbug-259300.js

+// 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 listened = false;
+var recursion_depth = 0;
+
+function listener(event, exec_state, event_data, data) {
+  if (event == Debug.DebugEvent.Break) {
+    recursion_depth++;
+    var disable_break = (recursion_depth > 2);
+    for (var i = 0; i < exec_state.frameCount(); i++) {
+      exec_state.frame(i).evaluate("debugger", disable_break);
+    }
+  }
+  listened = true;
+}
+
+Debug.setListener(listener);
+eval("debugger");
+Debug.setListener(null);
+assertTrue(listened);
+