Reviewers: Jakob,
Message:
PTAL.
http://codereview.chromium.org/9625011/diff/6/src/full-codegen.cc
File src/full-codegen.cc (right):
http://codereview.chromium.org/9625011/diff/6/src/full-codegen.cc#newcode1178
src/full-codegen.cc:1178: SetStatementPosition(stmt);
Only set a debug break slot if there is actually a statement.
http://codereview.chromium.org/9625011/diff/6/test/cctest/test-debug.cc
File test/cctest/test-debug.cc (right):
http://codereview.chromium.org/9625011/diff/6/test/cctest/test-debug.cc#newcode2748
test/cctest/test-debug.cc:2748: CHECK_EQ(34, break_point_hit_count);
We have exactly one debug break more whenever a for loop is involved.
Same for all changes in this file.
http://codereview.chromium.org/9625011/diff/6/test/mjsunit/debug-stepin-accessor.js
File test/mjsunit/debug-stepin-accessor.js (right):
http://codereview.chromium.org/9625011/diff/6/test/mjsunit/debug-stepin-accessor.js#newcode117
test/mjsunit/debug-stepin-accessor.js:117: var x = c['getter' + i];
We want to break before accessing c, not before entering the loop.
Description:
Set debug break slot at init of loop variable in a for loop.
BUG=102153
TEST=regress-102153.js
Please review this at http://codereview.chromium.org/9625011/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/full-codegen.cc
M test/cctest/test-debug.cc
M test/mjsunit/debug-stepin-accessor.js
A test/mjsunit/regress/regress-102153.js
Index: src/full-codegen.cc
diff --git a/src/full-codegen.cc b/src/full-codegen.cc
index
5f3c1d2ceb949eba99f3b60c1c8dd50df216732c..00804dbead115a26f0e5ed6ff3df2c5061a1ca46
100644
--- a/src/full-codegen.cc
+++ b/src/full-codegen.cc
@@ -1160,6 +1160,7 @@ void
FullCodeGenerator::VisitForStatement(ForStatement* stmt) {
Iteration loop_statement(this, stmt);
if (stmt->init() != NULL) {
+ SetStatementPosition(stmt);
Visit(stmt->init());
}
@@ -1173,8 +1174,8 @@ void
FullCodeGenerator::VisitForStatement(ForStatement* stmt) {
PrepareForBailoutForId(stmt->ContinueId(), NO_REGISTERS);
__ bind(loop_statement.continue_label());
- SetStatementPosition(stmt);
if (stmt->next() != NULL) {
+ SetStatementPosition(stmt);
Visit(stmt->next());
}
Index: test/cctest/test-debug.cc
diff --git a/test/cctest/test-debug.cc b/test/cctest/test-debug.cc
index
783c36d1de42cc138c584b58cec1770bb40ba517..af07e6847e1ade8048ba665cc27d57c6fdd7755a
100644
--- a/test/cctest/test-debug.cc
+++ b/test/cctest/test-debug.cc
@@ -2745,7 +2745,7 @@ TEST(DebugStepKeyedLoadLoop) {
foo->Call(env->Global(), kArgc, args);
// With stepping all break locations are hit.
- CHECK_EQ(33, break_point_hit_count);
+ CHECK_EQ(34, break_point_hit_count);
v8::Debug::SetDebugEventListener(NULL);
CheckDebuggerUnloaded();
@@ -2792,7 +2792,7 @@ TEST(DebugStepKeyedStoreLoop) {
foo->Call(env->Global(), kArgc, args);
// With stepping all break locations are hit.
- CHECK_EQ(32, break_point_hit_count);
+ CHECK_EQ(33, break_point_hit_count);
v8::Debug::SetDebugEventListener(NULL);
CheckDebuggerUnloaded();
@@ -2836,7 +2836,7 @@ TEST(DebugStepNamedLoadLoop) {
foo->Call(env->Global(), 0, NULL);
// With stepping all break locations are hit.
- CHECK_EQ(53, break_point_hit_count);
+ CHECK_EQ(54, break_point_hit_count);
v8::Debug::SetDebugEventListener(NULL);
CheckDebuggerUnloaded();
@@ -2880,7 +2880,7 @@ static void DoDebugStepNamedStoreLoop(int expected) {
// Test of the stepping mechanism for named load in a loop.
TEST(DebugStepNamedStoreLoop) {
- DoDebugStepNamedStoreLoop(22);
+ DoDebugStepNamedStoreLoop(23);
}
@@ -3252,7 +3252,7 @@ TEST(DebugStepForContinue) {
v8::Handle<v8::Value> argv_10[argc] = { v8::Number::New(10) };
result = foo->Call(env->Global(), argc, argv_10);
CHECK_EQ(5, result->Int32Value());
- CHECK_EQ(50, break_point_hit_count);
+ CHECK_EQ(51, break_point_hit_count);
// Looping 100 times.
step_action = StepIn;
@@ -3260,7 +3260,7 @@ TEST(DebugStepForContinue) {
v8::Handle<v8::Value> argv_100[argc] = { v8::Number::New(100) };
result = foo->Call(env->Global(), argc, argv_100);
CHECK_EQ(50, result->Int32Value());
- CHECK_EQ(455, break_point_hit_count);
+ CHECK_EQ(456, break_point_hit_count);
// Get rid of the debug event listener.
v8::Debug::SetDebugEventListener(NULL);
@@ -3304,7 +3304,7 @@ TEST(DebugStepForBreak) {
v8::Handle<v8::Value> argv_10[argc] = { v8::Number::New(10) };
result = foo->Call(env->Global(), argc, argv_10);
CHECK_EQ(9, result->Int32Value());
- CHECK_EQ(53, break_point_hit_count);
+ CHECK_EQ(54, break_point_hit_count);
// Looping 100 times.
step_action = StepIn;
@@ -3312,7 +3312,7 @@ TEST(DebugStepForBreak) {
v8::Handle<v8::Value> argv_100[argc] = { v8::Number::New(100) };
result = foo->Call(env->Global(), argc, argv_100);
CHECK_EQ(99, result->Int32Value());
- CHECK_EQ(503, break_point_hit_count);
+ CHECK_EQ(504, break_point_hit_count);
// Get rid of the debug event listener.
v8::Debug::SetDebugEventListener(NULL);
Index: test/mjsunit/debug-stepin-accessor.js
diff --git a/test/mjsunit/debug-stepin-accessor.js
b/test/mjsunit/debug-stepin-accessor.js
index
2c9c8c324f9125b3a386aa0f5efbc62e1892a1fa..ec84dbd8190a49d8d60b264a14fd5ad40f428ae3
100644
--- a/test/mjsunit/debug-stepin-accessor.js
+++ b/test/mjsunit/debug-stepin-accessor.js
@@ -112,8 +112,8 @@ function testGetter1_2() {
function testGetter1_3() {
expected_function_name = 'getter1';
expected_source_line_text = ' return this.name; // getter 1';
- debugger;
for (var i = 1; i < 2; i++) {
+ debugger;
var x = c['getter' + i];
}
}
Index: test/mjsunit/regress/regress-102153.js
diff --git a/test/mjsunit/regress/regress-102153.js
b/test/mjsunit/regress/regress-102153.js
new file mode 100644
index
0000000000000000000000000000000000000000..0f67656b61ea1ed17d8c9d5ee46d13dd54d250ea
--- /dev/null
+++ b/test/mjsunit/regress/regress-102153.js
@@ -0,0 +1,57 @@
+// Copyright 2012 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
+
+// Test that the break point is set before initializing the loop variable
+// so that we break before any iteration has been run.
+
+Debug = debug.Debug;
+
+var break_hit = false;
+
+function listener(event, exec_state, event_data, data) {
+ if (event == Debug.DebugEvent.Break) {
+ break_hit = true;
+ }
+}
+
+Debug.setListener(listener);
+
+function test() {
+ for (var i = 0; i < 3; i++) { // Break here.
+ if (i == 0) break;
+ }
+}
+
+Debug.setBreakPoint(test, 1, 0);
+
+assertTrue(Debug.showBreakPoints(test).indexOf("// Break here.") >= 0);
+
+test();
+
+assertTrue(break_hit);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
