From e6baec457780f9da395045cac89cd423d8b795ea Mon Sep 17 00:00:00 2001
From: Daniel Dunbar <daniel_dunbar@apple.com>
Date: Sat, 15 Oct 2016 21:52:15 -0700
Subject: [PATCH] [Core] Fix a case where we could fail to detect a cycle.

 - In rare circumstances (which unfortunately I have not been able to reduce a
   sensible test case for), we could get in a situation where we failed to
   detect a cycle, because we were not considering *all* of the scan records,
   just the ones accessible via a task.

 - https://bugs.swift.org/browse/SR-1948
---
 lib/Core/BuildEngine.cpp | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/lib/Core/BuildEngine.cpp b/lib/Core/BuildEngine.cpp
index f24daa867..bedb2a530 100644
--- a/lib/Core/BuildEngine.cpp
+++ b/lib/Core/BuildEngine.cpp
@@ -187,6 +187,10 @@ class BuildEngineImpl {
       assert(isScanning());
       return inProgressInfo.pendingScanRecord;
     }
+    const RuleScanRecord* getPendingScanRecord() const {
+      assert(isScanning());
+      return inProgressInfo.pendingScanRecord;
+    }
     void setPendingScanRecord(RuleScanRecord* value) {
       inProgressInfo.pendingScanRecord = value;
     }
@@ -195,6 +199,10 @@ class BuildEngineImpl {
       assert(isInProgress());
       return inProgressInfo.pendingTaskInfo;
     }
+    const TaskInfo* getPendingTaskInfo() const {
+      assert(isInProgress());
+      return inProgressInfo.pendingTaskInfo;
+    }
     void setPendingTaskInfo(TaskInfo* value) {
       assert(isInProgress());
       inProgressInfo.pendingTaskInfo = value;
@@ -869,7 +877,7 @@ class BuildEngineImpl {
 
     // Gather all of the successor relationships.
     std::unordered_map<Rule*, std::vector<Rule*>> graph;
-    std::vector<RuleScanRecord *> activeRuleScanRecords;
+    std::vector<const RuleScanRecord *> activeRuleScanRecords;
     for (const auto& it: taskInfos) {
       const TaskInfo& taskInfo = it.second;
       assert(taskInfo.forRuleInfo);
@@ -881,14 +889,22 @@ class BuildEngineImpl {
       for (const auto& request: taskInfo.deferredScanRequests) {
         // Add the sucessor for the deferred relationship itself.
         successors.push_back(&request.ruleInfo->rule);
-          
-        // Add the active rule scan record which needs to be traversed.
-        assert(request.ruleInfo->isScanning());
-        activeRuleScanRecords.push_back(
-            request.ruleInfo->getPendingScanRecord());
       }
       graph.insert({ &taskInfo.forRuleInfo->rule, successors });
     }
+
+    // Add the pending scan records for every rule.
+    //
+    // NOTE: There is a very subtle condition around this versus adding the ones
+    // accessible via the tasks, see https://bugs.swift.org/browse/SR-1948.
+    // Unfortunately, we do not have a test case for this!
+    for (const auto& it: ruleInfos) {
+      const RuleInfo& ruleInfo = it.second;
+      if (ruleInfo.isScanning()) {
+        const auto* scanRecord = ruleInfo.getPendingScanRecord();
+        activeRuleScanRecords.push_back(scanRecord);
+      }
+    }
       
     // Gather dependencies from all of the active scan records.
     std::unordered_set<const RuleScanRecord*> visitedRuleScanRecords;