From bb632dc49d22c7a26c93300f17d327044f0e1679 Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Tue, 26 Mar 2013 17:46:16 +0000 Subject: [PATCH] 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 --- src/runtime.cc | 49 ++++++++++------ test/mjsunit/regress/regress-crbug-171715.js | 87 ++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 19 deletions(-) create mode 100644 test/mjsunit/regress/regress-crbug-171715.js diff --git a/src/runtime.cc b/src/runtime.cc index d9dd12e..1ea9274 100644 --- 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 CopyNestedScopeContextChain(Isolate* isolate, @@ -11495,8 +11502,7 @@ static Handle CopyNestedScopeContextChain(Isolate* isolate, ScopeIterator it(isolate, frame, inlined_jsframe_index); if (it.Failed()) return Handle::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 CopyNestedScopeContextChain(Isolate* isolate, ASSERT(!(scope_info->HasContext() & current.is_null())); if (scope_info->Type() == CATCH_SCOPE) { + ASSERT(current->IsCatchContext()); Handle name(String::cast(current->extension())); Handle thrown_object(current->get(Context::THROWN_OBJECT_INDEX), isolate); @@ -11522,6 +11529,7 @@ static Handle 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 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 frame_context(Context::cast(frame->context())); - Handle function_context; - Handle scope_info(function->shared()->scope_info()); - if (scope_info->HasContext()) { - function_context = Handle(frame_context->declaration_context()); - } - Handle 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 frame_context(Context::cast(frame->context())); + Handle function_context; + Handle scope_info(function->shared()->scope_info()); + if (scope_info->HasContext()) { + function_context = Handle(frame_context->declaration_context()); + } + Handle 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 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); -- 2.7.4