From 9c2a91fb7236214bcec37bba3214921256c5fa6a Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Thu, 23 Jun 2022 23:51:48 +0000 Subject: [PATCH] Account for @pragma("vm:entry-point") creating additional "root" libraries when partitioning the program into loading units. If a library contained an entry point but was not reachable from the root library, it was not assigned to any loading unit and caused a null dereference in gen_snapshot. This is not possible with the standalone embedder, but is possible in Flutter because it passes multiple sources to frontend_server. E.g., `--source dart_plugin_registrant.dart`. TEST=gallery Bug: https://github.com/flutter/gallery/issues/545 Change-Id: I9c67f0e39f7509094ee873610d80851a702a0cf2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249640 Reviewed-by: Alexander Markov Commit-Queue: Ryan Macnak --- pkg/vm/lib/kernel_front_end.dart | 2 +- .../lib/transformations/deferred_loading.dart | 60 +++++++++++++++++-- .../deferred_loading_test.dart | 5 +- .../frontend/kernel_translation_helper.cc | 10 ++++ 4 files changed, 69 insertions(+), 8 deletions(-) diff --git a/pkg/vm/lib/kernel_front_end.dart b/pkg/vm/lib/kernel_front_end.dart index 03ffe271b854..3daefd450180 100644 --- a/pkg/vm/lib/kernel_front_end.dart +++ b/pkg/vm/lib/kernel_front_end.dart @@ -509,7 +509,7 @@ Future runGlobalTransformations( // it does it will need the obfuscation prohibitions. obfuscationProhibitions.transformComponent(component, coreTypes, target); - deferred_loading.transformComponent(component); + deferred_loading.transformComponent(component, coreTypes, target); } /// Runs given [action] with [CompilerContext]. This is needed to diff --git a/pkg/vm/lib/transformations/deferred_loading.dart b/pkg/vm/lib/transformations/deferred_loading.dart index e58109645bc2..2280c37d3d75 100644 --- a/pkg/vm/lib/transformations/deferred_loading.dart +++ b/pkg/vm/lib/transformations/deferred_loading.dart @@ -5,8 +5,12 @@ library vm.transformations.deferred_loading; import 'package:kernel/ast.dart'; +import 'package:kernel/core_types.dart' show CoreTypes; +import 'package:kernel/target/targets.dart' show Target; + import '../dominators.dart'; import '../metadata/loading_units.dart'; +import 'pragma.dart'; class _LoadingUnitBuilder { late int id; @@ -37,7 +41,42 @@ class _LibraryVertex extends Vertex<_LibraryVertex> { String toString() => "_LibraryVertex(${library.importUri})"; } -List computeLoadingUnits(Component component) { +class HasEntryPointVisitor extends RecursiveVisitor { + final PragmaAnnotationParser parser; + bool _hasEntryPoint = false; + + HasEntryPointVisitor(this.parser); + + visitAnnotations(List annotations) { + for (var ann in annotations) { + ParsedPragma? pragma = parser.parsePragma(ann); + if (pragma is ParsedEntryPointPragma) { + _hasEntryPoint = true; + return; + } + } + } + + @override + visitClass(Class klass) { + visitAnnotations(klass.annotations); + klass.visitChildren(this); + } + + @override + defaultMember(Member node) { + visitAnnotations(node.annotations); + } + + bool hasEntryPoint(Library lib) { + _hasEntryPoint = false; + visitLibrary(lib); + return _hasEntryPoint; + } +} + +List computeLoadingUnits( + Component component, HasEntryPointVisitor visitor) { // 1. Build the dominator tree for the library import graph. final map = {}; for (final lib in component.libraries) { @@ -51,10 +90,15 @@ List computeLoadingUnits(Component component) { } final root = map[component.mainMethod!.enclosingLibrary]!; - // Fake imports from root library to every core library so they end up in - // the same loading unit attributed to the user's root library. + // Fake imports from root library to every core library or library containing + // an entry point pragma so that they end up in the same loading unit + // attributed to the user's root library. for (final vertex in map.values) { - if (vertex.library.importUri.isScheme("dart")) { + if (vertex == root) { + continue; + } + if (vertex.library.importUri.isScheme("dart") || + visitor.hasEntryPoint(vertex.library)) { root.successors.add(vertex); vertex.isLoadingRoot = false; } @@ -127,8 +171,12 @@ List computeLoadingUnits(Component component) { return loadingUnits.map((u) => u.asLoadingUnit()).toList(); } -Component transformComponent(Component component) { - final metadata = new LoadingUnitsMetadata(computeLoadingUnits(component)); +Component transformComponent( + Component component, CoreTypes coreTypes, Target target) { + final parser = ConstantPragmaAnnotationParser(coreTypes, target); + final visitor = HasEntryPointVisitor(parser); + final metadata = + new LoadingUnitsMetadata(computeLoadingUnits(component, visitor)); final repo = new LoadingUnitsMetadataRepository(); component.addMetadataRepository(repo); repo.mapping[component] = metadata; diff --git a/pkg/vm/test/transformations/deferred_loading_test.dart b/pkg/vm/test/transformations/deferred_loading_test.dart index 5b7cce83b765..61a2534f2e05 100644 --- a/pkg/vm/test/transformations/deferred_loading_test.dart +++ b/pkg/vm/test/transformations/deferred_loading_test.dart @@ -6,6 +6,7 @@ import 'dart:io'; import 'package:kernel/target/targets.dart'; import 'package:kernel/ast.dart'; +import 'package:kernel/core_types.dart'; import 'package:kernel/kernel.dart'; import 'package:test/test.dart'; import 'package:vm/transformations/deferred_loading.dart' @@ -24,7 +25,9 @@ runTestCase(Uri source) async { final reversed = component.libraries.reversed.toList(); component.libraries.setAll(0, reversed); - component = transformComponent(component); + final coreTypes = CoreTypes(component); + + component = transformComponent(component, coreTypes, target); // Remove core libraries so the expected output isn't enormous and broken by // core libraries changes. diff --git a/runtime/vm/compiler/frontend/kernel_translation_helper.cc b/runtime/vm/compiler/frontend/kernel_translation_helper.cc index b84479f57c59..5758e51fb56f 100644 --- a/runtime/vm/compiler/frontend/kernel_translation_helper.cc +++ b/runtime/vm/compiler/frontend/kernel_translation_helper.cc @@ -1930,6 +1930,16 @@ void LoadingUnitsMetadataHelper::ReadMetadata(intptr_t node_offset) { object_store->set_loading_units(loading_units); ASSERT(object_store->loading_unit_uris() == Array::null()); object_store->set_loading_unit_uris(loading_unit_uris); + + const GrowableObjectArray& libraries = + GrowableObjectArray::Handle(zone, object_store->libraries()); + for (intptr_t i = 0; i < libraries.Length(); i++) { + lib ^= libraries.At(i); + unit = lib.loading_unit(); + if (unit.IsNull()) { + FATAL("%s is not attributed to any loading unit", lib.ToCString()); + } + } } CallSiteAttributesMetadataHelper::CallSiteAttributesMetadataHelper(