Skip to content

Commit 9c2a91f

Browse files
rmacnak-googleCommit Bot
authored and
Commit Bot
committed
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: flutter/gallery#545 Change-Id: I9c67f0e39f7509094ee873610d80851a702a0cf2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249640 Reviewed-by: Alexander Markov <[email protected]> Commit-Queue: Ryan Macnak <[email protected]>
1 parent 9395116 commit 9c2a91f

File tree

4 files changed

+69
-8
lines changed

4 files changed

+69
-8
lines changed

pkg/vm/lib/kernel_front_end.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ Future runGlobalTransformations(
509509
// it does it will need the obfuscation prohibitions.
510510
obfuscationProhibitions.transformComponent(component, coreTypes, target);
511511

512-
deferred_loading.transformComponent(component);
512+
deferred_loading.transformComponent(component, coreTypes, target);
513513
}
514514

515515
/// Runs given [action] with [CompilerContext]. This is needed to

pkg/vm/lib/transformations/deferred_loading.dart

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,12 @@
55
library vm.transformations.deferred_loading;
66

77
import 'package:kernel/ast.dart';
8+
import 'package:kernel/core_types.dart' show CoreTypes;
9+
import 'package:kernel/target/targets.dart' show Target;
10+
811
import '../dominators.dart';
912
import '../metadata/loading_units.dart';
13+
import 'pragma.dart';
1014

1115
class _LoadingUnitBuilder {
1216
late int id;
@@ -37,7 +41,42 @@ class _LibraryVertex extends Vertex<_LibraryVertex> {
3741
String toString() => "_LibraryVertex(${library.importUri})";
3842
}
3943

40-
List<LoadingUnit> computeLoadingUnits(Component component) {
44+
class HasEntryPointVisitor extends RecursiveVisitor {
45+
final PragmaAnnotationParser parser;
46+
bool _hasEntryPoint = false;
47+
48+
HasEntryPointVisitor(this.parser);
49+
50+
visitAnnotations(List<Expression> annotations) {
51+
for (var ann in annotations) {
52+
ParsedPragma? pragma = parser.parsePragma(ann);
53+
if (pragma is ParsedEntryPointPragma) {
54+
_hasEntryPoint = true;
55+
return;
56+
}
57+
}
58+
}
59+
60+
@override
61+
visitClass(Class klass) {
62+
visitAnnotations(klass.annotations);
63+
klass.visitChildren(this);
64+
}
65+
66+
@override
67+
defaultMember(Member node) {
68+
visitAnnotations(node.annotations);
69+
}
70+
71+
bool hasEntryPoint(Library lib) {
72+
_hasEntryPoint = false;
73+
visitLibrary(lib);
74+
return _hasEntryPoint;
75+
}
76+
}
77+
78+
List<LoadingUnit> computeLoadingUnits(
79+
Component component, HasEntryPointVisitor visitor) {
4180
// 1. Build the dominator tree for the library import graph.
4281
final map = <Library, _LibraryVertex>{};
4382
for (final lib in component.libraries) {
@@ -51,10 +90,15 @@ List<LoadingUnit> computeLoadingUnits(Component component) {
5190
}
5291
final root = map[component.mainMethod!.enclosingLibrary]!;
5392

54-
// Fake imports from root library to every core library so they end up in
55-
// the same loading unit attributed to the user's root library.
93+
// Fake imports from root library to every core library or library containing
94+
// an entry point pragma so that they end up in the same loading unit
95+
// attributed to the user's root library.
5696
for (final vertex in map.values) {
57-
if (vertex.library.importUri.isScheme("dart")) {
97+
if (vertex == root) {
98+
continue;
99+
}
100+
if (vertex.library.importUri.isScheme("dart") ||
101+
visitor.hasEntryPoint(vertex.library)) {
58102
root.successors.add(vertex);
59103
vertex.isLoadingRoot = false;
60104
}
@@ -127,8 +171,12 @@ List<LoadingUnit> computeLoadingUnits(Component component) {
127171
return loadingUnits.map((u) => u.asLoadingUnit()).toList();
128172
}
129173

130-
Component transformComponent(Component component) {
131-
final metadata = new LoadingUnitsMetadata(computeLoadingUnits(component));
174+
Component transformComponent(
175+
Component component, CoreTypes coreTypes, Target target) {
176+
final parser = ConstantPragmaAnnotationParser(coreTypes, target);
177+
final visitor = HasEntryPointVisitor(parser);
178+
final metadata =
179+
new LoadingUnitsMetadata(computeLoadingUnits(component, visitor));
132180
final repo = new LoadingUnitsMetadataRepository();
133181
component.addMetadataRepository(repo);
134182
repo.mapping[component] = metadata;

pkg/vm/test/transformations/deferred_loading_test.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'dart:io';
66

77
import 'package:kernel/target/targets.dart';
88
import 'package:kernel/ast.dart';
9+
import 'package:kernel/core_types.dart';
910
import 'package:kernel/kernel.dart';
1011
import 'package:test/test.dart';
1112
import 'package:vm/transformations/deferred_loading.dart'
@@ -24,7 +25,9 @@ runTestCase(Uri source) async {
2425
final reversed = component.libraries.reversed.toList();
2526
component.libraries.setAll(0, reversed);
2627

27-
component = transformComponent(component);
28+
final coreTypes = CoreTypes(component);
29+
30+
component = transformComponent(component, coreTypes, target);
2831

2932
// Remove core libraries so the expected output isn't enormous and broken by
3033
// core libraries changes.

runtime/vm/compiler/frontend/kernel_translation_helper.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1930,6 +1930,16 @@ void LoadingUnitsMetadataHelper::ReadMetadata(intptr_t node_offset) {
19301930
object_store->set_loading_units(loading_units);
19311931
ASSERT(object_store->loading_unit_uris() == Array::null());
19321932
object_store->set_loading_unit_uris(loading_unit_uris);
1933+
1934+
const GrowableObjectArray& libraries =
1935+
GrowableObjectArray::Handle(zone, object_store->libraries());
1936+
for (intptr_t i = 0; i < libraries.Length(); i++) {
1937+
lib ^= libraries.At(i);
1938+
unit = lib.loading_unit();
1939+
if (unit.IsNull()) {
1940+
FATAL("%s is not attributed to any loading unit", lib.ToCString());
1941+
}
1942+
}
19331943
}
19341944

19351945
CallSiteAttributesMetadataHelper::CallSiteAttributesMetadataHelper(

0 commit comments

Comments
 (0)