Skip to content

[5.1] Cherry-pick some more module interface fixes #24051

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.

Sign up for GitHub

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 2 commits into from
Apr 16, 2019
Merged

[5.1] Cherry-pick some more module interface fixes #24051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d40c866
[ParseableInterface] Normalize paths before comparison when serializi…
Apr 5, 2019
426fadb
[ParseableInterfaces] Improvements to forwarding module dependencies
Apr 11, 2019
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
157 changes: 103 additions & 54 deletions lib/Frontend/ParseableInterfaceModuleLoader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,32 @@ static Optional<llvm::vfs::Status> getStatusOfDependency(
return status.get();
}

/// If the file dependency in \p FullDepPath is inside the \p Base directory,
/// this returns its path relative to \p Base. Otherwise it returns None.
static Optional<StringRef> getRelativeDepPath(StringRef DepPath,
StringRef Base) {
// If Base is the root directory, or DepPath does not start with Base, bail.
if (Base.size() <= 1 || !DepPath.startswith(Base)) {
return None;
}

assert(DepPath.size() > Base.size() &&
"should never depend on a directory");

// Is the DepName something like ${Base}/foo.h"?
if (path::is_separator(DepPath[Base.size()]))
return DepPath.substr(Base.size() + 1);

// Is the DepName something like "${Base}foo.h", where Base
// itself contains a trailing slash?
if (path::is_separator(Base.back()))
return DepPath.substr(Base.size());

// We have something next to Base, like "Base.h", that's somehow
// become a dependency.
return None;
}

#pragma mark - Module Building

