Skip to content

[Dependency Scanning] Restrict Swift overlay lookup to "visible" Clang modules only #83050

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions include/swift/AST/ModuleDependencies.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include "swift/AST/Import.h"
#include "swift/AST/LinkLibrary.h"
#include "swift/Basic/Assertions.h"
#include "swift/Basic/CXXStdlibKind.h"
#include "swift/Basic/LLVM.h"
#include "swift/Serialization/Validation.h"
Expand Down Expand Up @@ -212,6 +213,11 @@ class ModuleDependencyInfoStorageBase {
/// The macro dependencies.
std::map<std::string, MacroPluginDependency> macroDependencies;

/// A list of Clang modules that are visible to this Swift module. This
/// includes both direct Clang modules as well as transitive Clang
/// module dependencies when they are exported
llvm::StringSet<> visibleClangModules;

/// ModuleDependencyInfo is finalized (with all transitive dependencies
/// and inputs).
bool finalized;
Expand Down Expand Up @@ -816,7 +822,17 @@ class ModuleDependencyInfo {
cast<ClangModuleDependencyStorage>(storage.get())->CASFileSystemRootID =
rootID;
else
llvm_unreachable("Unexpected type");
llvm_unreachable("Unexpected module dependency kind");
}

llvm::StringSet<> &getVisibleClangModules() const {
return storage->visibleClangModules;
}

void
addVisibleClangModules(const std::vector<std::string> &moduleNames) const {
storage->visibleClangModules.insert(moduleNames.begin(),
moduleNames.end());
}

/// Whether explicit input paths of all the module dependencies
Expand Down Expand Up @@ -1057,6 +1073,9 @@ class ModuleDependenciesCache {
/// Query all cross-import overlay dependencies
llvm::ArrayRef<ModuleDependencyID>
getCrossImportOverlayDependencies(const ModuleDependencyID &moduleID) const;
/// Query all visible Clang modules for a given Swift dependency
llvm::StringSet<>&
getVisibleClangModules(ModuleDependencyID moduleID) const;

/// Look for module dependencies for a module with the given ID
///
Expand Down Expand Up @@ -1087,9 +1106,9 @@ class ModuleDependenciesCache {
void recordDependency(StringRef moduleName,
ModuleDependencyInfo dependencies);

/// Record dependencies for the given module collection.
void recordDependencies(ModuleDependencyVector moduleDependencies,
DiagnosticEngine &diags);
/// Record dependencies for the given collection of Clang modules.
void recordClangDependencies(ModuleDependencyVector moduleDependencies,
DiagnosticEngine &diags);

/// Update stored dependencies for the given module.
void updateDependency(ModuleDependencyID moduleID,
Expand Down Expand Up @@ -1122,6 +1141,10 @@ class ModuleDependenciesCache {
void
setCrossImportOverlayDependencies(ModuleDependencyID moduleID,
const ArrayRef<ModuleDependencyID> dependencyIDs);
/// Add to this module's set of visible Clang modules
void
addVisibleClangModules(ModuleDependencyID moduleID,
const std::vector<std::string> &moduleNames);

StringRef getMainModuleName() const { return mainScanModuleName; }

Expand Down
3 changes: 2 additions & 1 deletion include/swift/DependencyScan/ModuleDependencyScanner.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class ModuleDependencyScanningWorker {

private:
/// Retrieve the module dependencies for the Clang module with the given name.
ModuleDependencyVector scanFilesystemForClangModuleDependency(
ClangModuleScannerQueryResult scanFilesystemForClangModuleDependency(
Identifier moduleName,
const llvm::DenseSet<clang::tooling::dependencies::ModuleID>
&alreadySeenModules);
Expand Down Expand Up @@ -72,6 +72,7 @@ class ModuleDependencyScanningWorker {
ModuleDependencyIDSetVector &headerClangModuleDependencies,
std::vector<std::string> &headerFileInputs,
std::vector<std::string> &bridgingHeaderCommandLine,
std::vector<std::string> &visibleClangModules,
std::optional<std::string> &includeTreeID);


Expand Down
12 changes: 12 additions & 0 deletions include/swift/Serialization/ScanningLoaders.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,18 @@ struct SwiftModuleScannerQueryResult {
std::vector<IncompatibleCandidate> incompatibleCandidates;
};

/// Result of looking up a Clang module on the current filesystem
/// search paths.
struct ClangModuleScannerQueryResult {
ClangModuleScannerQueryResult(const ModuleDependencyVector &dependencyModuleGraph,
const std::vector<std::string> &visibleModuleIdentifiers)
: foundDependencyModuleGraph(dependencyModuleGraph),
visibleModuleIdentifiers(visibleModuleIdentifiers) {}

ModuleDependencyVector foundDependencyModuleGraph;
std::vector<std::string> visibleModuleIdentifiers;
};

/// A module "loader" that looks for .swiftinterface and .swiftmodule files
/// for the purpose of determining dependencies, but does not attempt to
/// load the module files.
Expand Down
121 changes: 68 additions & 53 deletions lib/AST/ModuleDependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -778,65 +778,62 @@ void ModuleDependenciesCache::recordDependency(
map.insert({moduleName, dependency});
}

void ModuleDependenciesCache::recordDependencies(
void ModuleDependenciesCache::recordClangDependencies(
ModuleDependencyVector dependencies, DiagnosticEngine &diags) {
for (const auto &dep : dependencies) {
ASSERT(dep.first.Kind == ModuleDependencyKind::Clang);
auto newClangModuleDetails = dep.second.getAsClangModule();
if (hasDependency(dep.first)) {
if (dep.first.Kind == ModuleDependencyKind::Clang) {
auto priorClangModuleDetails =
findKnownDependency(dep.first).getAsClangModule();
auto newClangModuleDetails = dep.second.getAsClangModule();
auto priorContextHash = priorClangModuleDetails->contextHash;
auto newContextHash = newClangModuleDetails->contextHash;
if (priorContextHash != newContextHash) {
// This situation means that within the same scanning action, Clang
// Dependency Scanner has produced two different variants of the same
// module. This is not supposed to happen, but we are currently
// hunting down the rare cases where it does, seemingly due to
// differences in Clang Scanner direct by-name queries and transitive
// header lookup queries.
//
// Emit a failure diagnostic here that is hopefully more actionable
// for the time being.
diags.diagnose(SourceLoc(), diag::dependency_scan_unexpected_variant,
dep.first.ModuleName);
diags.diagnose(
SourceLoc(),
diag::dependency_scan_unexpected_variant_context_hash_note,
priorContextHash, newContextHash);
diags.diagnose(
SourceLoc(),
diag::dependency_scan_unexpected_variant_module_map_note,
priorClangModuleDetails->moduleMapFile,
newClangModuleDetails->moduleMapFile);

auto diagnoseExtraCommandLineFlags =
[&diags](const ClangModuleDependencyStorage *checkModuleDetails,
const ClangModuleDependencyStorage *baseModuleDetails,
bool isNewlyDiscovered) -> void {
std::unordered_set<std::string> baseCommandLineSet(
baseModuleDetails->buildCommandLine.begin(),
baseModuleDetails->buildCommandLine.end());
for (const auto &checkArg : checkModuleDetails->buildCommandLine)
if (baseCommandLineSet.find(checkArg) == baseCommandLineSet.end())
diags.diagnose(
SourceLoc(),
diag::dependency_scan_unexpected_variant_extra_arg_note,
isNewlyDiscovered, checkArg);
};
diagnoseExtraCommandLineFlags(priorClangModuleDetails,
newClangModuleDetails, true);
diagnoseExtraCommandLineFlags(newClangModuleDetails,
priorClangModuleDetails, false);
}
auto priorClangModuleDetails =
findKnownDependency(dep.first).getAsClangModule();
DEBUG_ASSERT(priorClangModuleDetails && newClangModuleDetails);
auto priorContextHash = priorClangModuleDetails->contextHash;
auto newContextHash = newClangModuleDetails->contextHash;
if (priorContextHash != newContextHash) {
// This situation means that within the same scanning action, Clang
// Dependency Scanner has produced two different variants of the same
// module. This is not supposed to happen, but we are currently
// hunting down the rare cases where it does, seemingly due to
// differences in Clang Scanner direct by-name queries and transitive
// header lookup queries.
//
// Emit a failure diagnostic here that is hopefully more actionable
// for the time being.
diags.diagnose(SourceLoc(), diag::dependency_scan_unexpected_variant,
dep.first.ModuleName);
diags.diagnose(
SourceLoc(),
diag::dependency_scan_unexpected_variant_context_hash_note,
priorContextHash, newContextHash);
diags.diagnose(
SourceLoc(),
diag::dependency_scan_unexpected_variant_module_map_note,
priorClangModuleDetails->moduleMapFile,
newClangModuleDetails->moduleMapFile);

auto diagnoseExtraCommandLineFlags =
[&diags](const ClangModuleDependencyStorage *checkModuleDetails,
const ClangModuleDependencyStorage *baseModuleDetails,
bool isNewlyDiscovered) -> void {
std::unordered_set<std::string> baseCommandLineSet(
baseModuleDetails->buildCommandLine.begin(),
baseModuleDetails->buildCommandLine.end());
for (const auto &checkArg : checkModuleDetails->buildCommandLine)
if (baseCommandLineSet.find(checkArg) == baseCommandLineSet.end())
diags.diagnose(
SourceLoc(),
diag::dependency_scan_unexpected_variant_extra_arg_note,
isNewlyDiscovered, checkArg);
};
diagnoseExtraCommandLineFlags(priorClangModuleDetails,
newClangModuleDetails, true);
diagnoseExtraCommandLineFlags(newClangModuleDetails,
priorClangModuleDetails, false);
}
} else
} else {
recordDependency(dep.first.ModuleName, dep.second);

if (dep.first.Kind == ModuleDependencyKind::Clang) {
auto clangModuleDetails = dep.second.getAsClangModule();
addSeenClangModule(clang::tooling::dependencies::ModuleID{
dep.first.ModuleName, clangModuleDetails->contextHash});
dep.first.ModuleName, newClangModuleDetails->contextHash});
}
}
}
Expand Down Expand Up @@ -918,6 +915,24 @@ ModuleDependenciesCache::setCrossImportOverlayDependencies(ModuleDependencyID mo
updateDependency(moduleID, updatedDependencyInfo);
}

void
ModuleDependenciesCache::addVisibleClangModules(ModuleDependencyID moduleID,
const std::vector<std::string> &moduleNames) {
if (moduleNames.empty())
return;
auto dependencyInfo = findKnownDependency(moduleID);
auto updatedDependencyInfo = dependencyInfo;
updatedDependencyInfo.addVisibleClangModules(moduleNames);
updateDependency(moduleID, updatedDependencyInfo);
}

llvm::StringSet<> &ModuleDependenciesCache::getVisibleClangModules(ModuleDependencyID moduleID) const {
ASSERT(moduleID.Kind == ModuleDependencyKind::SwiftSource ||
moduleID.Kind == ModuleDependencyKind::SwiftInterface ||
moduleID.Kind == ModuleDependencyKind::SwiftBinary);
return findKnownDependency(moduleID).getVisibleClangModules();
}

ModuleDependencyIDSetVector
ModuleDependenciesCache::getAllDependencies(const ModuleDependencyID &moduleID) const {
const auto &moduleInfo = findKnownDependency(moduleID);
Expand Down
Loading