Merge pull request swiftlang#35260 from CodaFi/continuation-passing-style

Close Soundness Hole in Incremental Build

Author: CodaFi

lib/FrontendTool/FrontendTool.cpp

@@ -617,7 +617,7 @@ constructDetailedTaskDescription(const CompilerInvocation &Invocation,
                                  Outputs};
 }
 
-static void emitReferenceDependenciesForAllPrimaryInputsIfNeeded(
+static void emitSwiftdepsForAllPrimaryInputsIfNeeded(
     CompilerInstance &Instance) {
   const auto &Invocation = Instance.getInvocation();
   if (Invocation.getFrontendOptions()
@@ -627,6 +627,22 @@ static void emitReferenceDependenciesForAllPrimaryInputsIfNeeded(
         SourceLoc(), diag::emit_reference_dependencies_without_primary_file);
     return;
   }
+
+  // Do not write out swiftdeps for any primaries if we've encountered an
+  // error. Without this, the driver will attempt to integrate swiftdeps
+  // from broken swift files. One way this could go wrong is if a primary that
+  // fails to build in an early wave has dependents in a later wave. The
+  // driver will not schedule those later dependents after this batch exits,
+  // so they will have no opportunity to bring their swiftdeps files up to
+  // date. With this early exit, the driver sees the same priors in the
+  // swiftdeps files from before errors were introduced into the batch, and
+  // integration therefore always hops from "known good" to "known good" states.
+  //
+  // FIXME: It seems more appropriate for the driver to notice the early-exit
+  // and react by always enqueuing the jobs it dropped in the other waves.
+  if (Instance.getDiags().hadAnyError())
+    return;
+
   for (auto *SF : Instance.getPrimarySourceFiles()) {
     const std::string &referenceDependenciesFilePath =
         Invocation.getReferenceDependenciesFilePathForPrimary(
@@ -1035,8 +1051,10 @@ static void performEndOfPipelineActions(CompilerInstance &Instance) {
     emitIndexData(Instance);
   }
 
-  // Emit dependencies.
-  emitReferenceDependenciesForAllPrimaryInputsIfNeeded(Instance);
+  // Emit Swiftdeps for every file in the batch.
+  emitSwiftdepsForAllPrimaryInputsIfNeeded(Instance);
+
+  // Emit Make-style dependencies.
   emitMakeDependenciesIfNeeded(Instance.getDiags(),
                                Instance.getDependencyTracker(), opts);
 

test/Incremental/Inputs/cross-file-failure/definesA-one.swift

@@ -0,0 +1 @@
+struct A {}

test/Incremental/Inputs/cross-file-failure/definesA-three.swift

@@ -0,0 +1 @@
+struct B {}

test/Incremental/Inputs/cross-file-failure/definesA-two.swift

@@ -0,0 +1 @@
+struct B< {}

test/Incremental/Inputs/cross-file-failure/main.swift

@@ -0,0 +1 @@
+

test/Incremental/Inputs/cross-file-failure/ofm.json

@@ -0,0 +1,18 @@
+{
+  "main.swift": {
+    "object": "./main.o",
+    "swift-dependencies": "./main.swiftdeps"
+  },
+  "definesA.swift": {
+    "object": "./definesA.o",
+    "swift-dependencies": "./definesA.swiftdeps"
+  },
+  "usesA.swift": {
+    "object": "./usesA.o",
+    "swift-dependencies": "./usesA.swiftdeps"
+  },
+  "": {
+    "swift-dependencies": "./main~buildrecord.swiftdeps"
+  }
+}
+

test/Incremental/Inputs/cross-file-failure/usesA.swift

@@ -0,0 +1,3 @@
+func use() {
+  let _ = A()
+}

test/Incremental/cross-file-failure.swift

@@ -0,0 +1,29 @@
+// Establish baseline
+
+// RUN: %empty-directory(%t)
+// RUN: cp %S/Inputs/cross-file-failure/* %t
+// RUN: cp %t/definesA{-one,}.swift
+// RUN: touch -t 200101010101 %t/*.swift
+// RUN: cd %t
+
+// RUN: %target-swiftc_driver -enable-batch-mode -j2 -incremental -driver-show-incremental main.swift definesA.swift usesA.swift -module-name main -output-file-map ofm.json >&output-baseline
+
+// Change one type and cause a syntax error. This should cause _both_ files to
+// rebuild.
+
+// RUN: cp %t/definesA{-two,}.swift
+// RUN: touch -t 200201010101 %t/*
+// RUN: touch -t 200101010101 %t/*.swift
+// RUN: touch -t 200301010101 %t/definesA.swift
+
+// RUN: not %target-swiftc_driver -enable-batch-mode -j2 -incremental -driver-show-incremental main.swift definesA.swift usesA.swift -module-name main -output-file-map ofm.json
+
+// RUN: cp %t/definesA{-three,}.swift
+// RUN: touch -t 200401010101 %t/definesA.swift
+
+// RUN: not %target-swiftc_driver -enable-batch-mode -j2 -incremental -driver-show-incremental main.swift definesA.swift usesA.swift -module-name main -output-file-map ofm.json >&output-incremental
+
+// RUN: %FileCheck -check-prefix=CHECK-RECOMPILED %s --dump-input=always < %t/output-incremental
+
+// CHECK-RECOMPILED: Queuing (initial): {compile: definesA.o <= definesA.swift}
+// CHECK-RECOMPILED: Queuing because of dependencies discovered later: {compile: usesA.o <= usesA.swift}