dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

[VM/Debugger] Fix behaviour of setting breakpoints

Browse Source 
My older CL that aimed to fix inline breakpoints was not quite correct.
It introduced some desirable behaviour, but also some undesirable
behaviour like https://github.com/dart-lang/sdk/issues/54100. This CL
reverts most of the changes in
https://dart-review.googlesource.com/c/sdk/+/316440 and then builds on
top of that.

TEST=pkg/vm_service/test/column_breakpoint_test.dart,
pkg/vm_service/test/column_breakpoint_test.dart, other debugger tests

Fixes: https://github.com/dart-lang/sdk/issues/54100
Change-Id: I2f5fdd55070aea9412a0b37e83c930cd24382adf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/338040
Commit-Queue: Derek Xu <derekx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
This commit is contained in:
Derek Xu
2023-11-27 18:20:18 +00:00
committed by Commit Queue
parent 3e160c861f
commit ec30968b54
6 changed files with 143 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/vm_service/test/add_breakpoint_rpc_kernel_test.dart
+3 -1
View File
@@ -144,12 +144,14 @@ final tests = <IsolateTest>[
bpt.location!.tokenPos!,
)!;
print('$LINE_A:$col -> $resolvedLine:$resolvedCol');
if (col <= 12) {
if (col < 12) {
// The second 'incValue' begins at column 12.
expect(resolvedLine, LINE_A);
expect(bpt.location!.line, LINE_A);
expect(resolvedCol, 3);
expect(bpt.location!.column, 3);
} else if (col <= 36) {
// The newline character at the end of LINE_A is at column 36.
expect(resolvedLine, LINE_A);
expect(bpt.location!.line, LINE_A);
expect(resolvedCol, 12);
pkg/vm_service/test/break_on_function_declared_inside_map_test.dart
+56
View File
@@ -0,0 +1,56 @@
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'common/service_test_common.dart';
import 'common/test_helper.dart';
// AUTOGENERATED START
//
// Update these constants by running:
//
// dart pkg/vm_service/test/update_line_numbers.dart <test.dart>
//
const LINE_A = 22;
const LINE_B = 25;
// AUTOGENERATED END
const String shortFile = 'break_on_function_declared_inside_map_test.dart';
void testMain() {
final funcs = {
/* LINE_A */ 'a': () {
print('a');
},
/* LINE_B */ 'b': () {
print('b');
},
};
funcs['a']!();
funcs['b']!();
}
final stops = <String>[];
const expected = <String>[
'$shortFile:$LINE_A:23', // on '(' of '()'
'$shortFile:$LINE_B:23', // on '(' of '()'
];
final tests = <IsolateTest>[
hasPausedAtStart,
setBreakpointAtLineColumn(LINE_A, 24), // on ')' of '()'
setBreakpointAtLine(LINE_B),
resumeProgramRecordingStops(stops, false),
checkRecordedStops(stops, expected),
];
Future<void> main([args = const <String>[]]) => runIsolateTests(
args,
tests,
'break_on_function_declared_inside_map_test.dart',
testeeConcurrent: testMain,
pauseOnStart: true,
pauseOnExit: true,
);
pkg/vm_service/test/column_breakpoint_test.dart
+19 -11
View File
@@ -5,27 +5,35 @@
import 'common/service_test_common.dart';
import 'common/test_helper.dart';
void testMain() {
final b = [1, 2].map((i) => i == 0).toList();
print(b.length);
}
// AUTOGENERATED START
//
// Update these constants by running:
//
// dart pkg/vm_service/test/update_line_numbers.dart <test.dart>
//
const LINE_A = 21;
const LINE_B = 22;
// AUTOGENERATED END
const int LINE = 9;
const int COLUMN = 29;
const String shortFile = 'column_breakpoint_test.dart';
void testMain() {
final b = [1, 2].map((i) => i == 0).toList(); // LINE_A
print(b.length); // LINE_B
}
final stops = <String>[];
const expected = <String>[
'$shortFile:${LINE + 0}:33', // on 'i == 0'
'$shortFile:${LINE + 0}:33', // iterate twice
'$shortFile:${LINE + 1}:11', //on 'b.length'
'$shortFile:$LINE_A:33', // on first '=' of 'i == 0'
'$shortFile:$LINE_A:33', // iterate twice
'$shortFile:$LINE_B:11', // on 'l' of 'b.length'
];
final tests = <IsolateTest>[
hasPausedAtStart,
setBreakpointAtLineColumn(LINE, COLUMN), // on 'i == 0'
setBreakpointAtLineColumn(LINE + 1, 9), // on 'b.length'
setBreakpointAtLineColumn(LINE_A, 34), // on second '=' of 'i == 0'
setBreakpointAtLineColumn(LINE_B, 13), // on 'n' of 'b.length'
resumeProgramRecordingStops(stops, false),
checkRecordedStops(stops, expected),
];
runtime/observatory/tests/service/add_breakpoint_rpc_kernel_test.dart
+1 -1
View File
@@ -88,7 +88,7 @@ var tests = <IsolateTest>[
int resolvedLine = await bpt.location!.getLine() as int;
int resolvedCol = await bpt.location!.getColumn() as int;
print('$LINE_A:${col} -> ${resolvedLine}:${resolvedCol}');
if (col <= 12) {
if (col < 12) {
expect(resolvedLine, equals(LINE_A));
expect(resolvedCol, equals(3));
} else if (col <= 36) {
runtime/observatory/tests/service/column_breakpoint_test.dart
+8 -8
View File
@@ -6,12 +6,12 @@ import 'test_helper.dart';
import 'service_test_common.dart';
void testMain() {
var b = [1, 2].map((i) => i == 0).toList();
final b = [1, 2].map((i) => i == 0).toList();
print(b.length);
}
const int LINE = 9;
const int COLUMN = 29;
const LINE_A = 9;
const LINE_B = 10;
const String shortFile = "column_breakpoint_test.dart";
const String breakpointFile =
"package:observatory_test_package/column_breakpoint_test.dart";
@@ -19,15 +19,15 @@ const String breakpointFile =
List<String> stops = [];
const List<String> expected = [
"$shortFile:${LINE + 0}:29", // on 'i == 0'
"$shortFile:${LINE + 0}:29", // iterate twice
"$shortFile:${LINE + 1}:11" //on 'b.length'
'$shortFile:$LINE_A:33', // on first '=' of 'i == 0'
'$shortFile:$LINE_A:33', // iterate twice
'$shortFile:$LINE_B:11', // on 'l' of 'b.length'
];
final tests = <IsolateTest>[
hasPausedAtStart,
setBreakpointAtLineColumn(LINE, COLUMN), // on 'i == 0'
setBreakpointAtLineColumn(LINE + 1, 9), // on 'b.length'
setBreakpointAtLineColumn(LINE_A, 34), // on second '=' of 'i == 0'
setBreakpointAtLineColumn(LINE_B, 13), // on 'n' of 'b.length'
resumeProgramRecordingStops(stops, false),
checkRecordedStops(stops, expected)
];
runtime/vm/debugger.cc
+56 -19
View File
@@ -1895,7 +1895,12 @@ void Debugger::PauseException(const Instance& exc) {
ClearCachedStackTraces();
}
// Helper to refine the resolved token pos.
// Helper that refines the resolved token pos.
//
// If |requested_column| is specified, then |exact_token_pos| must be the exact
// position where the user requested a column breakpoint to be set. If
// |requested_column| is |-1|, |exact_token_pos| must be
// |TokenPosition::kNoSource|.
static void RefineBreakpointPos(const Script& script,
TokenPosition pos,
TokenPosition next_closest_token_position,
@@ -1907,6 +1912,10 @@ static void RefineBreakpointPos(const Script& script,
intptr_t* best_column,
intptr_t* best_line,
TokenPosition* best_token_pos) {
ASSERT(requested_column == -1 &&
exact_token_pos == TokenPosition::kNoSource ||
requested_column > -1 && exact_token_pos != TokenPosition::kNoSource);
intptr_t token_start_column = -1;
intptr_t token_line = -1;
if (requested_column >= 0) {
@@ -1914,14 +1923,19 @@ static void RefineBreakpointPos(const Script& script,
TokenPosition end_of_line_pos = TokenPosition::kNoSource;
script.GetTokenLocation(pos, &token_line, &token_start_column);
script.TokenRangeAtLine(token_line, &ignored, &end_of_line_pos);
TokenPosition token_end_pos =
TokenPosition::Min(next_closest_token_position, end_of_line_pos);
TokenPosition token_end_pos = TokenPosition::Min(
TokenPosition::Deserialize(next_closest_token_position.Pos() - 1),
end_of_line_pos);
if ((token_end_pos.IsReal() && exact_token_pos.IsReal() &&
(token_end_pos < exact_token_pos)) ||
(token_start_column > *best_column)) {
// Prefer the token with the lowest column number compatible
// with the requested column.
// We prefer the token with the lowest column number compatible with the
// requested column. The current token under consideration either ends
// before the requested column, and is thus incompatible with it, or
// has a higher column number than the best token we've found so far, so
// we reject the current token under consideration.
return;
}
}
@@ -1946,6 +1960,11 @@ static void RefineBreakpointPos(const Script& script,
// represents one line of the program text, but can represent a larger
// range on recursive calls.
//
// If |requested_column| is specified, then |exact_token_pos| must be the exact
// position where the user requested a column breakpoint to be set. If
// |requested_column| is |-1|, |exact_token_pos| must be
// |TokenPosition::kNoSource|.
//
// The best fit is found in two passes.
//
// The first pass finds a candidate token which:
@@ -2001,6 +2020,9 @@ static TokenPosition ResolveBreakpointPos(const Function& func,
intptr_t requested_column,
TokenPosition exact_token_pos) {
ASSERT(!func.HasOptimizedCode());
ASSERT(requested_column == -1 &&
exact_token_pos == TokenPosition::kNoSource ||
requested_column > -1 && exact_token_pos != TokenPosition::kNoSource);
requested_token_pos =
TokenPosition::Max(requested_token_pos, func.token_pos());
@@ -2432,6 +2454,10 @@ bool Debugger::FindBestFit(const Script& script,
return false;
}
// If |requested_column| is specified, then |exact_token_pos| must be the exact
// position where the user requested a column breakpoint to be set. If
// |requested_column| is |-1|, |exact_token_pos| must be
// |TokenPosition::kNoSource|.
BreakpointLocation* Debugger::SetCodeBreakpoints(
const GrowableHandlePtrArray<const Script>& scripts,
TokenPosition token_pos,
@@ -2440,6 +2466,10 @@ BreakpointLocation* Debugger::SetCodeBreakpoints(
intptr_t requested_column,
TokenPosition exact_token_pos,
const GrowableObjectArray& functions) {
ASSERT(requested_column == -1 &&
exact_token_pos == TokenPosition::kNoSource ||
requested_column > -1 && exact_token_pos != TokenPosition::kNoSource);
Function& function = Function::Handle();
function ^= functions.At(0);
TokenPosition breakpoint_pos = ResolveBreakpointPos(
@@ -2526,14 +2556,6 @@ BreakpointLocation* Debugger::SetBreakpoint(
}
}
TokenPosition exact_token_pos = token_pos;
#if !defined(DART_PRECOMPILED_RUNTIME)
if (token_pos != last_token_pos && requested_column >= 0) {
exact_token_pos =
FindExactTokenPosition(script, token_pos, requested_column);
}
#endif // !defined(DART_PRECOMPILED_RUNTIME)
if (!func.IsNull()) {
// There may be more than one function object for a given function
// in source code. There may be implicit closure functions, and
@@ -2550,6 +2572,13 @@ BreakpointLocation* Debugger::SetBreakpoint(
// have already been compiled. We can resolve the breakpoint now.
// If requested_column is larger than zero, [token_pos, last_token_pos]
// governs one single line of code.
TokenPosition exact_token_pos = TokenPosition::kNoSource;
#if !defined(DART_PRECOMPILED_RUNTIME)
if (token_pos != last_token_pos && requested_column >= 0) {
exact_token_pos =
FindExactTokenPosition(script, token_pos, requested_column);
}
#endif // !defined(DART_PRECOMPILED_RUNTIME)
DeoptimizeWorld();
BreakpointLocation* loc =
SetCodeBreakpoints(scripts, token_pos, last_token_pos, requested_line,
@@ -2565,7 +2594,7 @@ BreakpointLocation* Debugger::SetBreakpoint(
if (FLAG_verbose_debug) {
intptr_t line_number = -1;
intptr_t column_number = -1;
script.GetTokenLocation(exact_token_pos, &line_number, &column_number);
script.GetTokenLocation(token_pos, &line_number, &column_number);
if (func.IsNull()) {
OS::PrintErr(
"Registering pending breakpoint for "
@@ -2580,11 +2609,10 @@ BreakpointLocation* Debugger::SetBreakpoint(
}
const String& script_url = String::Handle(script.url());
BreakpointLocation* loc = GetBreakpointLocation(
script_url, exact_token_pos, requested_line, requested_column);
script_url, token_pos, requested_line, requested_column);
if (loc == nullptr) {
loc =
new BreakpointLocation(this, scripts, exact_token_pos, exact_token_pos,
requested_line, requested_column);
loc = new BreakpointLocation(this, scripts, token_pos, last_token_pos,
requested_line, requested_column);
RegisterBreakpointLocation(loc);
}
return loc;
@@ -2890,7 +2918,16 @@ void GroupDebugger::NotifyCompilation(const Function& function) {
location->debugger()->isolate());
// Ensure the location is resolved for the original function.
location->EnsureIsResolved(function, location->token_pos());
TokenPosition exact_token_pos = TokenPosition::kNoSource;
#if !defined(DART_PRECOMPILED_RUNTIME)
if (location->token_pos() != location->end_token_pos() &&
location->requested_column_number() >= 0) {
exact_token_pos = FindExactTokenPosition(
Script::Handle(location->script()), location->token_pos(),
location->requested_column_number());
}
#endif // !defined(DART_PRECOMPILED_RUNTIME)
location->EnsureIsResolved(function, exact_token_pos);
if (FLAG_verbose_debug) {
Breakpoint* bpt = location->breakpoints();
while (bpt != nullptr) {