From 534c7a95698429947177f8ec486f5318dfde6dba Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Fri, 2 Aug 2024 14:21:17 -0700 Subject: [PATCH] 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 --- .../TypeSystem/Swift/SwiftASTContext.cpp | 50 ++++++------------- ...estSwiftClangImporterExplicitNoImplicit.py | 2 - .../private_import/TestSwiftPrivateImport.py | 29 +++++++---- 3 files changed, 32 insertions(+), 49 deletions(-) diff --git a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp index 9211db9c185e2..e520cd2ea3a4b 100644 --- a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp +++ b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp @@ -1018,6 +1018,20 @@ void SwiftASTContext::SetCompilerInvocationLLDBOverrides() { swift::LangOptions &lang_opts = m_compiler_invocation_ap->getLangOptions(); lang_opts.AllowDeserializingImplementationOnly = true; lang_opts.DebuggerSupport = true; + + // 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. It + // 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. + bool is_precise = ModuleList::GetGlobalModuleListProperties() + .GetUseSwiftPreciseCompilerInvocation(); + lang_opts.ImportNonPublicDependencies = is_precise ? false : true; // When loading Swift types that conform to ObjC protocols that have // been renamed with NS_SWIFT_NAME the DwarfImporterDelegate will crash // during protocol conformance checks as the underlying type cannot be @@ -9212,42 +9226,6 @@ bool SwiftASTContext::GetCompileUnitImportsImpl( LOG_PRINTF(GetLog(LLDBLog::Types), "Importing dependencies of current CU"); - // Turn off implicit clang modules while importing CU dependencies. - // 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. To - // avoid these costly and potentially dangerous imports we turn off - // implicit modules while importing the CU imports only. If a user - // manually evaluates an expression that contains an import - // statement that can still trigger an implict import. Implicit - // imports can be dangerous if an implicit module depends on a - // module that also exists as an explicit input: In this case, a - // subsequent explicit import of said dependency will error because - // Clang now knows about two versions of the same module. - clang::LangOptions *clang_lang_opts = nullptr; - auto reset = llvm::make_scope_exit([&] { - if (clang_lang_opts) { - LOG_PRINTF(GetLog(LLDBLog::Types), "Turning on implicit Clang modules"); - clang_lang_opts->ImplicitModules = true; - } - }); - if (auto *clang_importer = GetClangImporter()) { - if (m_has_explicit_modules) { - auto &clang_instance = const_cast( - clang_importer->getClangInstance()); - clang_lang_opts = &clang_instance.getLangOpts(); - // AddExtraArgs is supposed to always turn implicit modules on. - assert(clang_lang_opts->ImplicitModules && - "ClangImporter implicit module support is off"); - LOG_PRINTF(GetLog(LLDBLog::Types), "Turning off implicit Clang modules"); - clang_lang_opts->ImplicitModules = false; - } - } - std::string category = "Importing Swift module dependencies for "; category += compile_unit->GetPrimaryFile().GetFilename(); Progress progress(category, "", cu_imports.size()); diff --git a/lldb/test/API/lang/swift/clangimporter/explict_noimplicit/TestSwiftClangImporterExplicitNoImplicit.py b/lldb/test/API/lang/swift/clangimporter/explict_noimplicit/TestSwiftClangImporterExplicitNoImplicit.py index 46c7ed9a2b71a..c4de05b61eba3 100644 --- a/lldb/test/API/lang/swift/clangimporter/explict_noimplicit/TestSwiftClangImporterExplicitNoImplicit.py +++ b/lldb/test/API/lang/swift/clangimporter/explict_noimplicit/TestSwiftClangImporterExplicitNoImplicit.py @@ -30,6 +30,4 @@ def test(self): substrs=["hidden"]) self.filecheck('platform shell cat "%s"' % log, __file__) # CHECK-NOT: IMPLICIT-CLANG-MODULE-CACHE/{{.*}}/SwiftShims-{{.*}}.pcm -# CHECK: Turning off implicit Clang modules # CHECK: IMPLICIT-CLANG-MODULE-CACHE/{{.*}}/Hidden-{{.*}}.pcm -# CHECK: Turning on implicit Clang modules diff --git a/lldb/test/API/lang/swift/private_import/TestSwiftPrivateImport.py b/lldb/test/API/lang/swift/private_import/TestSwiftPrivateImport.py index 126ed1d1c85f1..8d467d0515cf4 100644 --- a/lldb/test/API/lang/swift/private_import/TestSwiftPrivateImport.py +++ b/lldb/test/API/lang/swift/private_import/TestSwiftPrivateImport.py @@ -16,17 +16,24 @@ def test_private_import(self): os.unlink(self.getBuildArtifact("Invisible.swiftmodule")) os.unlink(self.getBuildArtifact("Invisible.swiftinterface")) - if lldb.remote_platform: - wd = lldb.remote_platform.GetWorkingDirectory() - filename = 'libInvisible.dylib' - err = lldb.remote_platform.Put( - lldb.SBFileSpec(self.getBuildArtifact(filename)), - lldb.SBFileSpec(os.path.join(wd, filename))) - self.assertFalse(err.Fail(), 'Failed to copy ' + filename) - + log = self.getBuildArtifact("types.log") + self.expect('log enable lldb types -f "%s"' % log) lldbutil.run_to_source_breakpoint( self, 'break here', lldb.SBFileSpec('main.swift'), - extra_images=['Library']) + extra_images=['Library', 'Invisible']) + + precise = self.dbg.GetSetting('symbols.swift-precise-compiler-invocation').GetBooleanValue() + if precise: + # Test that importing the expression context (i.e., the module + # "a") pulls in the explicit dependencies, but not their + # private imports. This comes before the other checks, + # because type reconstruction will still trigger an import of + # the "Invisible" module that we can't prevent later one. + self.expect("expression 1+1") + self.filecheck('platform shell cat "%s"' % log, __file__) +# CHECK: Module import remark: loaded module 'Library' +# CHECK-NOT: Module import remark: loaded module 'Invisible' + self.expect("fr var -d run -- x", substrs=["(Invisible.InvisibleStruct)"]) - # FIXME: This crashes LLDB with a Swift DESERIALIZATION FAILURE. - # self.expect("fr var -d run -- y", substrs=["(Any)"]) + self.expect("fr var -d run -- y", substrs=["(Library.Conforming)", + "conforming"])