From 354f4f08588a2306ecb01870e5a6c253fccb00e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Z=C3=BCnd?= Date: Tue, 20 Sep 2022 07:42:48 +0200 Subject: [PATCH] [debug] Refactor 'shadowing' tests for debug-evaluate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We currently use 1 test case per file for tests that check that debug-evaluate correctly blocks the lookup of stack-allocated variables. This CL adapts a similar approach to `debug-scopes.js`, making it easier to add new test cases in the future. R=kimanh@chromium.org Bug: chromium:1363561 Change-Id: I8ff8cfe7d59f0b9808dc02c5579e058f490553eb Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3904544 Commit-Queue: Simon Zünd Reviewed-by: Kim-Anh Tran Cr-Commit-Position: refs/heads/main@{#83315} --- .../debug-evaluate-shadowed-context-2.js | 40 ----- .../debug-evaluate-shadowed-context-3.js | 39 ----- .../debug-evaluate-shadowed-context-4.js | 37 ---- .../debug/debug-evaluate-shadowed-context.js | 159 ++++++++++++++---- 4 files changed, 130 insertions(+), 145 deletions(-) delete mode 100644 test/debugger/debug/debug-evaluate-shadowed-context-2.js delete mode 100644 test/debugger/debug/debug-evaluate-shadowed-context-3.js delete mode 100644 test/debugger/debug/debug-evaluate-shadowed-context-4.js diff --git a/test/debugger/debug/debug-evaluate-shadowed-context-2.js b/test/debugger/debug/debug-evaluate-shadowed-context-2.js deleted file mode 100644 index 5edd03ca58..0000000000 --- a/test/debugger/debug/debug-evaluate-shadowed-context-2.js +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright 2016 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - - -// Test that debug-evaluate correctly collects free outer variables -// and does not get confused by variables in nested scopes. - -Debug = debug.Debug - -var exception = null; -function listener(event, exec_state, event_data, data) { - if (event != Debug.DebugEvent.Break) return; - try { - assertThrows(() => exec_state.frame(0).evaluate("x").value()); - } catch (e) { - exception = e; - print(e + e.stack); - } -} - -Debug.setListener(listener); - -(function() { - var x = 1; // context allocate x - (() => x); - (function() { - var x = 2; // stack allocate shadowing x - (function() { - { // context allocate x in a nested scope - let x = 3; - (() => x); - } - debugger; - })(); - })(); -})(); - -Debug.setListener(null); -assertNull(exception); diff --git a/test/debugger/debug/debug-evaluate-shadowed-context-3.js b/test/debugger/debug/debug-evaluate-shadowed-context-3.js deleted file mode 100644 index 2a41109565..0000000000 --- a/test/debugger/debug/debug-evaluate-shadowed-context-3.js +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright 2019 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -// Test that debug-evaluate properly shadows stack-allocated variables. - -Debug = debug.Debug - -let exception = null; -function listener(event, exec_state, event_data, data) { - if (event != Debug.DebugEvent.Break) return; - try { - assertEquals(2, exec_state.frame(0).evaluate("b").value()); - assertEquals(3, exec_state.frame(0).evaluate("c").value()) - assertThrows(() => exec_state.frame(0).evaluate("a").value()); - } catch (e) { - exception = e; - print(e + e.stack); - } -} - -Debug.setListener(listener); - -(function f() { - let a = 1; - let b = 2; - let c = 3; - () => a + c; // a and c are context-allocated - return function g() { - let a = 2; // a is stack-allocated - return function h() { - b; // b is allocated onto f's context. - debugger; - } - } -})()()(); - -Debug.setListener(null); -assertNull(exception); diff --git a/test/debugger/debug/debug-evaluate-shadowed-context-4.js b/test/debugger/debug/debug-evaluate-shadowed-context-4.js deleted file mode 100644 index 52f0ac5c45..0000000000 --- a/test/debugger/debug/debug-evaluate-shadowed-context-4.js +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2022 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -// Test that debug-evaluate properly shadows stack-allocated variables. - -Debug = debug.Debug - -let exception = null; -function listener(event, exec_state, event_data, data) { - if (event != Debug.DebugEvent.Break) return; - try { - assertThrows(() => exec_state.frame(0).evaluate("a").value()); - } catch (e) { - exception = e; - print(e + e.stack); - } -} - -Debug.setListener(listener); - -(function f() { - let a = 1; - () => a; // a is context-allocated - return function g() { - let a = 2; // a is stack-allocated - { - let b = 3; - return function h() { - debugger; - } - } - } -})()()(); - -Debug.setListener(null); -assertNull(exception); diff --git a/test/debugger/debug/debug-evaluate-shadowed-context.js b/test/debugger/debug/debug-evaluate-shadowed-context.js index 564bdc6fc3..ba21643aa9 100644 --- a/test/debugger/debug/debug-evaluate-shadowed-context.js +++ b/test/debugger/debug/debug-evaluate-shadowed-context.js @@ -11,56 +11,86 @@ Debug = debug.Debug -var exception = null; +let test_name; +let listener_delegate; +let listener_called; +let exception; +let begin_test_count = 0; +let end_test_count = 0; +let break_count = 0; + +// Debug event listener which delegates. function listener(event, exec_state, event_data, data) { - if (event != Debug.DebugEvent.Break) return; try { - for (var i = 0; i < exec_state.frameCount() - 1; i++) { - var frame = exec_state.frame(i); - var value; - try { - value = frame.evaluate("x").value(); - } catch (e) { - value = e.name; - } - print(frame.sourceLineText()); - var expected = frame.sourceLineText().match(/\/\/ (.*$)/)[1]; - assertEquals(String(expected), String(value)); + if (event == Debug.DebugEvent.Break) { + break_count++; + listener_called = true; + listener_delegate(exec_state); } - assertEquals("[object global]", - String(exec_state.frame(0).evaluate("this").value())); - assertEquals("y", exec_state.frame(0).evaluate("y").value()); - assertEquals("a", exec_state.frame(0).evaluate("a").value()); - exec_state.frame(0).evaluate("a = 'A'"); - assertThrows(() => exec_state.frame(0).evaluate("z"), ReferenceError); } catch (e) { exception = e; - print(e + e.stack); + print(e, e.stack); } } - Debug.setListener(listener); +function BeginTest(name) { + test_name = name; + listener_called = false; + exception = null; + begin_test_count++; +} + +function EndTest() { + assertTrue(listener_called, "listener not called for " + test_name); + assertNull(exception, test_name + " / " + exception); + end_test_count++; +} + +BeginTest("Check that 'x' resolves correctly and 'a' is written correctly"); var a = "a"; -(function() { +function f1() { var x = 1; // context allocate x (() => x); var y = "y"; var z = "z"; - (function() { + (function () { var x = 2; // stack allocate shadowing x - (function() { + (function () { y; // access y debugger; // ReferenceError })(); // 2 })(); // 1 return y; -})(); +} +listener_delegate = function(exec_state) { + for (var i = 0; i < exec_state.frameCount() - 1; i++) { + var frame = exec_state.frame(i); + var value; + try { + value = frame.evaluate("x").value(); + } catch (e) { + value = e.name; + } + print(frame.sourceLineText()); + var expected = frame.sourceLineText().match(/\/\/ (.*$)/)[1]; + assertEquals(String(expected), String(value)); + } + assertEquals("[object global]", + String(exec_state.frame(0).evaluate("this").value())); + assertEquals("y", exec_state.frame(0).evaluate("y").value()); + assertEquals("a", exec_state.frame(0).evaluate("a").value()); + exec_state.frame(0).evaluate("a = 'A'"); + assertThrows(() => exec_state.frame(0).evaluate("z"), ReferenceError); +} +f1(); assertEquals("A", a); a = "a"; +EndTest(); -(function() { +BeginTest("Check that a context-allocated 'this' works") +function f2() { var x = 1; // context allocate x (() => x); var y = "y"; @@ -75,9 +105,80 @@ a = "a"; })(); // 2 })(); // 1 return y; -})(); +}; +// Uses the same listener delgate as for `f1`. +f2(); assertEquals("A", a); +EndTest(); -Debug.setListener(null); -assertNull(exception); +BeginTest("Check that we don't get confused with nested scopes"); +function f3() { + var x = 1; // context allocate x + (() => x); + (function() { + var x = 2; // stack allocate shadowing x + (function() { + { // context allocate x in a nested scope + let x = 3; + (() => x); + } + debugger; + })(); + })(); +} + +listener_delegate = function(exec_state) { + assertThrows(() => exec_state.frame(0).evaluate("x").value()); +} +f3(); +EndTest(); + +BeginTest("Check that stack-allocated variable is unavailable"); +function f4() { + let a = 1; + let b = 2; + let c = 3; + () => a + c; // a and c are context-allocated + return function g() { + let a = 2; // a is stack-allocated + return function h() { + b; // b is allocated onto f's context. + debugger; + } + } +} + +listener_delegate = function(exec_state) { + assertEquals(2, exec_state.frame(0).evaluate("b").value()); + assertEquals(3, exec_state.frame(0).evaluate("c").value()) + assertThrows(() => exec_state.frame(0).evaluate("a").value()); +}; +(f4())()(); +EndTest(); + +BeginTest("Check that block lists on the closure boundary work as expected"); +function f5() { + let a = 1; + () => a; // a is context-allocated + return function g() { + let a = 2; // a is stack-allocated + { + let b = 3; + return function h() { + debugger; + } + } + } +} + +listener_delegate = function(exec_state) { + assertThrows(() => exec_state.frame(0).evaluate("a").value()); +}; +(f5())()(); +EndTest(); + +assertEquals(begin_test_count, break_count, + 'one or more tests did not enter the debugger'); +assertEquals(begin_test_count, end_test_count, + 'one or more tests did not have its result checked');