Skip to content

Commit b578838

Browse files
committed
Disable the import of private dependencies when using precise invocations.
ModuleFileSharedCore::getTransitiveLoadingBehavior() has a best-effort mode that is enabled when debugger support is turned on that will try to import implementation-only imports of Swift modules, but won't treat import failures as errors. When explicit modules are on, this has the unwanted side-effect of potentially triggering an implicit Clang module build if one of the internal dependencies of a library was not used to build the target. I previously fixed this problem by turning off implicit modules while loading the context dependencies. However, we discovered that the transitive loading of private dependencies can also lead to additional Swift modules being pulled in that through their dependencies can lead to dependency cycles that were not a problem at build time. Therefore, a simpler solution to both problem is turning of private dependency import altogether when using precise compiler invocations. Note that private dependencies can still sneak into a context via type reconstruction, which can also trigger module imports. rdar://132360374 (cherry picked from commit 534c7a9)
1 parent 2fec321 commit b578838

File tree

3 files changed

+32
-49
lines changed

3 files changed

+32
-49
lines changed

lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp

Lines changed: 14 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,6 +1017,20 @@ void SwiftASTContext::SetCompilerInvocationLLDBOverrides() {
10171017
swift::LangOptions &lang_opts = m_compiler_invocation_ap->getLangOptions();
10181018
lang_opts.AllowDeserializingImplementationOnly = true;
10191019
lang_opts.DebuggerSupport = true;
1020+
1021+
// ModuleFileSharedCore::getTransitiveLoadingBehavior() has a
1022+
// best-effort mode that is enabled when debugger support is turned
1023+
// on that will try to import implementation-only imports of Swift
1024+
// modules, but won't treat import failures as errors. When explicit
1025+
// modules are on, this has the unwanted side-effect of potentially
1026+
// triggering an implicit Clang module build if one of the internal
1027+
// dependencies of a library was not used to build the target. It
1028+
// can also lead to additional Swift modules being pulled in that
1029+
// through their dependencies can lead to dependency cycles that
1030+
// were not a problem at build time.
1031+
bool is_precise = ModuleList::GetGlobalModuleListProperties()
1032+
.GetUseSwiftPreciseCompilerInvocation();
1033+
lang_opts.ImportNonPublicDependencies = is_precise ? false : true;
10201034
// When loading Swift types that conform to ObjC protocols that have
10211035
// been renamed with NS_SWIFT_NAME the DwarfImporterDelegate will crash
10221036
// during protocol conformance checks as the underlying type cannot be
@@ -9211,42 +9225,6 @@ bool SwiftASTContext::GetCompileUnitImportsImpl(
92119225

92129226
LOG_PRINTF(GetLog(LLDBLog::Types), "Importing dependencies of current CU");
92139227

9214-
// Turn off implicit clang modules while importing CU dependencies.
9215-
// ModuleFileSharedCore::getTransitiveLoadingBehavior() has a
9216-
// best-effort mode that is enabled when debugger support is turned
9217-
// on that will try to import implementation-only imports of Swift
9218-
// modules, but won't treat import failures as errors. When explicit
9219-
// modules are on, this has the unwanted side-effect of potentially
9220-
// triggering an implicit Clang module build if one of the internal
9221-
// dependencies of a library was not used to build the target. To
9222-
// avoid these costly and potentially dangerous imports we turn off
9223-
// implicit modules while importing the CU imports only. If a user
9224-
// manually evaluates an expression that contains an import
9225-
// statement that can still trigger an implict import. Implicit
9226-
// imports can be dangerous if an implicit module depends on a
9227-
// module that also exists as an explicit input: In this case, a
9228-
// subsequent explicit import of said dependency will error because
9229-
// Clang now knows about two versions of the same module.
9230-
clang::LangOptions *clang_lang_opts = nullptr;
9231-
auto reset = llvm::make_scope_exit([&] {
9232-
if (clang_lang_opts) {
9233-
LOG_PRINTF(GetLog(LLDBLog::Types), "Turning on implicit Clang modules");
9234-
clang_lang_opts->ImplicitModules = true;
9235-
}
9236-
});
9237-
if (auto *clang_importer = GetClangImporter()) {
9238-
if (m_has_explicit_modules) {
9239-
auto &clang_instance = const_cast<clang::CompilerInstance &>(
9240-
clang_importer->getClangInstance());
9241-
clang_lang_opts = &clang_instance.getLangOpts();
9242-
// AddExtraArgs is supposed to always turn implicit modules on.
9243-
assert(clang_lang_opts->ImplicitModules &&
9244-
"ClangImporter implicit module support is off");
9245-
LOG_PRINTF(GetLog(LLDBLog::Types), "Turning off implicit Clang modules");
9246-
clang_lang_opts->ImplicitModules = false;
9247-
}
9248-
}
9249-
92509228
std::string category = "Importing Swift module dependencies for ";
92519229
category += compile_unit->GetPrimaryFile().GetFilename();
92529230
Progress progress(category, "", cu_imports.size());

lldb/test/API/lang/swift/clangimporter/explict_noimplicit/TestSwiftClangImporterExplicitNoImplicit.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,4 @@ def test(self):
3030
substrs=["hidden"])
3131
self.filecheck('platform shell cat "%s"' % log, __file__)
3232
# CHECK-NOT: IMPLICIT-CLANG-MODULE-CACHE/{{.*}}/SwiftShims-{{.*}}.pcm
33-
# CHECK: Turning off implicit Clang modules
3433
# CHECK: IMPLICIT-CLANG-MODULE-CACHE/{{.*}}/Hidden-{{.*}}.pcm
35-
# CHECK: Turning on implicit Clang modules

lldb/test/API/lang/swift/private_import/TestSwiftPrivateImport.py

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,24 @@ def test_private_import(self):
1616
os.unlink(self.getBuildArtifact("Invisible.swiftmodule"))
1717
os.unlink(self.getBuildArtifact("Invisible.swiftinterface"))
1818

19-
if lldb.remote_platform:
20-
wd = lldb.remote_platform.GetWorkingDirectory()
21-
filename = 'libInvisible.dylib'
22-
err = lldb.remote_platform.Put(
23-
lldb.SBFileSpec(self.getBuildArtifact(filename)),
24-
lldb.SBFileSpec(os.path.join(wd, filename)))
25-
self.assertFalse(err.Fail(), 'Failed to copy ' + filename)
26-
19+
log = self.getBuildArtifact("types.log")
20+
self.expect('log enable lldb types -f "%s"' % log)
2721
lldbutil.run_to_source_breakpoint(
2822
self, 'break here', lldb.SBFileSpec('main.swift'),
29-
extra_images=['Library'])
23+
extra_images=['Library', 'Invisible'])
24+
25+
precise = self.dbg.GetSetting('symbols.swift-precise-compiler-invocation').GetBooleanValue()
26+
if precise:
27+
# Test that importing the expression context (i.e., the module
28+
# "a") pulls in the explicit dependencies, but not their
29+
# private imports. This comes before the other checks,
30+
# because type reconstruction will still trigger an import of
31+
# the "Invisible" module that we can't prevent later one.
32+
self.expect("expression 1+1")
33+
self.filecheck('platform shell cat "%s"' % log, __file__)
34+
# CHECK: Module import remark: loaded module 'Library'
35+
# CHECK-NOT: Module import remark: loaded module 'Invisible'
36+
3037
self.expect("fr var -d run -- x", substrs=["(Invisible.InvisibleStruct)"])
31-
# FIXME: This crashes LLDB with a Swift DESERIALIZATION FAILURE.
32-
# self.expect("fr var -d run -- y", substrs=["(Any)"])
38+
self.expect("fr var -d run -- y", substrs=["(Library.Conforming)",
39+
"conforming"])

0 commit comments

Comments
 (0)