/// Builds a parseable module interface into a .swiftmodule at the provided
Expand Down Expand Up @@ -365,52 +391,33 @@ class swift::ParseableInterfaceBuilder {
bool collectDepsForSerialization(CompilerInstance &SubInstance,
SmallVectorImpl<FileDependency> &Deps,
bool IsHashBased) {
StringRef SDKPath = SubInstance.getASTContext().SearchPathOpts.SDKPath;
StringRef ResourcePath =
SubInstance.getASTContext().SearchPathOpts.RuntimeResourcePath;
auto &Opts = SubInstance.getASTContext().SearchPathOpts;
SmallString<128> SDKPath(Opts.SDKPath);
path::native(SDKPath);
SmallString<128> ResourcePath(Opts.RuntimeResourcePath);
path::native(ResourcePath);

auto DTDeps = SubInstance.getDependencyTracker()->getDependencies();
SmallVector<StringRef, 16> InitialDepNames(DTDeps.begin(), DTDeps.end());
InitialDepNames.push_back(interfacePath);
llvm::StringSet<> AllDepNames;
SmallString<128> Scratch;

for (const auto &InitialDepName : InitialDepNames) {
path::native(InitialDepName, Scratch);
StringRef DepName = Scratch.str();

for (auto const &DepName : InitialDepNames) {
assert(moduleCachePath.empty() || !DepName.startswith(moduleCachePath));
assert(prebuiltCachePath.empty() || !DepName.startswith(prebuiltCachePath));

/// Lazily load the dependency buffer if we need it. If we're not
/// dealing with a hash-based dependencies, and if the dependency is
/// not a .swiftmodule, we can avoid opening the buffer.
std::unique_ptr<llvm::MemoryBuffer> DepBuf = nullptr;
auto getDepBuf = [&]() -> llvm::MemoryBuffer * {
if (DepBuf) return DepBuf.get();
if (auto Buf = getBufferOfDependency(fs, DepName, interfacePath,
diags, diagnosticLoc)) {
DepBuf = std::move(Buf);
return DepBuf.get();
}
return nullptr;
};
// Serialize the paths of dependencies in the SDK relative to it.
Optional<StringRef> SDKRelativePath = getRelativeDepPath(DepName, SDKPath);
StringRef DepNameToStore = SDKRelativePath.getValueOr(DepName);
bool IsSDKRelative = SDKRelativePath.hasValue();

// Adjust the paths of dependences in the SDK to be relative to it
bool IsSDKRelative = false;
StringRef DepNameToStore = DepName;
if (SDKPath.size() > 1 && DepName.startswith(SDKPath)) {
assert(DepName.size() > SDKPath.size() &&
"should never depend on a directory");
if (llvm::sys::path::is_separator(DepName[SDKPath.size()])) {
// Is the DepName something like ${SDKPath}/foo.h"?
DepNameToStore = DepName.substr(SDKPath.size() + 1);
IsSDKRelative = true;
} else if (llvm::sys::path::is_separator(SDKPath.back())) {
// Is the DepName something like "${SDKPath}foo.h", where SDKPath
// itself contains a trailing slash?
DepNameToStore = DepName.substr(SDKPath.size());
IsSDKRelative = true;
} else {
// We have something next to an SDK, like "Foo.sdk.h", that's somehow
// become a dependency.
}
}
// Forwarding modules add the underlying prebuilt module to their
// dependency list -- don't serialize that.
if (!prebuiltCachePath.empty() && DepName.startswith(prebuiltCachePath))
continue;

if (AllDepNames.insert(DepName).second && dependencyTracker) {
dependencyTracker->addDependency(DepName, /*isSystem*/IsSDKRelative);
Expand All @@ -425,6 +432,20 @@ class swift::ParseableInterfaceBuilder {
if (!Status)
return true;

/// Lazily load the dependency buffer if we need it. If we're not
/// dealing with a hash-based dependencies, and if the dependency is
/// not a .swiftmodule, we can avoid opening the buffer.
std::unique_ptr<llvm::MemoryBuffer> DepBuf = nullptr;
auto getDepBuf = [&]() -> llvm::MemoryBuffer * {
if (DepBuf) return DepBuf.get();
if (auto Buf = getBufferOfDependency(fs, DepName, interfacePath,
diags, diagnosticLoc)) {
DepBuf = std::move(Buf);
return DepBuf.get();
}
return nullptr;
};

if (IsHashBased) {
auto buf = getDepBuf();
if (!buf) return true;
Expand Down Expand Up @@ -687,8 +708,7 @@ class ParseableInterfaceModuleLoaderImpl {
if (!dep.isSDKRelative())
return dep.getPath();

StringRef SDKPath = ctx.SearchPathOpts.SDKPath;
scratch.assign(SDKPath.begin(), SDKPath.end());
path::native(ctx.SearchPathOpts.SDKPath, scratch);
llvm::sys::path::append(scratch, dep.getPath());
return StringRef(scratch.data(), scratch.size());
}
Expand Down Expand Up @@ -816,6 +836,10 @@ class ParseableInterfaceModuleLoaderImpl {
}
path::append(scratch, path::filename(modulePath));

// If there isn't a file at this location, skip returning a path.
if (!fs.exists(scratch))
return None;

return scratch.str();
}

Expand Down Expand Up @@ -902,7 +926,7 @@ class ParseableInterfaceModuleLoaderImpl {
return DiscoveredModule::prebuilt(*path, std::move(moduleBuffer));
} else {
LLVM_DEBUG(llvm::dbgs() << "Found out-of-date prebuilt module at "
<< modulePath << "\n");
<< path->str() << "\n");
}
}
}
Expand Down Expand Up @@ -939,40 +963,62 @@ class ParseableInterfaceModuleLoaderImpl {

/// Writes the "forwarding module" that will forward to a module in the
/// prebuilt cache.
///
/// Since forwarding modules track dependencies separately from the module
/// they point to, we'll need to grab the up-to-date file status while doing
/// this.
bool writeForwardingModule(const DiscoveredModule &mod,
StringRef outputPath,
ArrayRef<FileDependency> deps) {
/// this. If the write was successful, it also updates the
/// list of dependencies to match what was written to the forwarding file.
bool writeForwardingModuleAndUpdateDeps(
const DiscoveredModule &mod, StringRef outputPath,
SmallVectorImpl<FileDependency> &deps) {
assert(mod.isPrebuilt() &&
"cannot write forwarding file for non-prebuilt module");
ForwardingModule fwd(mod.path);

SmallVector<FileDependency, 16> depsAdjustedToMTime;

// FIXME: We need to avoid re-statting all these dependencies, otherwise
// we may record out-of-date information.
auto addDependency = [&](StringRef path) {
auto addDependency = [&](StringRef path) -> FileDependency {
auto status = fs.status(path);
uint64_t mtime =
status->getLastModificationTime().time_since_epoch().count();
fwd.addDependency(path, status->getSize(), mtime);

// Construct new FileDependency matching what we've added to the
// forwarding module. This is no longer SDK-relative because the absolute
// path has already been resolved.
return FileDependency::modTimeBased(path, /*isSDKRelative*/false,
status->getSize(), mtime);
};

// Add the prebuilt module as a dependency of the forwarding module.
addDependency(fwd.underlyingModulePath);
// Add the prebuilt module as a dependency of the forwarding module, but
// don't add it to the outer dependency list.
(void)addDependency(fwd.underlyingModulePath);

// Add all the dependencies from the prebuilt module.
// Add all the dependencies from the prebuilt module, and update our list
// of dependencies to reflect what's recorded in the forwarding module.
SmallString<128> SDKRelativeBuffer;
for (auto dep : deps) {
addDependency(getFullDependencyPath(dep, SDKRelativeBuffer));
auto adjustedDep =
addDependency(getFullDependencyPath(dep, SDKRelativeBuffer));
depsAdjustedToMTime.push_back(adjustedDep);
}

return withOutputFile(diags, outputPath,
auto hadError = withOutputFile(diags, outputPath,
[&](llvm::raw_pwrite_stream &out) {
llvm::yaml::Output yamlWriter(out);
yamlWriter << fwd;
return false;
});

if (hadError)
return true;

// If and only if we succeeded writing the forwarding file, update the
// provided list of dependencies.
deps = depsAdjustedToMTime;
return false;
}

/// Looks up the best module to load for a given interface, and returns a
Expand Down Expand Up @@ -1021,17 +1067,20 @@ class ParseableInterfaceModuleLoaderImpl {
if (moduleOrErr) {
auto module = std::move(moduleOrErr.get());

// If it's prebuilt, use this time to generate a forwarding module.
// If it's prebuilt, use this time to generate a forwarding module and
// update the dependencies to use modification times.
if (module.isPrebuilt())
if (writeForwardingModule(module, cachedOutputPath, allDeps))
if (writeForwardingModuleAndUpdateDeps(module, cachedOutputPath,
allDeps))
return std::make_error_code(std::errc::not_supported);

// Report the module's dependencies to the dependencyTracker
if (dependencyTracker) {
SmallString<128> SDKRelativeBuffer;
for (auto &dep: allDeps) {
StringRef fullPath = getFullDependencyPath(dep, SDKRelativeBuffer);
dependencyTracker->addDependency(fullPath, dep.isSDKRelative());
dependencyTracker->addDependency(fullPath,
/*IsSystem=*/dep.isSDKRelative());
}
}

Expand Down
56 changes: 56 additions & 0 deletions test/ParseableInterface/ModuleCache/forwarding-module-dependencies.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// RUN: %empty-directory(%t)
// RUN: %empty-directory(%t/mock-sdk)
// RUN: %empty-directory(%t/ModuleCache)
// RUN: %empty-directory(%t/PrebuiltModuleCache)

// This test makes sure that we propagate the right set of dependencies from a
// prebuilt module to its corresponding forwarding module.

// Setup. Copy the mock SDK into the tmp directory.
// RUN: cp -r %S/Inputs/mock-sdk/* %t/mock-sdk/.

// 1. Compile ExportedLib.swiftinterface, which a) is in the SDK, and b) depends
// on a C module with headers that should be in the dependency list.
// Put it in the prebuilt cache.

// RUN: %target-swift-frontend -build-module-from-parseable-interface %t/mock-sdk/ExportedLib.swiftinterface -sdk %t/mock-sdk -o %t/PrebuiltModuleCache/ExportedLib.swiftmodule -serialize-parseable-module-interface-dependency-hashes -track-system-dependencies

// 2. Make sure the prebuilt module we built has SomeCModule.h as a dependency.

// RUN: llvm-bcanalyzer -dump %t/PrebuiltModuleCache/ExportedLib.swiftmodule | grep 'FILE_DEPENDENCY.*SomeCModule.h'

// 3. Typecheck this file, which imports SdkLib, which imports ExportedLib.
// Because ExportedLib is prebuilt, we expect a forwarding module for
// ExportedLib in the module cache, and a serialized module for SdkLib in
// the cache.

// RUN: %target-swift-frontend -typecheck %s -prebuilt-module-cache-path %t/PrebuiltModuleCache -module-cache-path %t/ModuleCache -sdk %t/mock-sdk -I %t/mock-sdk -track-system-dependencies

// 4. Make sure the forwarding module is installed in the cache.

// RUN: %{python} %S/Inputs/check-is-forwarding-module.py %t/ModuleCache/ExportedLib-*.swiftmodule

// 5. Make sure the forwarding module depends on the prebuilt module and the C
// header.

// RUN: grep ' *path:.*ExportedLib.swiftmodule' %t/ModuleCache/ExportedLib-*.swiftmodule
// RUN: grep ' *path:.*SomeCModule.h' %t/ModuleCache/ExportedLib-*.swiftmodule

// 6. Make sure the dependencies from the forwarding module make it into the
// cached module.

// RUN: llvm-bcanalyzer -dump %t/ModuleCache/SdkLib-*.swiftmodule | grep 'FILE_DEPENDENCY.*SomeCModule.h'

// 7. Make sure the prebuilt ExportedLib module did NOT get propagated to SdkLib.

// RUN: llvm-bcanalyzer -dump %t/ModuleCache/SdkLib-*.swiftmodule | not grep 'FILE_DEPENDENCY.*PrebuiltModuleCache'

// 8. Make sure we re-build the SdkLib module if one of the dependencies changes its mtime (but not its hash).
// This will ensure we recorded the forwarding module's dependencies, not the prebuilt.

// RUN: %{python} %S/Inputs/make-old.py %t/ModuleCache/SdkLib-*.swiftmodule
// RUN: %{python} %S/Inputs/make-old.py %t/mock-sdk/usr/include/SomeCModule.h
// RUN: %target-swift-frontend -typecheck %s -prebuilt-module-cache-path %t/PrebuiltModuleCache -module-cache-path %t/ModuleCache -sdk %t/mock-sdk -I %t/mock-sdk -track-system-dependencies
// RUN: %{python} %S/Inputs/check-is-new.py %t/ModuleCache/SdkLib-*.swiftmodule

import SdkLib