Add option to check if a dill is self-contained

This CL adds an option to the dill loader (i.e. BinaryBuilder) to
enable performing a check of all canonical names. The theory is,
that if a dill is self-contained "every" (1) canonical name should
contain a reference to a node.

This check is (currently) disabled by default, but enabled
excplicitly in the incremental compiler when initializing from dill.
If the check fails a warning will be issued (and the dill wont be
used).

On my machine the check seems to take ~15 ms in all tested cases
(sdk, flutter test).

(1): Except for extra layer of URI of a library if the qualified
name is private.

Closes #32449.

Change-Id: I65c8fba647be3d5ba47f7ee16caef78cb2cfae07
Reviewed-on: https://dart-review.googlesource.com/51462
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>

pkg/front_end/lib/src/fasta/fasta_codes_generated.dart

@@ -2832,6 +2832,60 @@ const MessageCode messageInheritedMembersConflictCause2 = const MessageCode(
     severity: Severity.context,
     message: r"""This is the other inherited member.""");
 
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Template<
+    Message Function(
+        String
+            string)> templateInitializeFromDillNotSelfContained = const Template<
+        Message Function(String string)>(
+    messageTemplate:
+        r"""Tried to initialize from a previous compilation (#string), but the file was not self-contained. This might be a bug, please report at http://dartbug.com/new.""",
+    withArguments: _withArgumentsInitializeFromDillNotSelfContained);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Message Function(String string)>
+    codeInitializeFromDillNotSelfContained =
+    const Code<Message Function(String string)>(
+        "InitializeFromDillNotSelfContained",
+        templateInitializeFromDillNotSelfContained,
+        severity: Severity.warning);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+Message _withArgumentsInitializeFromDillNotSelfContained(String string) {
+  return new Message(codeInitializeFromDillNotSelfContained,
+      message:
+          """Tried to initialize from a previous compilation (${string}), but the file was not self-contained. This might be a bug, please report at http://dartbug.com/new.""",
+      arguments: {'string': string});
+}
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Template<
+    Message Function(
+        String string,
+        String
+            string2)> templateInitializeFromDillUnknownProblem = const Template<
+        Message Function(String string, String string2)>(
+    messageTemplate:
+        r"""Tried to initialize from a previous compilation (#string), but couldn't. This might be a bug, please report at http://dartbug.com/new. Error message was '#string2'.""",
+    withArguments: _withArgumentsInitializeFromDillUnknownProblem);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Message Function(String string, String string2)>
+    codeInitializeFromDillUnknownProblem =
+    const Code<Message Function(String string, String string2)>(
+        "InitializeFromDillUnknownProblem",
+        templateInitializeFromDillUnknownProblem,
+        severity: Severity.warning);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+Message _withArgumentsInitializeFromDillUnknownProblem(
+    String string, String string2) {
+  return new Message(codeInitializeFromDillUnknownProblem,
+      message:
+          """Tried to initialize from a previous compilation (${string}), but couldn't. This might be a bug, please report at http://dartbug.com/new. Error message was '${string2}'.""",
+      arguments: {'string': string, 'string2': string2});
+}
+
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Code<Null> codeInitializedVariableInForEach =
     messageInitializedVariableInForEach;

pkg/front_end/lib/src/fasta/incremental_compiler.dart

@@ -8,62 +8,72 @@ import 'dart:async' show Future;
 
 import 'package:kernel/binary/ast_from_binary.dart' show BinaryBuilder;
 
-import 'package:kernel/class_hierarchy.dart' show ClassHierarchy;
+import 'package:kernel/binary/ast_from_binary.dart'
+    show BinaryBuilder, CanonicalNameError, InvalidKernelVersionError;
 
-import '../api_prototype/file_system.dart' show FileSystemEntity;
+import 'package:kernel/class_hierarchy.dart' show ClassHierarchy;
 
 import 'package:kernel/kernel.dart'
     show
