From c13f7b0360b4e13e84e329a4d0a55a88e2441afb Mon Sep 17 00:00:00 2001 From: Emmanuel Pellereau Date: Fri, 24 Jun 2022 07:35:08 +0000 Subject: [PATCH] Revert "Account for @pragma("vm:entry-point") creating additional "root" libraries when partitioning the program into loading units." This reverts commit 9c2a91fb7236214bcec37bba3214921256c5fa6a. Reason for revert: breaks google3 (b/237016312) Original change's description: > 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 TBR=rmacnak@google.com,alexmarkov@google.com Change-Id: I3e17bf29b8f29e4797abfca35fa82b9ca3b5a160 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: https://github.com/flutter/gallery/issues/545 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249681 Reviewed-by: Daco Harkes Commit-Queue: Emmanuel Pellereau Reviewed-by: Emmanuel Pellereau --- 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, 8 insertions(+), 69 deletions(-) diff --git a/pkg/vm/lib/kernel_front_end.dart b/pkg/vm/lib/kernel_front_end.dart index 3daefd450180..03ffe271b854 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, coreTypes, target); + deferred_loading.transformComponent(component); } /// 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 2280c37d3d75..e58109645bc2 100644 --- a/pkg/vm/lib/transformations/deferred_loading.dart +++ b/pkg/vm/lib/transformations/deferred_loading.dart @@ -5,12 +5,8 @@ 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; @@ -41,42 +37,7 @@ class _LibraryVertex extends Vertex<_LibraryVertex> { String toString() => "_LibraryVertex(${library.importUri})"; } -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) { +List computeLoadingUnits(Component component) { // 1. Build the dominator tree for the library import graph. final map = {}; for (final lib in component.libraries) { @@ -90,15 +51,10 @@ List computeLoadingUnits( } final root = map[component.mainMethod!.enclosingLibrary]!; - // 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. + // 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. for (final vertex in map.values) { - if (vertex == root) { - continue; - } - if (vertex.library.importUri.isScheme("dart") || - visitor.hasEntryPoint(vertex.library)) { + if (vertex.library.importUri.isScheme("dart")) { root.successors.add(vertex); vertex.isLoadingRoot = false; } @@ -171,12 +127,8 @@ List computeLoadingUnits( return loadingUnits.map((u) => u.asLoadingUnit()).toList(); } -Component transformComponent( - Component component, CoreTypes coreTypes, Target target) { - final parser = ConstantPragmaAnnotationParser(coreTypes, target); - final visitor = HasEntryPointVisitor(parser); - final metadata = - new LoadingUnitsMetadata(computeLoadingUnits(component, visitor)); +Component transformComponent(Component component) { + final metadata = new LoadingUnitsMetadata(computeLoadingUnits(component)); 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 61a2534f2e05..5b7cce83b765 100644 --- a/pkg/vm/test/transformations/deferred_loading_test.dart +++ b/pkg/vm/test/transformations/deferred_loading_test.dart @@ -6,7 +6,6 @@ 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' @@ -25,9 +24,7 @@ runTestCase(Uri source) async { final reversed = component.libraries.reversed.toList(); component.libraries.setAll(0, reversed); - final coreTypes = CoreTypes(component); - - component = transformComponent(component, coreTypes, target); + component = transformComponent(component); // 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 5758e51fb56f..b84479f57c59 100644 --- a/runtime/vm/compiler/frontend/kernel_translation_helper.cc +++ b/runtime/vm/compiler/frontend/kernel_translation_helper.cc @@ -1930,16 +1930,6 @@ 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(