From 471b01de2e4e1ca731a3ef1097253cf4e6a29d7e Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Tue, 11 Aug 2020 20:42:29 +0000 Subject: [PATCH] [vm] Topologically sort loading units. The VM assumes parents appear before children when iterating loading units in id order. Normally this happens as a side-effect of sources being loaded in import order, but this might not happen in environments that combine separately produced kernel files. Bug: https://github.com/dart-lang/sdk/issues/42985 Change-Id: Ice9e2e07cf2aa89e91ab313951cc33c1924f17ed Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/158066 Commit-Queue: Ryan Macnak Reviewed-by: Alexander Markov --- .../lib/transformations/deferred_loading.dart | 29 ++- .../deferred_loading_test.dart | 4 + .../deferred_loading/main.dart.expect | 238 +++++++++--------- .../frontend/kernel_translation_helper.cc | 3 +- 4 files changed, 149 insertions(+), 125 deletions(-) diff --git a/pkg/vm/lib/transformations/deferred_loading.dart b/pkg/vm/lib/transformations/deferred_loading.dart index f2b03187299..03bb9c2ba21 100644 --- a/pkg/vm/lib/transformations/deferred_loading.dart +++ b/pkg/vm/lib/transformations/deferred_loading.dart @@ -9,14 +9,15 @@ import '../dominators.dart'; import '../metadata/loading_units.dart'; class _LoadingUnitBuilder { - final int id; + int id; final _LibraryVertex root; final List members = []; + final List<_LoadingUnitBuilder> children = <_LoadingUnitBuilder>[]; - _LoadingUnitBuilder(this.id, this.root); + _LoadingUnitBuilder(this.root); - int get parentId => - root.dominator == null ? 0 : root.dominator.loadingUnit.id; + _LoadingUnitBuilder get parent => root.dominator?.loadingUnit; + int get parentId => parent == null ? 0 : parent.id; LoadingUnit asLoadingUnit() { return new LoadingUnit( @@ -82,7 +83,7 @@ List computeLoadingUnits(Component component) { var loadingUnits = <_LoadingUnitBuilder>[]; for (var vertex in map.values) { if (vertex.isLoadingRoot) { - var unit = new _LoadingUnitBuilder(loadingUnits.length + 1, vertex); + var unit = new _LoadingUnitBuilder(vertex); vertex.loadingUnit = unit; unit.members.add(vertex.library); loadingUnits.add(unit); @@ -105,6 +106,24 @@ List computeLoadingUnits(Component component) { vertex.loadingUnit.members.add(vertex.library); } + // 4. Sort loading units so parents are before children. Normally this order + // would already exist as a side effect of loading sources in import order, + // but this isn't guarenteed when combining separately produced kernel files. + for (var unit in loadingUnits) { + var parent = unit.parent; + if (parent != null) { + parent.children.add(unit); + } + } + var index = 0; + loadingUnits.clear(); + loadingUnits.add(root.loadingUnit); + while (index < loadingUnits.length) { + var unit = loadingUnits[index]; + unit.id = ++index; + loadingUnits.addAll(unit.children); + } + return loadingUnits.map((u) => u.asLoadingUnit()).toList(); } diff --git a/pkg/vm/test/transformations/deferred_loading_test.dart b/pkg/vm/test/transformations/deferred_loading_test.dart index 4b5f96195eb..f36b610e790 100644 --- a/pkg/vm/test/transformations/deferred_loading_test.dart +++ b/pkg/vm/test/transformations/deferred_loading_test.dart @@ -20,6 +20,10 @@ runTestCase(Uri source) async { Component component = await compileTestCaseToKernelProgram(source, target: target); + // Disrupt the import order as a way of simulating issue 42985. + final reversed = component.libraries.reversed.toList(); + component.libraries.setAll(0, reversed); + component = transformComponent(component); // Remove core libraries so the expected output isn't enormous and broken by diff --git a/pkg/vm/testcases/transformations/deferred_loading/main.dart.expect b/pkg/vm/testcases/transformations/deferred_loading/main.dart.expect index 3dbc78e243a..f6484afa823 100644 --- a/pkg/vm/testcases/transformations/deferred_loading/main.dart.expect +++ b/pkg/vm/testcases/transformations/deferred_loading/main.dart.expect @@ -2,39 +2,39 @@ main = #lib::main; [@vm.loading-units.metadata=LoadingUnitsMetadata( LoadingUnit(id=1, parent=0, #lib - #pkg/vm/testcases/transformations/deferred_loading/a.dart - #pkg/vm/testcases/transformations/deferred_loading/b.dart - #pkg/vm/testcases/transformations/deferred_loading/c.dart - dart:async - dart:collection - dart:convert - dart:developer - dart:ffi - dart:_internal - dart:isolate - dart:math - dart:mirrors - dart:typed_data - dart:vmservice_io - dart:_vmservice - dart:_builtin - dart:nativewrappers - dart:io - dart:cli - dart:wasm - dart:core dart:_http + dart:core + dart:wasm + dart:cli + dart:io + dart:nativewrappers + dart:_builtin + dart:_vmservice + dart:vmservice_io + dart:typed_data + dart:mirrors + dart:math + dart:isolate + dart:_internal + dart:ffi + dart:developer + dart:convert + dart:collection + dart:async + #pkg/vm/testcases/transformations/deferred_loading/c.dart + #pkg/vm/testcases/transformations/deferred_loading/b.dart + #pkg/vm/testcases/transformations/deferred_loading/a.dart ) LoadingUnit(id=2, parent=1, - #pkg/vm/testcases/transformations/deferred_loading/d.dart - #pkg/vm/testcases/transformations/deferred_loading/e.dart + #pkg/vm/testcases/transformations/deferred_loading/g.dart + #pkg/vm/testcases/transformations/deferred_loading/h.dart ) LoadingUnit(id=3, parent=1, #pkg/vm/testcases/transformations/deferred_loading/f.dart ) LoadingUnit(id=4, parent=1, - #pkg/vm/testcases/transformations/deferred_loading/g.dart - #pkg/vm/testcases/transformations/deferred_loading/h.dart + #pkg/vm/testcases/transformations/deferred_loading/d.dart + #pkg/vm/testcases/transformations/deferred_loading/e.dart ) LoadingUnit(id=5, parent=3, #pkg/vm/testcases/transformations/deferred_loading/i.dart @@ -42,12 +42,11 @@ LoadingUnit(id=5, parent=3, LoadingUnit(id=6, parent=5, #pkg/vm/testcases/transformations/deferred_loading/j.dart ) -)]library #lib from "#lib" as #lib { +)]library j from "#pkg/vm/testcases/transformations/deferred_loading/j.dart" as j { - import "#pkg/vm/testcases/transformations/deferred_loading/a.dart"; - import "#pkg/vm/testcases/transformations/deferred_loading/b.dart"; + import "#pkg/vm/testcases/transformations/deferred_loading/a.dart" as a; - static method main() → dynamic /* originally async */ { + static method j() → dynamic /* originally async */ { final dart.async::_AsyncAwaitCompleter* :async_completer = new dart.async::_AsyncAwaitCompleter::•(); FutureOr* :return_value; dynamic :async_stack_trace; @@ -55,15 +54,11 @@ LoadingUnit(id=6, parent=5, (dart.core::Object*, dart.core::StackTrace*) →* dynamic :async_op_error; dart.core::int* :await_jump_var = 0; dynamic :await_ctx_var; - dynamic :saved_try_context_var0; function :async_op([dynamic :result, dynamic :exception, dynamic :stack_trace]) → dynamic yielding try { #L1: { - [yield] let dynamic #t1 = dart.async::_awaitHelper(a::a(), :async_op_then, :async_op_error, :async_op) in null; - :result; - [yield] let dynamic #t2 = dart.async::_awaitHelper(b::b(), :async_op_then, :async_op_error, :async_op) in null; - :result; + dart.core::print("J"); } dart.async::_completeOnAsyncReturn(:async_completer, :return_value); return; @@ -78,11 +73,43 @@ LoadingUnit(id=6, parent=5, return :async_completer.{dart.async::Completer::future}; } } -library a from "#pkg/vm/testcases/transformations/deferred_loading/a.dart" as a { +library h from "#pkg/vm/testcases/transformations/deferred_loading/h.dart" as h { - import "#pkg/vm/testcases/transformations/deferred_loading/d.dart" deferred as d; + import "#pkg/vm/testcases/transformations/deferred_loading/g.dart" as g; - static method a() → dynamic /* originally async */ { + static method h() → dynamic /* originally async */ { + final dart.async::_AsyncAwaitCompleter* :async_completer = new dart.async::_AsyncAwaitCompleter::•(); + FutureOr* :return_value; + dynamic :async_stack_trace; + (dynamic) →* dynamic :async_op_then; + (dart.core::Object*, dart.core::StackTrace*) →* dynamic :async_op_error; + dart.core::int* :await_jump_var = 0; + dynamic :await_ctx_var; + function :async_op([dynamic :result, dynamic :exception, dynamic :stack_trace]) → dynamic yielding + try { + #L2: + { + dart.core::print("H"); + } + dart.async::_completeOnAsyncReturn(:async_completer, :return_value); + return; + } + on dynamic catch(dynamic exception, dart.core::StackTrace* stack_trace) { + :async_completer.{dart.async::Completer::completeError}(exception, stack_trace); + } + :async_stack_trace = dart.async::_asyncStackTraceHelper(:async_op); + :async_op_then = dart.async::_asyncThenWrapperHelper(:async_op); + :async_op_error = dart.async::_asyncErrorWrapperHelper(:async_op); + :async_completer.{dart.async::_AsyncAwaitCompleter::start}(:async_op); + return :async_completer.{dart.async::Completer::future}; + } +} +library i from "#pkg/vm/testcases/transformations/deferred_loading/i.dart" as i { + + import "#pkg/vm/testcases/transformations/deferred_loading/j.dart" deferred as j; + import "#pkg/vm/testcases/transformations/deferred_loading/b.dart" as b; + + static method i() → dynamic /* originally async */ { final dart.async::_AsyncAwaitCompleter* :async_completer = new dart.async::_AsyncAwaitCompleter::•(); FutureOr* :return_value; dynamic :async_stack_trace; @@ -91,47 +118,14 @@ library a from "#pkg/vm/testcases/transformations/deferred_loading/a.dart" as a dart.core::int* :await_jump_var = 0; dynamic :await_ctx_var; dynamic :saved_try_context_var0; - function :async_op([dynamic :result, dynamic :exception, dynamic :stack_trace]) → dynamic yielding - try { - #L2: - { - dart.core::print("A"); - [yield] let dynamic #t3 = dart.async::_awaitHelper(LoadLibrary(d), :async_op_then, :async_op_error, :async_op) in null; - :result; - :return_value = let final dynamic #t4 = CheckLibraryIsLoaded(d) in d::d(); - break #L2; - } - dart.async::_completeOnAsyncReturn(:async_completer, :return_value); - return; - } - on dynamic catch(dynamic exception, dart.core::StackTrace* stack_trace) { - :async_completer.{dart.async::Completer::completeError}(exception, stack_trace); - } - :async_stack_trace = dart.async::_asyncStackTraceHelper(:async_op); - :async_op_then = dart.async::_asyncThenWrapperHelper(:async_op); - :async_op_error = dart.async::_asyncErrorWrapperHelper(:async_op); - :async_completer.{dart.async::_AsyncAwaitCompleter::start}(:async_op); - return :async_completer.{dart.async::Completer::future}; - } -} -library b from "#pkg/vm/testcases/transformations/deferred_loading/b.dart" as b { - - import "#pkg/vm/testcases/transformations/deferred_loading/c.dart" as c; - - static method b() → dynamic /* originally async */ { - final dart.async::_AsyncAwaitCompleter* :async_completer = new dart.async::_AsyncAwaitCompleter::•(); - FutureOr* :return_value; - dynamic :async_stack_trace; - (dynamic) →* dynamic :async_op_then; - (dart.core::Object*, dart.core::StackTrace*) →* dynamic :async_op_error; - dart.core::int* :await_jump_var = 0; - dynamic :await_ctx_var; function :async_op([dynamic :result, dynamic :exception, dynamic :stack_trace]) → dynamic yielding try { #L3: { - dart.core::print("B"); - :return_value = c::c(); + dart.core::print("I"); + [yield] let dynamic #t1 = dart.async::_awaitHelper(LoadLibrary(j), :async_op_then, :async_op_error, :async_op) in null; + :result; + :return_value = let final dynamic #t2 = CheckLibraryIsLoaded(j) in j::j(); break #L3; } dart.async::_completeOnAsyncReturn(:async_completer, :return_value); @@ -147,11 +141,11 @@ library b from "#pkg/vm/testcases/transformations/deferred_loading/b.dart" as b return :async_completer.{dart.async::Completer::future}; } } -library d from "#pkg/vm/testcases/transformations/deferred_loading/d.dart" as d { +library g from "#pkg/vm/testcases/transformations/deferred_loading/g.dart" as g { - import "#pkg/vm/testcases/transformations/deferred_loading/e.dart" as e; + import "#pkg/vm/testcases/transformations/deferred_loading/h.dart" as h; - static method d() → dynamic /* originally async */ { + static method g() → dynamic /* originally async */ { final dart.async::_AsyncAwaitCompleter* :async_completer = new dart.async::_AsyncAwaitCompleter::•(); FutureOr* :return_value; dynamic :async_stack_trace; @@ -163,9 +157,7 @@ library d from "#pkg/vm/testcases/transformations/deferred_loading/d.dart" as d try { #L4: { - dart.core::print("D"); - :return_value = e::e(); - break #L4; + dart.core::print("G"); } dart.async::_completeOnAsyncReturn(:async_completer, :return_value); return; @@ -180,12 +172,12 @@ library d from "#pkg/vm/testcases/transformations/deferred_loading/d.dart" as d return :async_completer.{dart.async::Completer::future}; } } -library c from "#pkg/vm/testcases/transformations/deferred_loading/c.dart" as c { +library f from "#pkg/vm/testcases/transformations/deferred_loading/f.dart" as f { - import "#pkg/vm/testcases/transformations/deferred_loading/b.dart" as b; - import "#pkg/vm/testcases/transformations/deferred_loading/f.dart" deferred as f; + import "#pkg/vm/testcases/transformations/deferred_loading/g.dart" deferred as g; + import "#pkg/vm/testcases/transformations/deferred_loading/i.dart" deferred as i; - static method c() → dynamic /* originally async */ { + static method f() → dynamic /* originally async */ { final dart.async::_AsyncAwaitCompleter* :async_completer = new dart.async::_AsyncAwaitCompleter::•(); FutureOr* :return_value; dynamic :async_stack_trace; @@ -198,10 +190,14 @@ library c from "#pkg/vm/testcases/transformations/deferred_loading/c.dart" as c try { #L5: { - dart.core::print("C"); - [yield] let dynamic #t5 = dart.async::_awaitHelper(LoadLibrary(f), :async_op_then, :async_op_error, :async_op) in null; + dart.core::print("F"); + [yield] let dynamic #t3 = dart.async::_awaitHelper(LoadLibrary(g), :async_op_then, :async_op_error, :async_op) in null; :result; - :return_value = let final dynamic #t6 = CheckLibraryIsLoaded(f) in f::f(); + :return_value = let final dynamic #t4 = CheckLibraryIsLoaded(g) in g::g(); + break #L5; + [yield] let dynamic #t5 = dart.async::_awaitHelper(LoadLibrary(i), :async_op_then, :async_op_error, :async_op) in null; + :result; + :return_value = let final dynamic #t6 = CheckLibraryIsLoaded(i) in i::i(); break #L5; } dart.async::_completeOnAsyncReturn(:async_completer, :return_value); @@ -253,12 +249,12 @@ library e from "#pkg/vm/testcases/transformations/deferred_loading/e.dart" as e return :async_completer.{dart.async::Completer::future}; } } -library f from "#pkg/vm/testcases/transformations/deferred_loading/f.dart" as f { +library c from "#pkg/vm/testcases/transformations/deferred_loading/c.dart" as c { - import "#pkg/vm/testcases/transformations/deferred_loading/g.dart" deferred as g; - import "#pkg/vm/testcases/transformations/deferred_loading/i.dart" deferred as i; + import "#pkg/vm/testcases/transformations/deferred_loading/b.dart" as b; + import "#pkg/vm/testcases/transformations/deferred_loading/f.dart" deferred as f; - static method f() → dynamic /* originally async */ { + static method c() → dynamic /* originally async */ { final dart.async::_AsyncAwaitCompleter* :async_completer = new dart.async::_AsyncAwaitCompleter::•(); FutureOr* :return_value; dynamic :async_stack_trace; @@ -271,14 +267,10 @@ library f from "#pkg/vm/testcases/transformations/deferred_loading/f.dart" as f try { #L7: { - dart.core::print("F"); - [yield] let dynamic #t9 = dart.async::_awaitHelper(LoadLibrary(g), :async_op_then, :async_op_error, :async_op) in null; + dart.core::print("C"); + [yield] let dynamic #t9 = dart.async::_awaitHelper(LoadLibrary(f), :async_op_then, :async_op_error, :async_op) in null; :result; - :return_value = let final dynamic #t10 = CheckLibraryIsLoaded(g) in g::g(); - break #L7; - [yield] let dynamic #t11 = dart.async::_awaitHelper(LoadLibrary(i), :async_op_then, :async_op_error, :async_op) in null; - :result; - :return_value = let final dynamic #t12 = CheckLibraryIsLoaded(i) in i::i(); + :return_value = let final dynamic #t10 = CheckLibraryIsLoaded(f) in f::f(); break #L7; } dart.async::_completeOnAsyncReturn(:async_completer, :return_value); @@ -294,11 +286,11 @@ library f from "#pkg/vm/testcases/transformations/deferred_loading/f.dart" as f return :async_completer.{dart.async::Completer::future}; } } -library g from "#pkg/vm/testcases/transformations/deferred_loading/g.dart" as g { +library d from "#pkg/vm/testcases/transformations/deferred_loading/d.dart" as d { - import "#pkg/vm/testcases/transformations/deferred_loading/h.dart" as h; + import "#pkg/vm/testcases/transformations/deferred_loading/e.dart" as e; - static method g() → dynamic /* originally async */ { + static method d() → dynamic /* originally async */ { final dart.async::_AsyncAwaitCompleter* :async_completer = new dart.async::_AsyncAwaitCompleter::•(); FutureOr* :return_value; dynamic :async_stack_trace; @@ -310,7 +302,9 @@ library g from "#pkg/vm/testcases/transformations/deferred_loading/g.dart" as g try { #L8: { - dart.core::print("G"); + dart.core::print("D"); + :return_value = e::e(); + break #L8; } dart.async::_completeOnAsyncReturn(:async_completer, :return_value); return; @@ -325,12 +319,11 @@ library g from "#pkg/vm/testcases/transformations/deferred_loading/g.dart" as g return :async_completer.{dart.async::Completer::future}; } } -library i from "#pkg/vm/testcases/transformations/deferred_loading/i.dart" as i { +library b from "#pkg/vm/testcases/transformations/deferred_loading/b.dart" as b { - import "#pkg/vm/testcases/transformations/deferred_loading/j.dart" deferred as j; - import "#pkg/vm/testcases/transformations/deferred_loading/b.dart" as b; + import "#pkg/vm/testcases/transformations/deferred_loading/c.dart" as c; - static method i() → dynamic /* originally async */ { + static method b() → dynamic /* originally async */ { final dart.async::_AsyncAwaitCompleter* :async_completer = new dart.async::_AsyncAwaitCompleter::•(); FutureOr* :return_value; dynamic :async_stack_trace; @@ -338,15 +331,12 @@ library i from "#pkg/vm/testcases/transformations/deferred_loading/i.dart" as i (dart.core::Object*, dart.core::StackTrace*) →* dynamic :async_op_error; dart.core::int* :await_jump_var = 0; dynamic :await_ctx_var; - dynamic :saved_try_context_var0; function :async_op([dynamic :result, dynamic :exception, dynamic :stack_trace]) → dynamic yielding try { #L9: { - dart.core::print("I"); - [yield] let dynamic #t13 = dart.async::_awaitHelper(LoadLibrary(j), :async_op_then, :async_op_error, :async_op) in null; - :result; - :return_value = let final dynamic #t14 = CheckLibraryIsLoaded(j) in j::j(); + dart.core::print("B"); + :return_value = c::c(); break #L9; } dart.async::_completeOnAsyncReturn(:async_completer, :return_value); @@ -362,11 +352,11 @@ library i from "#pkg/vm/testcases/transformations/deferred_loading/i.dart" as i return :async_completer.{dart.async::Completer::future}; } } -library h from "#pkg/vm/testcases/transformations/deferred_loading/h.dart" as h { +library a from "#pkg/vm/testcases/transformations/deferred_loading/a.dart" as a { - import "#pkg/vm/testcases/transformations/deferred_loading/g.dart" as g; + import "#pkg/vm/testcases/transformations/deferred_loading/d.dart" deferred as d; - static method h() → dynamic /* originally async */ { + static method a() → dynamic /* originally async */ { final dart.async::_AsyncAwaitCompleter* :async_completer = new dart.async::_AsyncAwaitCompleter::•(); FutureOr* :return_value; dynamic :async_stack_trace; @@ -374,11 +364,16 @@ library h from "#pkg/vm/testcases/transformations/deferred_loading/h.dart" as h (dart.core::Object*, dart.core::StackTrace*) →* dynamic :async_op_error; dart.core::int* :await_jump_var = 0; dynamic :await_ctx_var; + dynamic :saved_try_context_var0; function :async_op([dynamic :result, dynamic :exception, dynamic :stack_trace]) → dynamic yielding try { #L10: { - dart.core::print("H"); + dart.core::print("A"); + [yield] let dynamic #t11 = dart.async::_awaitHelper(LoadLibrary(d), :async_op_then, :async_op_error, :async_op) in null; + :result; + :return_value = let final dynamic #t12 = CheckLibraryIsLoaded(d) in d::d(); + break #L10; } dart.async::_completeOnAsyncReturn(:async_completer, :return_value); return; @@ -393,11 +388,12 @@ library h from "#pkg/vm/testcases/transformations/deferred_loading/h.dart" as h return :async_completer.{dart.async::Completer::future}; } } -library j from "#pkg/vm/testcases/transformations/deferred_loading/j.dart" as j { +library #lib from "#lib" as #lib { - import "#pkg/vm/testcases/transformations/deferred_loading/a.dart" as a; + import "#pkg/vm/testcases/transformations/deferred_loading/a.dart"; + import "#pkg/vm/testcases/transformations/deferred_loading/b.dart"; - static method j() → dynamic /* originally async */ { + static method main() → dynamic /* originally async */ { final dart.async::_AsyncAwaitCompleter* :async_completer = new dart.async::_AsyncAwaitCompleter::•(); FutureOr* :return_value; dynamic :async_stack_trace; @@ -405,11 +401,15 @@ library j from "#pkg/vm/testcases/transformations/deferred_loading/j.dart" as j (dart.core::Object*, dart.core::StackTrace*) →* dynamic :async_op_error; dart.core::int* :await_jump_var = 0; dynamic :await_ctx_var; + dynamic :saved_try_context_var0; function :async_op([dynamic :result, dynamic :exception, dynamic :stack_trace]) → dynamic yielding try { #L11: { - dart.core::print("J"); + [yield] let dynamic #t13 = dart.async::_awaitHelper(a::a(), :async_op_then, :async_op_error, :async_op) in null; + :result; + [yield] let dynamic #t14 = dart.async::_awaitHelper(b::b(), :async_op_then, :async_op_error, :async_op) in null; + :result; } dart.async::_completeOnAsyncReturn(:async_completer, :return_value); return; diff --git a/runtime/vm/compiler/frontend/kernel_translation_helper.cc b/runtime/vm/compiler/frontend/kernel_translation_helper.cc index a1ea46f0a84..718c6df31a0 100644 --- a/runtime/vm/compiler/frontend/kernel_translation_helper.cc +++ b/runtime/vm/compiler/frontend/kernel_translation_helper.cc @@ -1829,8 +1829,9 @@ void LoadingUnitsMetadataHelper::ReadMetadata(intptr_t node_offset) { unit.set_id(id); intptr_t parent_id = helper_->ReadUInt(); + RELEASE_ASSERT(parent_id < id); parent ^= loading_units.At(parent_id); - ASSERT(parent.IsNull() == (parent_id == 0)); + RELEASE_ASSERT(parent.IsNull() == (parent_id == 0)); unit.set_parent(parent); intptr_t library_count = helper_->ReadUInt();