-        Library,
-        Name,
-        ReturnStatement,
-        FunctionNode,
         Class,
-        Expression,
-        DartType,
-        LibraryPart,
         Component,
+        DartType,
+        Expression,
+        FunctionNode,
+        Library,
         LibraryDependency,
-        Source,
+        LibraryPart,
+        Name,
         Procedure,
-        TypeParameter,
-        ProcedureKind;
-
-import '../api_prototype/memory_file_system.dart' show MemoryFileSystem;
-
-import 'hybrid_file_system.dart' show HybridFileSystem;
+        ProcedureKind,
+        ReturnStatement,
+        Source,
+        TreeNode,
+        TypeParameter;
 
 import 'package:kernel/kernel.dart' as kernel show Combinator;
 
+import '../api_prototype/file_system.dart' show FileSystemEntity;
+
 import '../api_prototype/incremental_kernel_generator.dart'
     show IncrementalKernelGenerator, isLegalIdentifier;
 
+import '../api_prototype/memory_file_system.dart' show MemoryFileSystem;
+
 import 'builder/builder.dart' show LibraryBuilder;
 
 import 'builder_graph.dart' show BuilderGraph;
 
+import 'combinator.dart' show Combinator;
+
 import 'compiler_context.dart' show CompilerContext;
 
 import 'dill/dill_library_builder.dart' show DillLibraryBuilder;
 
 import 'dill/dill_target.dart' show DillTarget;
 
+import 'fasta_codes.dart'
+    show
+        templateInitializeFromDillNotSelfContained,
+        templateInitializeFromDillUnknownProblem;
+
+import 'hybrid_file_system.dart' show HybridFileSystem;
+
 import 'kernel/kernel_incremental_target.dart'
     show KernelIncrementalTarget, KernelIncrementalTargetErroneousComponent;
 
-import 'library_graph.dart' show LibraryGraph;
-
 import 'kernel/kernel_library_builder.dart' show KernelLibraryBuilder;
+
 import 'kernel/kernel_shadow_ast.dart' show ShadowVariableDeclaration;
 
+import 'library_graph.dart' show LibraryGraph;
+
 import 'source/source_library_builder.dart' show SourceLibraryBuilder;
 
 import 'ticker.dart' show Ticker;
 
 import 'uri_translator.dart' show UriTranslator;
 
-import 'combinator.dart' show Combinator;
-
 class IncrementalCompiler implements IncrementalKernelGenerator {
   final CompilerContext context;
 
@@ -112,6 +122,25 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
             // We might have loaded x out of y libraries into the component.
             // To avoid any unforeseen problems start over.
             bytesLength = prepareSummary(summaryBytes, uriTranslator, c, data);
+
+            if (e is InvalidKernelVersionError || e is PackageChangedError) {
+              // Don't report any warning.
+            } else if (e is CanonicalNameError) {
+              dillLoadedData.loader.addProblem(
+                  templateInitializeFromDillNotSelfContained
+                      .withArguments(initializeFromDillUri.toString()),
+                  TreeNode.noOffset,
+                  1,
+                  null);
+            } else {
+              // Unknown error: Report problem as such.
+              dillLoadedData.loader.addProblem(
+                  templateInitializeFromDillUnknownProblem.withArguments(
+                      initializeFromDillUri.toString(), "$e"),
+                  TreeNode.noOffset,
+                  1,
+                  null);
+            }
           }
         }
         appendLibraries(data, bytesLength);
@@ -348,7 +377,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
         // We're going to output all we read here so lazy loading it
         // doesn't make sense.
         new BinaryBuilder(initializationBytes, disableLazyReading: true)
-            .readComponent(data.component);
+            .readComponent(data.component, checkCanonicalNames: true);
 
         // Check the any package-urls still point to the same file
         // (e.g. the package still exists and hasn't been updated).
