From 4e162c5318c5f82a5973af91aaaab73fabf6d538 Mon Sep 17 00:00:00 2001 From: Jens Johansen Date: Wed, 20 Dec 2017 13:02:12 +0000 Subject: [PATCH] [kernel] Fix vm crash when trying to break on default constructor Writing "break Foo" in observatory with a file including ``` class Foo { } ``` would crash them VM. This CL introduces a test that reproduces it and fixes the issue. Bug: Change-Id: I24780d9204117ade19a0cb590c45aa31b7c04f7e Reviewed-on: https://dart-review.googlesource.com/30445 Commit-Queue: Jens Johansen Reviewed-by: Vyacheslav Egorov Reviewed-by: Kevin Millikin --- .../break_on_default_constructor_test.dart | 76 +++++++++++++++++++ .../step_through_constructor_test.dart | 38 ++++++++++ .../frontend/kernel_binary_flowgraph.h | 2 + runtime/vm/debugger.cc | 4 + runtime/vm/kernel_loader.cc | 4 + 5 files changed, 124 insertions(+) create mode 100644 runtime/observatory/tests/service/break_on_default_constructor_test.dart create mode 100644 runtime/observatory/tests/service/step_through_constructor_test.dart diff --git a/runtime/observatory/tests/service/break_on_default_constructor_test.dart b/runtime/observatory/tests/service/break_on_default_constructor_test.dart new file mode 100644 index 00000000000..93901a8a6c2 --- /dev/null +++ b/runtime/observatory/tests/service/break_on_default_constructor_test.dart @@ -0,0 +1,76 @@ +// Copyright (c) 2017, 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:async'; + +import 'package:observatory/debugger.dart'; +import 'package:observatory/service_io.dart'; +import 'package:unittest/unittest.dart'; +import 'package:observatory/service.dart' as S; + +import 'service_test_common.dart'; +import 'test_helper.dart'; + +class Foo {} + +code() { + new Foo(); +} + +class TestDebugger extends Debugger { + TestDebugger(this.isolate, this.stack); + + VM get vm => isolate.vm; + Isolate isolate; + ServiceMap stack; + int currentFrame = 0; +} + +Future initDebugger(Isolate isolate) { + return isolate.getStack().then((stack) { + return new TestDebugger(isolate, stack); + }); +} + +List stops = []; + +var tests = [ + hasPausedAtStart, + // Load the isolate's libraries + (Isolate isolate) async { + for (var lib in isolate.libraries) { + await lib.load(); + } + }, + + (Isolate isolate) async { + var debugger = await initDebugger(isolate); + var loc = await DebuggerLocation.parse(debugger, 'Foo'); + + if (loc.valid) { + if (loc.function != null) { + try { + await debugger.isolate.addBreakpointAtEntry(loc.function); + } on S.ServerRpcException catch (e) { + if (e.code == S.ServerRpcException.kCannotAddBreakpoint) { + // Expected + } else { + fail("Got unexpected error $e"); + } + } + } else { + fail("Expected to find function"); + } + } else { + fail("Expected to find function"); + } + + isolate.resume(); + }, +]; + +main(args) { + runIsolateTestsSynchronous(args, tests, + testeeConcurrent: code, pause_on_start: true, pause_on_exit: true); +} diff --git a/runtime/observatory/tests/service/step_through_constructor_test.dart b/runtime/observatory/tests/service/step_through_constructor_test.dart new file mode 100644 index 00000000000..42fc5d6d291 --- /dev/null +++ b/runtime/observatory/tests/service/step_through_constructor_test.dart @@ -0,0 +1,38 @@ +// Copyright (c) 2017, 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 'test_helper.dart'; +import 'service_test_common.dart'; + +const int LINE = 17; +const String file = "step_through_constructor_test.dart"; + +code() { + new Foo(); +} + +class Foo { + Foo() { + print("Hello from Foo!"); + } +} + +List stops = []; +List expected = [ + "$file:${LINE+0}:5", // on 'print' + "$file:${LINE+1}:3", // on ending '}' +]; + +var tests = [ + hasPausedAtStart, + setBreakpointAtLine(LINE), + runStepIntoThroughProgramRecordingStops(stops), + checkRecordedStops(stops, expected, + debugPrint: true, debugPrintFile: file, debugPrintLine: LINE) +]; + +main(args) { + runIsolateTestsSynchronous(args, tests, + testeeConcurrent: code, pause_on_start: true, pause_on_exit: true); +} diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h index 3035c758a85..91962768f2a 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h @@ -322,6 +322,7 @@ class ConstructorHelper { enum Flag { kConst = 1 << 0, kExternal = 1 << 1, + kSyntheticDefault = 1 << 2, }; explicit ConstructorHelper(StreamingFlowGraphBuilder* builder) { @@ -340,6 +341,7 @@ class ConstructorHelper { bool IsExternal() { return (flags_ & kExternal) != 0; } bool IsConst() { return (flags_ & kConst) != 0; } + bool IsSyntheticDefault() { return (flags_ & kSyntheticDefault) != 0; } NameIndex canonical_name_; TokenPosition position_; diff --git a/runtime/vm/debugger.cc b/runtime/vm/debugger.cc index 6a868de509b..5c508f06819 100644 --- a/runtime/vm/debugger.cc +++ b/runtime/vm/debugger.cc @@ -2827,6 +2827,10 @@ Breakpoint* Debugger::SetBreakpointAtEntry(const Function& target_function, BreakpointLocation* bpt_location = SetBreakpoint( script, target_function.token_pos(), target_function.end_token_pos(), -1, -1 /* no requested line/col */); + if (bpt_location == NULL) { + return NULL; + } + if (single_shot) { return bpt_location->AddSingleShot(this); } else { diff --git a/runtime/vm/kernel_loader.cc b/runtime/vm/kernel_loader.cc index 50fa6892048..5d8388cc171 100644 --- a/runtime/vm/kernel_loader.cc +++ b/runtime/vm/kernel_loader.cc @@ -986,6 +986,10 @@ void KernelLoader::FinishClassLoading(const Class& klass, true, // is_method false, // is_closure &function_node_helper); + if (constructor_helper.IsSyntheticDefault()) { + function.set_is_debuggable(false); + } + function_node_helper.ReadUntilExcluding(FunctionNodeHelper::kEnd); constructor_helper.SetJustRead(ConstructorHelper::kFunction); constructor_helper.ReadUntilExcluding(ConstructorHelper::kEnd);