[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
diff --git a/lib/Core/BuildEngine.cpp b/lib/Core/BuildEngine.cpp
index f24daa8..bedb2a5 100644
--- a/lib/Core/BuildEngine.cpp
+++ b/lib/Core/BuildEngine.cpp
@@ -187,6 +187,10 @@
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 @@
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 @@
// 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 @@
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;
