Close Soundness Hole in Incremental Build

Fix a longstanding bug in the driver where the incremental build would
fail to execute dependent jobs and allow errors in primaries in later
waves to become buried. A simplified version of a situation that exposes
this bug has been committed into the tests.

Imagine two files:

```
// one.swift

struct A {}

// two.swift

func use() {
  let _ = A()
}
```

After a clean build, we modify one.swift as follows:

```
// one.swift

struct B< {}
```

The syntax error in this file caused the baseline driver to fail the
build immediately and thus we would not schedule two.swift for
execution. This is incorrect! Consider correcting the syntax error in
one.swift:

```
// one.swift

struct B {}
```

two.swift _still_ won't be rebuilt because
1) It was never modified during any step of this process
2) The swiftdeps of two.swift have not been updated to flush out the
   dependency on A.

rdar://72800626
This commit is contained in:
Robert Widmann
2021-01-04 20:06:02 -08:00
parent d710cfc3a5
commit 9d06eef2a6
8 changed files with 75 additions and 3 deletions

View File

@@ -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);