@@ -360,7 +389,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
             // Everything that depends on it should be thrown away.
             // TODO(jensj): Anything that doesn't depend on it can be kept.
             // For now just don't initialize from this dill.
-            throw "Changed package";
+            throw const PackageChangedError();
           }
         }
 
@@ -612,6 +641,10 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
   void recordInvalidatedImportUrisForTesting(List<Uri> uris) {}
 }
 
+class PackageChangedError {
+  const PackageChangedError();
+}
+
 class IncrementalCompilerData {
   bool includeUserLoadedLibraries;
   Procedure userLoadedUriMain;

pkg/front_end/messages.yaml

@@ -2281,3 +2281,15 @@ InitializingFormalTypeMismatchField:
 UseOfDeprecatedIdentifier:
   template: "'#name' is deprecated."
   severity: IGNORED
+
+InitializeFromDillNotSelfContained:
+  template: "Tried to initialize from a previous compilation (#string), but the file was not self-contained. This might be a bug, please report at http://dartbug.com/new."
+  severity: WARNING
+  frontendInternal: true
+  external: test/incremental_load_from_invalid_dill_test.dart
+
+InitializeFromDillUnknownProblem:
+  template: "Tried to initialize from a previous compilation (#string), but couldn't. This might be a bug, please report at http://dartbug.com/new. Error message was '#string2'."
+  severity: WARNING
+  frontendInternal: true
+  external: test/incremental_load_from_invalid_dill_test.dart

pkg/front_end/test/fasta/messages_test.dart

@@ -229,8 +229,10 @@ class MessageTestSuite extends ChainContext {
           null,
           exampleAndAnalyzerCodeRequired &&
                   externalTest != null &&
-                  !(new File(externalTest).existsSync())
-              ? "Given external example for $name points to a nonexisting file."
+                  !(new File.fromUri(suite.uri.resolve(externalTest))
+                      .existsSync())
+              ? "Given external example for $name points to a nonexisting file "
+                  "(${suite.uri.resolve(externalTest)})."
               : null);
 
       yield createDescription(

pkg/front_end/test/incremental_dart2js_load_from_dill_test.dart

@@ -29,28 +29,17 @@ void testDart2jsCompile() async {
   Uri fullDillFromInitialized =
       outDir.uri.resolve("dart2js.full_from_initialized.dill");
   Uri nonexisting = outDir.uri.resolve("dart2js.nonexisting.dill");
-  Uri nonLoadable = outDir.uri.resolve("dart2js.nonloadable.dill");
 
   // Compile dart2js without initializing from dill.
   Stopwatch stopwatch = new Stopwatch()..start();
   await normalCompile(dart2jsUrl, normalDill);
   print("Normal compile took ${stopwatch.elapsedMilliseconds} ms");
 
-  // Create a file that cannot be (fully) loaded as a dill file.
-  List<int> corruptData = new File.fromUri(normalDill).readAsBytesSync();
-  for (int i = 10 * (corruptData.length ~/ 16);
-      i < 15 * (corruptData.length ~/ 16);
-      ++i) {
-    corruptData[i] = 42;
-  }
-  new File.fromUri(nonLoadable).writeAsBytesSync(corruptData);
-
   // Compile dart2js, initializing from the just-compiled dill,
   // a nonexisting file and a dill file that isn't valid.
   for (List<Object> initializationData in [
     [normalDill, true],
     [nonexisting, false],
-    //  [nonLoadable, false] // disabled for now
   ]) {
     Uri initializeWith = initializationData[0];
     bool initializeExpect = initializationData[1];

pkg/front_end/test/incremental_load_from_dill_test.dart

@@ -115,8 +115,8 @@ class RunCompilations extends Step<TestData, TestData, Context> {
   }
 }
 
-void basicTest(Map<String, String> sourceFiles, String entryPoint, bool strong,
-    List<String> invalidate, Directory outDir) async {
+Future<Null> basicTest(Map<String, String> sourceFiles, String entryPoint,
+    bool strong, List<String> invalidate, Directory outDir) async {
   Uri entryPointUri = outDir.uri.resolve(entryPoint);
   Set<String> invalidateFilenames = invalidate?.toSet() ?? new Set<String>();
   List<Uri> invalidateUris = <Uri>[];
@@ -172,7 +172,7 @@ void basicTest(Map<String, String> sourceFiles, String entryPoint, bool strong,
   checkIsEqual(normalDillData, initializedDillData);
 }
 
-void newWorldTest(bool strong, List worlds) async {
+Future<Null> newWorldTest(bool strong, List worlds) async {
   final Uri sdkRoot = computePlatformBinariesLocation();
   final Uri base = Uri.parse("org-dartlang-test:///");
   final Uri sdkSummary = base.resolve("vm_platform.dill");

pkg/front_end/test/incremental_load_from_invalid_dill_test.dart

@@ -0,0 +1,175 @@
+// Copyright (c) 2018, 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' show Future;
+
+import 'dart:io' show File;
+
+import 'package:expect/expect.dart' show Expect;
+
+import 'package:front_end/src/api_prototype/compiler_options.dart'
+    show CompilerOptions;
+
+import "package:front_end/src/api_prototype/memory_file_system.dart"
+    show MemoryFileSystem;
+
+import 'package:front_end/src/base/processed_options.dart'
+    show ProcessedOptions;
+
+import 'package:front_end/src/compute_platform_binaries_location.dart'
+    show computePlatformBinariesLocation;
+
+import 'package:front_end/src/fasta/compiler_context.dart' show CompilerContext;
+
+import 'package:front_end/src/fasta/fasta_codes.dart'
+    show
+        Template,
+        templateInitializeFromDillNotSelfContained,
+        templateInitializeFromDillUnknownProblem,
+        FormattedMessage;
+
+import 'package:front_end/src/fasta/fasta_codes.dart' show FormattedMessage;
+
+import 'package:front_end/src/fasta/incremental_compiler.dart'
+    show IncrementalCompiler;
+
+import 'package:front_end/src/fasta/kernel/utils.dart' show serializeComponent;
+
+import 'package:front_end/src/fasta/severity.dart' show Severity;
+
+import 'package:kernel/kernel.dart' show Component;
+
+import 'incremental_load_from_dill_test.dart' show getOptions;
+
+Future<Null> main() async {
+  Tester tester = new Tester();
+  await tester.initialize();
+  await tester.test();
+}
+
+class Tester {
+  Uri sdkRoot;
+  Uri base;
+  Uri sdkSummary;
+  Uri initializeFrom;
+  Uri helperFile;
+  Uri entryPoint;
+  Uri platformUri;
+  List<int> sdkSummaryData;
+  List<FormattedMessage> formattedErrors;
+  List<FormattedMessage> formattedWarnings;
+  MemoryFileSystem fs;
+  CompilerOptions options;
+
+  compileExpectInitializeFailAndSpecificWarning(
+      Template expectedWarningTemplate) async {
+    formattedErrors.clear();
+    formattedWarnings.clear();
+    IncrementalCompiler compiler = new IncrementalCompiler(
+        new CompilerContext(new ProcessedOptions(options, [entryPoint])),
+        initializeFrom);
+    await compiler.computeDelta();
+    if (compiler.initializedFromDill) {
+      Expect.fail("Expected to not be able to initialized from dill, but did.");
+    }
+    if (formattedErrors.isNotEmpty) {
+      Expect.fail("Got unexpected errors: $formattedErrors");
+    }
+    if (formattedWarnings.length != 1) {
+      Expect.fail(
+          "Got unexpected errors: Expected one, got this: $formattedWarnings");
+    }
+    if (formattedWarnings[0].code.template != expectedWarningTemplate) {
+      Expect.fail("Expected $expectedWarningTemplate "
+          "but got $formattedWarnings");
+    }
+  }
+
+  initialize() async {
+    sdkRoot = computePlatformBinariesLocation();
+    base = Uri.parse("org-dartlang-test:///");
+    sdkSummary = base.resolve("vm_platform.dill");
+    initializeFrom = base.resolve("initializeFrom.dill");
+    helperFile = base.resolve("helper.dart");
+    entryPoint = base.resolve("small.dart");
+    platformUri = sdkRoot.resolve("vm_platform_strong.dill");
+    sdkSummaryData = await new File.fromUri(platformUri).readAsBytes();
+    formattedErrors = <FormattedMessage>[];
+    formattedWarnings = <FormattedMessage>[];
+    fs = new MemoryFileSystem(base);
+    options = getOptions(true);
+
+    options.fileSystem = fs;
+    options.sdkRoot = null;
+    options.sdkSummary = sdkSummary;
+    options.onProblem = (FormattedMessage problem, Severity severity,
+        List<FormattedMessage> context) {
+      if (severity == Severity.error) {
+        formattedErrors.add(problem);
+      } else if (severity == Severity.warning) {
+        formattedWarnings.add(problem);
+      }
+    };
+
+    fs.entityForUri(sdkSummary).writeAsBytesSync(sdkSummaryData);
+    fs.entityForUri(helperFile).writeAsStringSync("""
+foo() {
+    print("hello from foo");
+}
+""");
+    fs.entityForUri(entryPoint).writeAsStringSync("""
+import "helper.dart" as helper;
+main() {
+    helper.foo();
+}
+""");
+  }
+
+  Future<Null> test() async {
+    IncrementalCompiler compiler = new IncrementalCompiler(
+        new CompilerContext(new ProcessedOptions(options, [entryPoint])),
+        initializeFrom);
+
+    Component componentGood = await compiler.computeDelta();
+    List<int> dataGood = serializeComponent(componentGood);
+    fs.entityForUri(initializeFrom).writeAsBytesSync(dataGood);
+
+    // Initialize from good dill file should be ok.
+    compiler = new IncrementalCompiler(
+        new CompilerContext(new ProcessedOptions(options, [entryPoint])),
+        initializeFrom);
+    compiler.invalidate(entryPoint);
+    Component component = await compiler.computeDelta();
+    if (!compiler.initializedFromDill) {
+      Expect.fail(
+          "Expected to have sucessfully initialized from dill, but didn't.");
+    }
+    if (formattedErrors.isNotEmpty) {
+      Expect.fail("Got unexpected errors: $formattedErrors");
+    }
+    if (formattedWarnings.isNotEmpty) {
+      Expect.fail("Got unexpected errors: $formattedWarnings");
+    }
+
+    // Create a partial dill file.
+    compiler.invalidate(entryPoint);
+    component = await compiler.computeDelta();
+    if (component.libraries.length != 1) {
+      Expect.fail("Expected 1 library, got ${component.libraries.length}: "
+          "${component.libraries}");
+    }
+    List<int> data = serializeComponent(component);
+    fs.entityForUri(initializeFrom).writeAsBytesSync(data);
+
+    // Initializing from partial dill should not be ok.
+    await compileExpectInitializeFailAndSpecificWarning(
+        templateInitializeFromDillNotSelfContained);
+
+    // Create a invalid dill file to load from: Should not be ok.
+    data = new List<int>.filled(42, 42);
+    fs.entityForUri(initializeFrom).writeAsBytesSync(data);
+    await compileExpectInitializeFailAndSpecificWarning(
+        templateInitializeFromDillUnknownProblem);
+  }
+}

pkg/kernel/lib/binary/ast_from_binary.dart

@@ -22,6 +22,18 @@ class ParseError {
   String toString() => '$filename:$byteIndex: $message at $path';
 }
 
+class InvalidKernelVersionError {
+  final String message;
+
+  InvalidKernelVersionError(this.message);
+}
+
+class CanonicalNameError {
+  final String message;
+
+  CanonicalNameError(this.message);
+}
+
 class _ComponentIndex {
   static const numberOfFixedFields = 9;
 
@@ -390,7 +402,7 @@ class BinaryBuilder {
   /// computed ahead of time.
   ///
   /// The input bytes may contain multiple files concatenated.
-  void readComponent(Component component) {
+  void readComponent(Component component, {bool checkCanonicalNames: false}) {
     List<int> componentFileSizes = _indexComponents();
     if (componentFileSizes.length > 1) {
       _disableLazyReading = true;
@@ -400,6 +412,10 @@ class BinaryBuilder {
       _readOneComponent(component, componentFileSizes[componentFileIndex]);
       ++componentFileIndex;
     }
+
+    if (checkCanonicalNames) {
+      _checkCanonicalNameChildren(component.root);
+    }
   }
 
   /// Deserializes the source and stores it in [component].
@@ -426,7 +442,8 @@ class BinaryBuilder {
   ///
   /// This should *only* be used when there is a reason to not allow
   /// concatenated files.
-  void readSingleFileComponent(Component component) {
+  void readSingleFileComponent(Component component,
+      {bool checkCanonicalNames: false}) {
     List<int> componentFileSizes = _indexComponents();
     if (componentFileSizes.isEmpty) throw "Invalid component data.";
     _readOneComponent(component, componentFileSizes[0]);
@@ -440,6 +457,48 @@ class BinaryBuilder {
       }
       throw 'Unrecognized bytes following component data';
     }
+
+    if (checkCanonicalNames) {
+      _checkCanonicalNameChildren(component.root);
+    }
+  }
+
+  void _checkCanonicalNameChildren(CanonicalName parent) {
+    for (CanonicalName child in parent.children) {
+      if (child.name != '@methods' &&
+          child.name != '@typedefs' &&
+          child.name != '@fields' &&
+          child.name != '@getters' &&
+          child.name != '@setters' &&
+          child.name != '@factories' &&
+          child.name != '@constructors') {
+        bool checkReferenceNode = true;
+        if (child.reference == null) {
+          // OK for "if private: URI of library" part of "Qualified name"...
+          Iterable<CanonicalName> children = child.children;
+          if (parent.parent != null &&
+              children.isNotEmpty &&
+              children.first.name.startsWith("_")) {
+            // OK then.
+            checkReferenceNode = false;
+          } else {
+            throw new CanonicalNameError(
+                "Null reference (${child.name}) ($child).");
+          }
+        }
+        if (checkReferenceNode) {
+          if (child.reference.canonicalName != child) {
+            throw new CanonicalNameError(
+                "Canonical name and reference doesn't agree.");
+          }
+          if (child.reference.node == null) {
+            throw new CanonicalNameError(
+                "Reference is null (${child.name}) ($child).");
+          }
+        }
+      }
+      _checkCanonicalNameChildren(child);
+    }
   }
 
   _ComponentIndex _readComponentIndex(int componentFileSize) {
@@ -520,7 +579,8 @@ class BinaryBuilder {
 
     final int formatVersion = readUint32();
     if (formatVersion != Tag.BinaryFormatVersion) {
-      throw fail('Invalid kernel binary format version '
+      throw new InvalidKernelVersionError(
+          'Invalid kernel binary format version '
           '(found ${formatVersion}, expected ${Tag.BinaryFormatVersion})');
     }
 

pkg/kernel/lib/canonical_name.dart

@@ -30,6 +30,11 @@ import 'ast.dart';
 ///         "@fields"
 ///         Qualified name
 ///
+///      Typedef:
+///         Canonical name of enclosing class
+///         "@typedefs"
+///         Name text
+///
 ///      Procedure that is not an accessor or factory:
 ///         Canonical name of enclosing class or library
 ///         "@methods"