diff --git a/pkg/vm_service/test/add_breakpoint_rpc_kernel_test.dart b/pkg/vm_service/test/add_breakpoint_rpc_kernel_test.dart index d9200af2a57..3d5da038b45 100644 --- a/pkg/vm_service/test/add_breakpoint_rpc_kernel_test.dart +++ b/pkg/vm_service/test/add_breakpoint_rpc_kernel_test.dart @@ -36,7 +36,6 @@ Future testMain() async { final tests = [ hasPausedAtStart, - // Test future breakpoints. (VmService service, IsolateRef isolateRef) async { final isolateId = isolateRef.id!; final isolate = await service.getIsolate(isolateId); @@ -44,60 +43,23 @@ final tests = [ await service.getObject(isolateId, isolate.rootLib!.id!) as Library; final rootLibId = rootLib.id!; final scriptId = rootLib.scripts![0].id!; - final script = await service.getObject(isolateId, scriptId) as Script; - // Future breakpoint. - var futureBpt1 = await service.addBreakpoint(isolateId, scriptId, LINE_A); - expect(futureBpt1.breakpointNumber, 1); - expect(futureBpt1.resolved, isFalse); - expect(await futureBpt1.location!.line!, LINE_A); - expect(await futureBpt1.location!.column, null); + final bpt1 = await service.addBreakpoint(isolateId, scriptId, LINE_A); + expect(bpt1.breakpointNumber, 1); + expect(bpt1.resolved, true); + expect(await bpt1.location!.line!, LINE_A); + expect(await bpt1.location!.column, 12); - // Future breakpoint with specific column. - var futureBpt2 = + // Breakpoint with specific column. + final bpt2 = await service.addBreakpoint(isolateId, scriptId, LINE_A, column: 3); - expect(futureBpt2.breakpointNumber, 2); - expect(futureBpt2.resolved, isFalse); - expect(await futureBpt2.location!.line!, LINE_A); - expect(await futureBpt2.location!.column!, 3); - - final int resolvedCount = await resumeAndCountResolvedBreakpointsUntilPause( - service, - isolate, - ); - - // After resolution the breakpoints have assigned line & column. - expect(resolvedCount, 2); - - // Refresh objects - futureBpt1 = - await service.getObject(isolateId, futureBpt1.id!) as Breakpoint; - futureBpt2 = - await service.getObject(isolateId, futureBpt2.id!) as Breakpoint; - - expect(futureBpt1.resolved, isTrue); - expect( - script.getLineNumberFromTokenPos(futureBpt1.location!.tokenPos!), - LINE_A, - ); - expect(futureBpt1.location!.line, LINE_A); - expect( - script.getColumnNumberFromTokenPos(futureBpt1.location!.tokenPos!), - 12, - ); - expect(futureBpt1.location!.column, 12); - expect(futureBpt2.resolved, isTrue); - expect( - script.getLineNumberFromTokenPos(futureBpt2.location!.tokenPos!), - LINE_A, - ); - expect(futureBpt2.location!.line, LINE_A); - expect( - script.getColumnNumberFromTokenPos(futureBpt2.location!.tokenPos!), - 3, - ); - expect(futureBpt2.location!.column, 3); + expect(bpt2.breakpointNumber, 2); + expect(bpt2.resolved, true); + expect(await bpt2.location!.line!, LINE_A); + expect(await bpt2.location!.column!, 3); + await service.resume(isolateId); + await hasStoppedAtBreakpoint(service, isolate); // The first breakpoint hits before value is modified. InstanceRef result = await service.evaluate(isolateId, rootLibId, 'value') as InstanceRef; @@ -105,7 +67,6 @@ final tests = [ await service.resume(isolateId); await hasStoppedAtBreakpoint(service, isolate); - // The second breakpoint hits after value has been modified once. result = await service.evaluate(isolateId, rootLibId, 'value') as InstanceRef; @@ -113,11 +74,11 @@ final tests = [ // Remove the breakpoints. expect( - (await service.removeBreakpoint(isolateId, futureBpt1.id!)).type, + (await service.removeBreakpoint(isolateId, bpt1.id!)).type, 'Success', ); expect( - (await service.removeBreakpoint(isolateId, futureBpt2.id!)).type, + (await service.removeBreakpoint(isolateId, bpt2.id!)).type, 'Success', ); }, @@ -182,31 +143,7 @@ final tests = [ }, ]; -Future resumeAndCountResolvedBreakpointsUntilPause( - VmService service, - Isolate isolate, -) async { - final completer = Completer(); - late StreamSubscription subscription; - int resolvedCount = 0; - - subscription = service.onDebugEvent.listen((event) { - if (event.kind == EventKind.kBreakpointResolved) { - resolvedCount++; - } else if (event.kind == EventKind.kPauseBreakpoint) { - subscription.cancel(); - service.streamCancel(EventStreams.kDebug); - completer.complete(); - } - }); - await service.streamListen(EventStreams.kDebug); - - await service.resume(isolate.id!); - await completer.future; - return resolvedCount; -} - -void main([args = const []]) => runIsolateTests( +Future main(args) => runIsolateTests( args, tests, 'add_breakpoint_rpc_kernel_test.dart', diff --git a/pkg/vm_service/test/breakpoint_async_break_test.dart b/pkg/vm_service/test/breakpoint_async_break_test.dart index fd54ea5397b..23b7db22ed9 100644 --- a/pkg/vm_service/test/breakpoint_async_break_test.dart +++ b/pkg/vm_service/test/breakpoint_async_break_test.dart @@ -24,57 +24,25 @@ Future testMain() async { var tests = [ hasPausedAtStart, - // Test future breakpoints. (VmService service, IsolateRef isolateRef) async { final isolateId = isolateRef.id!; final isolate = await service.getIsolate(isolateId); final rootLib = await service.getObject(isolateId, isolate.rootLib!.id!) as Library; final scriptId = rootLib.scripts![0].id!; - final script = await service.getObject(isolateId, scriptId) as Script; - // Future breakpoint. - var futureBpt = await service.addBreakpoint(isolateId, scriptId, LINE); - expect(futureBpt.breakpointNumber, 1); - expect(futureBpt.resolved, isFalse); - expect(await futureBpt.location!.line, LINE); - expect(await futureBpt.location!.column, null); + final bpt = await service.addBreakpoint(isolateId, scriptId, LINE); + expect(bpt.breakpointNumber, 1); + expect(bpt.resolved, isTrue); + expect(await bpt.location!.line, LINE); + expect(await bpt.location!.column, 7); - final completer = Completer(); - int resolvedCount = 0; - late StreamSubscription subscription; - subscription = service.onDebugEvent.listen((event) { - if (event.kind == EventKind.kBreakpointResolved) { - resolvedCount++; - } else if (event.kind == EventKind.kPauseBreakpoint) { - subscription.cancel(); - service.streamCancel(EventStreams.kDebug); - completer.complete(); - } - }); - - await service.streamListen(EventStreams.kDebug); await service.resume(isolateId); await hasStoppedAtBreakpoint(service, isolate); - // After resolution the breakpoints have assigned line & column. - expect(resolvedCount, 1); - futureBpt = await service.getObject(isolateId, futureBpt.id!) as Breakpoint; - expect(futureBpt.resolved, isTrue); - expect( - script.getLineNumberFromTokenPos(futureBpt.location!.tokenPos), - LINE, - ); - expect(futureBpt.location!.line, LINE); - expect( - script.getColumnNumberFromTokenPos(futureBpt.location!.tokenPos), - COL, - ); - expect(futureBpt.location!.column, COL); - // Remove the breakpoints. expect( - (await service.removeBreakpoint(isolateId, futureBpt.id!)).type, + (await service.removeBreakpoint(isolateId, bpt.id!)).type, 'Success', ); }, diff --git a/pkg/vm_service/test/breakpoint_inside_nested_closures_test.dart b/pkg/vm_service/test/breakpoint_inside_nested_closures_test.dart new file mode 100644 index 00000000000..442de882e89 --- /dev/null +++ b/pkg/vm_service/test/breakpoint_inside_nested_closures_test.dart @@ -0,0 +1,49 @@ +// Copyright (c) 2024, 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 +// +const LINE_A = 23; +// AUTOGENERATED END + +int f() { + return (() { + (() { + return 1 + 2; + })(); + return (() { + return 3 + 4; // LINE_A + })(); + })(); +} + +void testeeMain() { + f(); +} + +final tests = [ + hasPausedAtStart, + setBreakpointAtLine(LINE_A), + resumeIsolate, + hasStoppedAtBreakpoint, + stoppedAtLine(LINE_A), + resumeIsolate, + hasStoppedAtExit, +]; + +void main([args = const []]) => runIsolateTests( + args, + tests, + 'breakpoint_inside_nested_closures_test.dart', + testeeConcurrent: testeeMain, + pauseOnStart: true, + pauseOnExit: true, + ); diff --git a/pkg/vm_service/test/set_reload_and_reset_breakpoints_test.dart b/pkg/vm_service/test/set_reload_and_reset_breakpoints_test.dart new file mode 100644 index 00000000000..754d03a39fd --- /dev/null +++ b/pkg/vm_service/test/set_reload_and_reset_breakpoints_test.dart @@ -0,0 +1,175 @@ +// Copyright (c) 2024, 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 'dart:developer' show debugger; +import 'dart:io' show Directory, File; +import 'dart:isolate' as i; + +import 'package:path/path.dart' show join; +import 'package:test/test.dart'; +import 'package:vm_service/vm_service.dart'; + +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 +// +const LINE_A = 44; +// AUTOGENERATED END + +const v0Contents = ''' +import 'dart:developer'; + +void f() {} + +void main() { + debugger(); + f(); +} +'''; + +Future testeeMain() async { + // Spawn the child isolate. + final tempDir = Directory.systemTemp.createTempSync(); + try { + final rootLib = File(join(tempDir.path, 'main.dart')); + rootLib.writeAsStringSync(v0Contents); + + await i.Isolate.spawnUri(rootLib.uri, [], null); + debugger(); // LINE_A + tempDir.deleteSync(recursive: true); + } catch (_) { + tempDir.deleteSync(recursive: true); + rethrow; + } +} + +final tests = [ + // Ensure that the main isolate has stopped at the [debugger] statement at the + // end of [testeeMain]. + hasStoppedAtBreakpoint, + stoppedAtLine(LINE_A), + (VmService service, IsolateRef isolateRef) async { + final tempDir = Directory.systemTemp.createTempSync(); + try { + const numberOfBreakpoints = 20; + + final v1Contents = StringBuffer(); + v1Contents.write(''' +import 'dart:developer'; + +void f() { +'''); + + for (int i = 0; i < numberOfBreakpoints; i++) { + // Add closure definitions to [v1Contents]. The test will set + // breakpoints at the [print] calls in these closures to make the + // debugger eagerly compile the closures. + v1Contents.write(''' + (() { + print($i); + })(); + '''); + } + + v1Contents.write(''' +} + +void main() { + debugger(); + f(); +} +'''); + + final spawnedIsolateRootLib = File(join(tempDir.path, 'main.dart')); + + // Find the spawned isolate. + final vm = await service.getVM(); + final isolates = vm.isolates!; + expect(isolates.length, 2); + final spawnedIsolateRef = isolates.firstWhere( + (i) => i != isolateRef, + ); + final spawnedIsolateId = spawnedIsolateRef.id!; + + // Load [v1Contents] into the spawned isolate. + spawnedIsolateRootLib.writeAsStringSync(v1Contents.toString()); + await service.reloadSources( + spawnedIsolateId, + rootLibUri: spawnedIsolateRootLib.uri.toString(), + force: true, + ); + + Isolate spawnedIsolate = await service.getIsolate(spawnedIsolateId); + Library rootLib = await service.getObject( + spawnedIsolateId, + spawnedIsolate.rootLib!.id!, + ) as Library; + String scriptId = rootLib.scripts![0].id!; + + final testStartTime = DateTime.now(); + + // Add a breakpoints at all of the `print` calls. + for (int i = 0; i < numberOfBreakpoints; i++) { + expect( + (await service.addBreakpoint(spawnedIsolateId, scriptId, 5 + 3 * i)) + .resolved, + true, + ); + } + + // Force a reload of the sources. + await service.reloadSources( + spawnedIsolateId, + rootLibUri: spawnedIsolateRootLib.uri.toString(), + force: true, + ); + + spawnedIsolate = await service.getIsolate(spawnedIsolateId); + rootLib = await service.getObject( + spawnedIsolateId, + spawnedIsolate.rootLib!.id!, + ) as Library; + scriptId = rootLib.scripts![0].id!; + + // Remove the [numberOfBreakpoints] existing breakpoints. + for (int i = 1; i <= numberOfBreakpoints; i++) { + await service.removeBreakpoint(spawnedIsolateId, 'breakpoints/$i'); + } + // Add [numberOfBreakpoints] new breakpoints. + for (int i = 0; i < numberOfBreakpoints; i++) { + expect( + (await service.addBreakpoint(spawnedIsolateId, scriptId, 5 + 3 * i)) + .resolved, + true, + ); + } + // Remove the [numberOfBreakpoints] new breakpoints. + for (int i = numberOfBreakpoints + 1; i <= numberOfBreakpoints * 2; i++) { + await service.removeBreakpoint(spawnedIsolateId, 'breakpoints/$i'); + } + + print( + 'Took ${DateTime.now().difference(testStartTime).inMilliseconds}ms', + ); + + await resumeIsolate(service, spawnedIsolateRef); + tempDir.deleteSync(recursive: true); + } catch (_) { + tempDir.deleteSync(recursive: true); + rethrow; + } + }, +]; + +void main([args = const []]) => runIsolateTests( + args, + tests, + 'set_reload_and_reset_breakpoints_test.dart', + testeeConcurrent: testeeMain, + ); diff --git a/runtime/observatory/tests/service/add_breakpoint_rpc_kernel_test.dart b/runtime/observatory/tests/service/add_breakpoint_rpc_kernel_test.dart index 67fa8babee3..2e126027030 100644 --- a/runtime/observatory/tests/service/add_breakpoint_rpc_kernel_test.dart +++ b/runtime/observatory/tests/service/add_breakpoint_rpc_kernel_test.dart @@ -28,54 +28,39 @@ Future testMain() async { var tests = [ hasPausedAtStart, - // Test future breakpoints. (Isolate isolate) async { var rootLib = isolate.rootLibrary; await rootLib.load(); var script = rootLib.scripts[0]; - // Future breakpoint. - var futureBpt1 = await isolate.addBreakpoint(script, LINE_A); - expect(futureBpt1.number, equals(1)); - expect(futureBpt1.resolved, isFalse); - expect(await futureBpt1.location!.getLine(), equals(LINE_A)); - expect(await futureBpt1.location!.getColumn(), equals(null)); + final bpt1 = await isolate.addBreakpoint(script, LINE_A); + expect(bpt1.number, equals(1)); + expect(bpt1.resolved, true); + expect(await bpt1.location!.getLine(), equals(LINE_A)); + expect(await bpt1.location!.getColumn(), equals(12)); - // Future breakpoint with specific column. - var futureBpt2 = await isolate.addBreakpoint(script, LINE_A, 3); - expect(futureBpt2.number, equals(2)); - expect(futureBpt2.resolved, isFalse); - expect(await futureBpt2.location!.getLine(), equals(LINE_A)); - expect(await futureBpt2.location!.getColumn(), equals(3)); - - int resolvedCount = - await resumeAndCountResolvedBreakpointsUntilPause(isolate); - - // After resolution the breakpoints have assigned line & column. - expect(resolvedCount, equals(2)); - expect(futureBpt1.resolved, isTrue); - expect(await futureBpt1.location!.getLine(), equals(LINE_A)); - expect(await futureBpt1.location!.getColumn(), equals(12)); - expect(futureBpt2.resolved, isTrue); - expect(await futureBpt2.location!.getLine(), equals(LINE_A)); - expect(await futureBpt2.location!.getColumn(), equals(3)); + // Breakpoint with specific column. + final bpt2 = await isolate.addBreakpoint(script, LINE_A, 3); + expect(bpt2.number, equals(2)); + expect(bpt2.resolved, true); + expect(await bpt2.location!.getLine(), equals(LINE_A)); + expect(await bpt2.location!.getColumn(), equals(3)); + await isolate.resume(); + await hasStoppedAtBreakpoint(isolate); // The first breakpoint hits before value is modified. Instance result = await rootLib.evaluate('value') as Instance; expect(result.valueAsString, equals('0')); - isolate.resume(); + await isolate.resume(); await hasStoppedAtBreakpoint(isolate); - // The second breakpoint hits after value has been modified once. result = await rootLib.evaluate('value') as Instance; expect(result.valueAsString, equals('1')); // Remove the breakpoints. - expect( - (await isolate.removeBreakpoint(futureBpt1)).type, equals('Success')); - expect( - (await isolate.removeBreakpoint(futureBpt2)).type, equals('Success')); + expect((await isolate.removeBreakpoint(bpt1)).type, equals('Success')); + expect((await isolate.removeBreakpoint(bpt2)).type, equals('Success')); }, // Test resolution of column breakpoints. @@ -115,24 +100,5 @@ var tests = [ }, ]; -Future resumeAndCountResolvedBreakpointsUntilPause(Isolate isolate) async { - var stream = await isolate.vm.getEventStream(VM.kDebugStream); - Completer completer = new Completer(); - var subscription; - int resolvedCount = 0; - subscription = stream.listen((ServiceEvent event) async { - if (event.kind == ServiceEvent.kBreakpointResolved) { - resolvedCount++; - } - if (event.kind == ServiceEvent.kPauseBreakpoint) { - subscription.cancel(); - completer.complete(); - } - }); - await isolate.resume(); - await completer.future; - return resolvedCount; -} - main(args) => runIsolateTests(args, tests, testeeConcurrent: testMain, pause_on_start: true); diff --git a/runtime/observatory/tests/service/breakpoint_async_break_test.dart b/runtime/observatory/tests/service/breakpoint_async_break_test.dart index 671577ec344..138fad13449 100644 --- a/runtime/observatory/tests/service/breakpoint_async_break_test.dart +++ b/runtime/observatory/tests/service/breakpoint_async_break_test.dart @@ -22,44 +22,23 @@ Future testMain() async { var tests = [ hasPausedAtStart, - - // Test future breakpoints. (Isolate isolate) async { var rootLib = isolate.rootLibrary; await rootLib.load(); var script = rootLib.scripts[0]; // Future breakpoint. - var futureBpt = await isolate.addBreakpoint(script, LINE); - expect(futureBpt.number, 1); - expect(futureBpt.resolved, isFalse); - expect(await futureBpt.location!.getLine(), LINE); - expect(await futureBpt.location!.getColumn(), null); + var bpt = await isolate.addBreakpoint(script, LINE); + expect(bpt.number, 1); + expect(bpt.resolved, isTrue); + expect(await bpt.location!.getLine(), LINE); + expect(await bpt.location!.getColumn(), 7); - var stream = await isolate.vm.getEventStream(VM.kDebugStream); - Completer completer = new Completer(); - var subscription; - var resolvedCount = 0; - subscription = stream.listen((ServiceEvent event) async { - if (event.kind == ServiceEvent.kBreakpointResolved) { - resolvedCount++; - } - if (event.kind == ServiceEvent.kPauseBreakpoint) { - subscription.cancel(); - completer.complete(); - } - }); await isolate.resume(); await hasStoppedAtBreakpoint(isolate); - // After resolution the breakpoints have assigned line & column. - expect(resolvedCount, 1); - expect(futureBpt.resolved, isTrue); - expect(await futureBpt.location!.getLine(), LINE); - expect(await futureBpt.location!.getColumn(), 7); - // Remove the breakpoints. - expect((await isolate.removeBreakpoint(futureBpt)).type, 'Success'); + expect((await isolate.removeBreakpoint(bpt)).type, 'Success'); }, ]; diff --git a/runtime/tests/vm/dart/regress_48523_test.dart b/runtime/tests/vm/dart/regress_48523_test.dart index a3c7bb81ade..8a1c518d33d 100644 --- a/runtime/tests/vm/dart/regress_48523_test.dart +++ b/runtime/tests/vm/dart/regress_48523_test.dart @@ -2,9 +2,6 @@ // 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 'dart:async'; -import 'dart:convert'; - import 'package:expect/expect.dart'; import 'isolates/reload_utils.dart'; @@ -18,30 +15,17 @@ main() async { await reloader.waitUntilStdoutContains('[testee] helper isolate is ready'); - final helperIsolateId = await reloader.getIsolateId('helper-isolate'); - - // Set breakpoint. - final debugEvents = StreamIterator(reloader.getDebugStream()); - await reloader.addBreakpoint(7, isolateId: helperIsolateId); - // Reload 1 final reloadResult1 = await reloader.reload(dills[1]); Expect.equals('ReloadReport', reloadResult1['type']); Expect.equals(true, reloadResult1['success']); - // Now we should get a debug resolved event. - if (!await debugEvents.moveNext()) throw 'failed'; - final event = debugEvents.current; - print(JsonEncoder.withIndent(' ').convert(event)); - if (event['kind'] != 'BreakpointResolved') throw 'failed'; - print('Got breakpoint resolved event ($event)'); - // Continue testee, which will run (and therefore compile) old closure // without a script. await reloader.waitUntilStdoutContains('[testee] running old closure'); await reloader.waitUntilStdoutContains('[testee] done'); - // Reload 1 + // Reload 2 print('reloading'); final reloadResult2 = await reloader.reload(dills[2]); Expect.equals('ReloadReport', reloadResult2['type']); @@ -78,13 +62,10 @@ Future main() async { ReceivePort(); }, null, debugName: 'helper-isolate'); - // Debugger should now set breakpoint on - // myOldClosure. - // Wait until we got reloaded. while (await waitUntilReloadDone()); - // Now run the old closure (which has breakpoint in it). + // Now run the old closure. escapedOldClosure(); print('[testee] done'); diff --git a/runtime/vm/debugger.cc b/runtime/vm/debugger.cc index fc4e59ae9ee..a18d064e665 100644 --- a/runtime/vm/debugger.cc +++ b/runtime/vm/debugger.cc @@ -2277,14 +2277,15 @@ void GroupDebugger::MakeCodeBreakpointAt(const Function& func, } } -ErrorPtr Debugger::FindCompiledFunctions( +ErrorPtr Debugger::FindAndCompileMatchingFunctions( const GrowableHandlePtrArray& scripts, TokenPosition start_pos, TokenPosition end_pos, - GrowableObjectArray* code_function_list) { + GrowableObjectArray& code_function_list) const { auto thread = Thread::Current(); auto zone = thread->zone(); Script& script = Script::Handle(zone); + Object& ensure_has_code_result = Object::Handle(zone); for (intptr_t i = 0; i < scripts.length(); ++i) { script = scripts.At(i).ptr(); ClosureFunctionsCache::ForAllClosureFunctions( @@ -2292,14 +2293,21 @@ ErrorPtr Debugger::FindCompiledFunctions( ASSERT(!function.IsNull()); if ((function.token_pos() == start_pos) && (function.end_token_pos() == end_pos) && - (function.script() == script.ptr())) { - if (function.is_debuggable() && function.HasCode()) { - code_function_list->Add(function); + (function.script() == script.ptr()) && function.is_debuggable()) { + // If we've found a matching function, ensure it's compiled so + // the breakpoint currently being set can be resolved immediately. + ensure_has_code_result = function.EnsureHasCodeNoThrow(); + if (ensure_has_code_result.IsError()) { + return false; // Stop iterating. } + code_function_list.Add(function); ASSERT(!function.HasImplicitClosureFunction()); } - return true; // Continue iteration. + return true; // Continue iterating. }); + if (ensure_has_code_result.IsError()) { + return Error::Cast(ensure_has_code_result).ptr(); + } Class& cls = Class::Handle(zone); Array& functions = Array::Handle(zone); @@ -2333,17 +2341,28 @@ ErrorPtr Debugger::FindCompiledFunctions( function ^= functions.At(pos); ASSERT(!function.IsNull()); bool function_added = false; - if (function.is_debuggable() && function.HasCode() && - function.token_pos() == start_pos && + if (function.is_debuggable() && function.token_pos() == start_pos && function.end_token_pos() == end_pos && function.script() == script.ptr()) { - code_function_list->Add(function); + // If we've found a matching function, ensure it's compiled so + // the breakpoint currently being set can be resolved immediately. + ensure_has_code_result = function.EnsureHasCodeNoThrow(); + if (ensure_has_code_result.IsError()) { + return Error::Cast(ensure_has_code_result).ptr(); + } + code_function_list.Add(function); function_added = true; } if (function_added && function.HasImplicitClosureFunction()) { function = function.ImplicitClosureFunction(); - if (function.is_debuggable() && function.HasCode()) { - code_function_list->Add(function); + if (function.is_debuggable()) { + // Ensure that the implicit closure function is compiled so the + // breakpoint currently being set can be resolved immediately. + ensure_has_code_result = function.EnsureHasCodeNoThrow(); + if (ensure_has_code_result.IsError()) { + return Error::Cast(ensure_has_code_result).ptr(); + } + code_function_list.Add(function); } } } @@ -2366,7 +2385,11 @@ ErrorPtr Debugger::FindCompiledFunctions( function.token_pos() == start_pos && function.end_token_pos() == end_pos && function.script() == script.ptr()) { - code_function_list->Add(function); + ensure_has_code_result = function.EnsureHasCodeNoThrow(); + if (ensure_has_code_result.IsError()) { + return Error::Cast(ensure_has_code_result).ptr(); + } + code_function_list.Add(function); } } } @@ -2387,10 +2410,11 @@ static void UpdateBestFit(Function* best_fit, const Function& func) { } } -// Returns true if a best fit is found. A best fit can either be a function -// or a field. If it is a function, then the best fit function is returned -// in |best_fit|. If a best fit is a field, it means that a latent -// breakpoint can be set in the range |token_pos| to |last_token_pos|. +// If a best fit function is found, stores that function in |best_fit| and +// returns |true|. +// Note that in some cases, there may be a closure that is a better fit that the +// function returned in |best_fit|, because |FindBestFit| is only able to detect +// that closure if the function containing it has already been compiled. bool Debugger::FindBestFit(const Script& script, TokenPosition token_pos, TokenPosition last_token_pos, @@ -2485,9 +2509,11 @@ bool Debugger::FindBestFit(const Script& script, for (intptr_t pos = 0; pos < num_functions; pos++) { function ^= functions.At(pos); ASSERT(!function.IsNull()); - if (IsImplicitFunction(function)) { - // Implicit functions do not have a user specifiable source - // location. + if (IsImplicitFunction(function) || function.is_synthetic() || + !function.is_debuggable()) { + // We skip implicit functions and synthetic functions because they + // do not have user specifiable source locations. We also skip + // functions marked as undebuggable. continue; } if (FunctionOverlaps(function, script_url, token_pos, @@ -2523,7 +2549,7 @@ bool Debugger::FindBestFit(const Script& script, end = field.end_token_pos(); if (token_pos.IsWithin(start, end) || start.IsWithin(token_pos, last_token_pos)) { - *best_fit = field.InitializerFunction(); + *best_fit = field.EnsureInitializerFunction(); return true; } } @@ -2632,7 +2658,22 @@ ErrorPtr Debugger::SetBreakpoint( if (!FindBestFit(script, token_pos, last_token_pos, &func)) { return Object::no_debuggable_code_error().ptr(); } - // If func was not set (still Null), the best fit is a field. + ASSERT(!func.IsNull()); + // There may be closures that were not considered by the first call to + // |FindBestFit| because the functions containing them were not yet + // compiled. So, we must recursively compile functions until we converge + // at a true best fit function. + Function& tmp = Function::Handle(); + Object& ensure_has_code_result = Object::Handle(); + do { + ensure_has_code_result = func.EnsureHasCodeNoThrow(); + if (ensure_has_code_result.IsError()) { + return Error::Cast(ensure_has_code_result).ptr(); + } + tmp = func.ptr(); + FindBestFit(script, token_pos, last_token_pos, &func); + ASSERT(!func.IsNull()); + } while (tmp.ptr() != func.ptr()); } else { func = function.ptr(); if (!func.token_pos().IsReal()) { @@ -2642,15 +2683,16 @@ ErrorPtr Debugger::SetBreakpoint( } 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 - // there may be copies of mixin functions. Collect all compiled - // functions whose source code range matches exactly the best fit - // function we found. + // There may be more than one function object for a given function in source + // code. There may be implicit closure functions, and there may be copies of + // mixin functions. Compile and collect all functions whose source code + // range matches exactly the best fit function we found. We compile all of + // theses functions eagerly so that the breakpoint currently being set can + // be resolved immediately. GrowableObjectArray& code_functions = GrowableObjectArray::Handle(GrowableObjectArray::New()); - const Error& error = Error::Handle(FindCompiledFunctions( - scripts, func.token_pos(), func.end_token_pos(), &code_functions)); + const Error& error = Error::Handle(FindAndCompileMatchingFunctions( + scripts, func.token_pos(), func.end_token_pos(), code_functions)); if (!error.IsNull()) { return error.ptr(); } @@ -2682,34 +2724,8 @@ ErrorPtr Debugger::SetBreakpoint( } } } - // There is either an uncompiled function, or an uncompiled function literal - // initializer of a field at |token_pos|. Hence, Register an unresolved - // breakpoint. - if (FLAG_verbose_debug) { - intptr_t line_number = -1; - intptr_t column_number = -1; - script.GetTokenLocation(token_pos, &line_number, &column_number); - if (func.IsNull()) { - OS::PrintErr( - "Registering pending breakpoint for " - "an uncompiled function literal at line %" Pd " col %" Pd "\n", - line_number, column_number); - } else { - OS::PrintErr( - "Registering pending breakpoint for " - "uncompiled function '%s' at line %" Pd " col %" Pd "\n", - func.ToFullyQualifiedCString(), line_number, column_number); - } - } - const String& script_url = String::Handle(script.url()); - BreakpointLocation* loc = GetBreakpointLocation( - script_url, token_pos, requested_line, requested_column); - if (loc == nullptr) { - loc = new BreakpointLocation(this, scripts, token_pos, last_token_pos, - requested_line, requested_column); - RegisterBreakpointLocation(loc); - } - *result_breakpoint_location = loc; + // There is no debuggable code at all at |token_pos|, so we leave + // |*result_breakpoint_location| as null. return Error::null(); } diff --git a/runtime/vm/debugger.h b/runtime/vm/debugger.h index dd29e15e815..8dc7311f4bd 100644 --- a/runtime/vm/debugger.h +++ b/runtime/vm/debugger.h @@ -830,11 +830,15 @@ class Debugger { void SendBreakpointEvent(ServiceEvent::EventKind kind, Breakpoint* bpt); - ErrorPtr FindCompiledFunctions( + // Finds all |Function|s that span the token range [start_pos, end_pos] in any + // of the scripts in |scripts|, compiles these functions, and then adds them + // to |code_function_list|. If an error occurs during compilation, the error + // is returned. Otherwise, |Error::null()| is returned. + ErrorPtr FindAndCompileMatchingFunctions( const GrowableHandlePtrArray& scripts, TokenPosition start_pos, TokenPosition end_pos, - GrowableObjectArray* code_function_list); + GrowableObjectArray& code_function_list) const; bool FindBestFit(const Script& script, TokenPosition token_pos, TokenPosition last_token_pos, diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index e2d17c06405..083b6aa9e62 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -11340,13 +11340,14 @@ bool Function::CheckSourceFingerprint(int32_t fp, const char* kind) const { } CodePtr Function::EnsureHasCode() const { - if (HasCode()) return CurrentCode(); + if (HasCode()) { + return CurrentCode(); + } Thread* thread = Thread::Current(); ASSERT(thread->IsDartMutatorThread()); DEBUG_ASSERT(thread->TopErrorHandlerIsExitFrame()); Zone* zone = thread->zone(); - const Object& result = - Object::Handle(zone, Compiler::CompileFunction(thread, *this)); + const Object& result = Object::Handle(zone, EnsureHasCodeNoThrow()); if (result.IsError()) { if (result.ptr() == Object::out_of_memory_error().ptr()) { Exceptions::ThrowOOM(); @@ -11358,6 +11359,22 @@ CodePtr Function::EnsureHasCode() const { } Exceptions::PropagateError(Error::Cast(result)); UNREACHABLE(); + } else { + return Code::Cast(result).ptr(); + } +} + +ObjectPtr Function::EnsureHasCodeNoThrow() const { + if (HasCode()) { + return CurrentCode(); + } + Thread* thread = Thread::Current(); + ASSERT(thread->IsDartMutatorThread()); + Zone* zone = thread->zone(); + const Object& result = + Object::Handle(zone, Compiler::CompileFunction(thread, *this)); + if (result.IsError()) { + return result.ptr(); } // Compiling in unoptimized mode should never fail if there are no errors. RELEASE_ASSERT(HasCode()); diff --git a/runtime/vm/object.h b/runtime/vm/object.h index ac554ba6175..5429e7c5922 100644 --- a/runtime/vm/object.h +++ b/runtime/vm/object.h @@ -3166,6 +3166,12 @@ class Function : public Object { // current code, whether it is optimized or unoptimized. CodePtr EnsureHasCode() const; + // Ensures that the function has code. If there is no code, this method + // compiles the unoptimized version of the code. If an error occurs during + // compilation, the error is returned. Normally returns the function's code, + // whether optimized or unoptimized. + ObjectPtr EnsureHasCodeNoThrow() const; + // Disables optimized code and switches to unoptimized code (or the lazy // compilation stub). void SwitchToLazyCompiledUnoptimizedCode() const;