[lldb] Fix clang importer for swift caching debugging (attempt #2)

cachemeifyoucan · cachemeifyoucan · commit d3b9f2cc2d33 · 2024-09-17T09:27:36.000-07:00
Further improve how lldb constructing clang importer to utilize explicit
modules:
* Add logics to initialize the swift ASTContext from direct cc1
  arguments embedded in the swiftmodule
* Make sure the direct cc1 arguments are not repeated constructed. It
  should only be constructed as a fresh invocation
* Make sure the lldb doesn't overwrite the explicit module build
  settings when filter arguments. This ensures the already built
  explicit modules are used.
* Use the newly added clang option that ignore CAS info when loading
  PCMs. This allows lldb to load CAS enabled PCM directly from disk.

rdar://135611011
diff --git a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp
@@ -57,6 +57,7 @@
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Lex/Preprocessor.h"
 
 #include "clang/Lex/PreprocessorOptions.h"
@@ -1573,21 +1574,7 @@ bool ShouldUnique(StringRef arg) {
 
 // static
 void SwiftASTContext::AddExtraClangArgs(const std::vector<std::string> &source,
-                                        std::vector<std::string> &dest,
-                                        bool cc1) {
-  // FIXME: Support for cc1 flags isn't complete.  The uniquing
-  // algortihm below does not work for cc1 flags. Since cc1 flags are
-  // not stable it's not feasible to keep a list of all multi-arg
-  // flags, for example. It also makes it difficult to correctly
-  // identify where workng directories and path remappings should
-  // applied. For all these reasons, using cc1 flags for anything but
-  // a local build with explicit modules and precise compiler
-  // invocations isn't supported yet.
-  if (cc1) {
-    dest.insert(dest.end(), source.begin(), source.end());
-    return;
-  }
-
+                                        std::vector<std::string> &dest) {
   llvm::StringSet<> unique_flags;
   for (auto &arg : dest)
     unique_flags.insert(arg);
@@ -1764,23 +1751,37 @@ static void applyOverrideOptions(std::vector<std::string> &args,
 
 void SwiftASTContext::AddExtraClangArgs(
     const std::vector<std::string> &ExtraArgs, StringRef overrideOpts) {
+  if (ExtraArgs.empty())
+    return;
+
   swift::ClangImporterOptions &importer_options = GetClangImporterOptions();
 
   // Detect cc1 flags.  When DirectClangCC1ModuleBuild is on then the
   // clang arguments in the serialized invocation are clang cc1 flags,
   // which are very specific to one compiler version and cannot
   // be merged with driver options.
   bool fresh_invocation = importer_options.ExtraArgs.empty();
-  if (fresh_invocation && !ExtraArgs.empty() && ExtraArgs.front() == "-cc1")
-    importer_options.DirectClangCC1ModuleBuild = true;
-  if (!importer_options.DirectClangCC1ModuleBuild && !ExtraArgs.empty() &&
-      ExtraArgs.front() == "-cc1")
+  bool invocation_direct_cc1 = ExtraArgs.front() == "-cc1";
+
+  // If it is not a fresh invocation, make sure the cc1 option matches.
+  if (!fresh_invocation &&
+      (importer_options.DirectClangCC1ModuleBuild != invocation_direct_cc1))
     AddDiagnostic(
         eSeverityWarning,
         "Mixing and matching of driver and cc1 Clang options detected");
 
-  AddExtraClangArgs(ExtraArgs, importer_options.ExtraArgs,
-                    importer_options.DirectClangCC1ModuleBuild);
+  importer_options.DirectClangCC1ModuleBuild = invocation_direct_cc1;
+
+  // If using direct cc1 flags, compute the arguments and return.
+  // Since this is cc1 flags, override options are driver flags and don't apply.
+  if (importer_options.DirectClangCC1ModuleBuild) {
+    if (!fresh_invocation)
+      importer_options.ExtraArgs.clear();
+    AddExtraClangCC1Args(ExtraArgs, importer_options.ExtraArgs);
+    return;
+  }
+
+  AddExtraClangArgs(ExtraArgs, importer_options.ExtraArgs);
   applyOverrideOptions(importer_options.ExtraArgs, overrideOpts);
   if (HasNonexistentExplicitModule(importer_options.ExtraArgs))
     RemoveExplicitModules(importer_options.ExtraArgs);
@@ -1792,6 +1793,78 @@ void SwiftASTContext::AddExtraClangArgs(
       });
 }
 
+void SwiftASTContext::AddExtraClangCC1Args(
+    const std::vector<std::string> &source, std::vector<std::string> &dest) {
+  clang::CompilerInvocation invocation;
+  llvm::SmallVector<const char *> clangArgs;
+  clangArgs.reserve(source.size());
+  llvm::for_each(source, [&](const std::string &Arg) {
+    // Workaround for the extra driver argument embedded in the swiftmodule by
+    // some swift compiler version. It always starts with `--target=` and it is
+    // not a valid cc1 option.
+    if (!StringRef(Arg).starts_with("--target="))
+      clangArgs.push_back(Arg.c_str());
+  });
+
+  std::string diags;
+  llvm::raw_string_ostream os(diags);
+  auto diagOpts = llvm::makeIntrusiveRefCnt<clang::DiagnosticOptions>();
+  clang::DiagnosticsEngine clangDiags(
+      new clang::DiagnosticIDs(), diagOpts,
+      new clang::TextDiagnosticPrinter(os, diagOpts.get()));
+
+  if (!clang::CompilerInvocation::CreateFromArgs(invocation, clangArgs,
+                                                 clangDiags)) {
+    // If cc1 arguments failed to parse, report diagnostics and return
+    // immediately.
+    AddDiagnostic(eSeverityError, diags);
+    // Disable direct-cc1 build as fallback.
+    GetClangImporterOptions().DirectClangCC1ModuleBuild = false;
+    return;
+  }
+
+  // Clear module cache key and other CAS options to load modules from disk
+  // directly.
+  invocation.getFrontendOpts().ModuleCacheKeys.clear();
+  invocation.getCASOpts() = clang::CASOptions();
+
+  // Ignore CAS info inside modules when loading.
+  invocation.getFrontendOpts().ModuleLoadIgnoreCAS = true;
+
+  // Remove non-existing modules in a systematic way.
+  bool module_missing = false;
+  auto CheckFileExists = [&](const char *file) {
+    if (!llvm::sys::fs::exists(file)) {
+      std::string warn;
+      llvm::raw_string_ostream(warn)
+          << "Nonexistent explicit module file " << file;
+      AddDiagnostic(eSeverityWarning, warn);
+      module_missing = true;
+    }
+  };
+  llvm::for_each(invocation.getHeaderSearchOpts().PrebuiltModuleFiles,
+                 [&](const auto &mod) { CheckFileExists(mod.second.c_str()); });
+  llvm::for_each(invocation.getFrontendOpts().ModuleFiles,
+                 [&](const auto &mod) { CheckFileExists(mod.c_str()); });
+
+  // If missing, clear all the prebuilt module options and switch to implicit
+  // modules build.
+  if (module_missing) {
+    invocation.getHeaderSearchOpts().PrebuiltModuleFiles.clear();
+    invocation.getFrontendOpts().ModuleFiles.clear();
+    invocation.getLangOpts().ImplicitModules = true;
+    invocation.getHeaderSearchOpts().ImplicitModuleMaps = true;
+  }
+
+  invocation.generateCC1CommandLine(
+      [&](const llvm::Twine &arg) { dest.push_back(arg.str()); });
+
+  // If cc1 arguments are parsed and generated correctly, set explicitly-built
+  // module since only explicit module build can use direct cc1 mode.
+  m_has_explicit_modules = true;
+  return;
+}
+
 void SwiftASTContext::AddUserClangArgs(TargetProperties &props) {
   Args args(props.GetSwiftExtraClangFlags());
   std::vector<std::string> user_clang_flags;
@@ -1883,6 +1956,10 @@ void SwiftASTContext::RemapClangImporterOptions(
 
 void SwiftASTContext::FilterClangImporterOptions(
     std::vector<std::string> &extra_args, SwiftASTContext *ctx) {
+  // The direct cc1 mode do not need any extra audit.
+  if (ctx && ctx->GetClangImporterOptions().DirectClangCC1ModuleBuild)
+    return;
+
   std::string ivfs_arg;
   // Copy back a filtered version of ExtraArgs.
   std::vector<std::string> orig_args(std::move(extra_args));
@@ -1893,11 +1970,6 @@ void SwiftASTContext::FilterClangImporterOptions(
         arg_sr == "-fno-implicit-module-maps")
       continue;
 
-    // This is not a cc1 option.
-    if (arg_sr.starts_with("--target=") && ctx &&
-        ctx->GetClangImporterOptions().DirectClangCC1ModuleBuild)
-      continue;
-
     // The VFS options turn into fatal errors when the referenced file
     // is not found. Since the Xcode build system tends to create a
     // lot of VFS overlays by default, stat them and emit a warning if
@@ -2184,6 +2256,11 @@ ProcessModule(Module &module, std::string m_description,
   for (auto path : opts.getFrameworkSearchPaths())
     framework_search_paths.push_back({path.Path, path.IsSystem});
   auto &clang_opts = invocation.getClangImporterOptions().ExtraArgs;
+  // If the args embedded are cc1 args, they are not compatible with existing
+  // setting. Clear the previous args.
+  if (!clang_opts.empty() && clang_opts.front() == "-cc1")
+    extra_clang_args.clear();
+
   for (const std::string &arg : clang_opts) {
     extra_clang_args.push_back(arg);
     LOG_VERBOSE_PRINTF(GetLog(LLDBLog::Types), "adding Clang argument \"%s\".",
diff --git a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.h b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.h
@@ -278,8 +278,10 @@ class SwiftASTContext : public TypeSystemSwift {
   /// apply the working directory to any relative paths.
   void AddExtraClangArgs(const std::vector<std::string> &ExtraArgs,
                          llvm::StringRef overrideOpts = "");
+  void AddExtraClangCC1Args(const std::vector<std::string>& source,
+                                std::vector<std::string>& dest);
   static void AddExtraClangArgs(const std::vector<std::string>& source,
-                                std::vector<std::string>& dest, bool cc1);
+                                std::vector<std::string>& dest);
   static std::string GetPluginServer(llvm::StringRef plugin_library_path);
   /// Removes nonexisting VFS overlay options.
   static void FilterClangImporterOptions(std::vector<std::string> &extra_args,
diff --git a/lldb/test/API/lang/swift/clangimporter/caching/Makefile b/lldb/test/API/lang/swift/clangimporter/caching/Makefile
@@ -0,0 +1,5 @@
+SWIFT_SOURCES := main.swift
+SWIFT_ENABLE_EXPLICIT_MODULES := YES
+SWIFTFLAGS_EXTRAS = -I$(SRCDIR) -cache-compile-job -cas-path $(BUILDDIR)/cas
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/swift/clangimporter/caching/TestSwiftClangImporterCaching.py b/lldb/test/API/lang/swift/clangimporter/caching/TestSwiftClangImporterCaching.py
@@ -0,0 +1,34 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+import unittest2
+
+class TestSwiftClangImporterCaching(TestBase):
+
+    NO_DEBUG_INFO_TESTCASE = True
+
+    # Don't run ClangImporter tests if Clangimporter is disabled.
+    @skipIf(setting=('symbols.use-swift-clangimporter', 'false'))
+    @skipIf(setting=('symbols.swift-precise-compiler-invocation', 'false'))
+    @skipIf(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true'), bugnumber='rdar://128094135')
+    @skipUnlessDarwin
+    @swiftTest
+    def test(self):
+        """
+        Test flipping on/off implicit modules.
+        """
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "break here",
+                                          lldb.SBFileSpec('main.swift'))
+        log = self.getBuildArtifact("types.log")
+        self.expect('log enable lldb types -f "%s"' % log)
+        self.expect("expression obj", DATA_TYPES_DISPLAYED_CORRECTLY,
+                    substrs=["b ="])
+        self.filecheck('platform shell cat "%s"' % log, __file__)
+### -cc1 should be round-tripped so there is no more `-cc1` in the extra args. Look for `-triple` which is a cc1 flag.
+#       CHECK:  SwiftASTContextForExpressions(module: "a", cu: "main.swift")::LogConfiguration() --     -triple
+#       CHECK:  SwiftASTContextForExpressions(module: "a", cu: "main.swift") Module import remark: loaded module 'ClangA'
+#       CHECK-NOT: -cc1
+#       CHECK-NOT: -fmodule-file-cache-key
+#       CHECK-NOT: Clang error:
diff --git a/lldb/test/API/lang/swift/clangimporter/caching/a.h b/lldb/test/API/lang/swift/clangimporter/caching/a.h
@@ -0,0 +1,5 @@
+#include "b.h"
+
+struct A {
+  struct B b;
+};
diff --git a/lldb/test/API/lang/swift/clangimporter/caching/b.h b/lldb/test/API/lang/swift/clangimporter/caching/b.h
@@ -0,0 +1 @@
+struct B {};
diff --git a/lldb/test/API/lang/swift/clangimporter/caching/main.swift b/lldb/test/API/lang/swift/clangimporter/caching/main.swift
@@ -0,0 +1,3 @@
+import ClangA
+let obj = A()
+print("break here \(obj)")
diff --git a/lldb/test/API/lang/swift/clangimporter/caching/module.modulemap b/lldb/test/API/lang/swift/clangimporter/caching/module.modulemap
@@ -0,0 +1,7 @@
+module ClangA {
+  header "a.h"
+}
+
+module ClangB {
+  header "b.h"
+}
diff --git a/lldb/test/API/lang/swift/clangimporter/explicit_cc1/TestSwiftClangImporterExplicitCC1.py b/lldb/test/API/lang/swift/clangimporter/explicit_cc1/TestSwiftClangImporterExplicitCC1.py
@@ -26,4 +26,6 @@ def test(self):
         self.expect("expression obj", DATA_TYPES_DISPLAYED_CORRECTLY,
                     substrs=["b ="])
         self.filecheck('platform shell cat "%s"' % log, __file__)
-#       CHECK:  SwiftASTContextForExpressions(module: "a", cu: "main.swift")::LogConfiguration() --     -cc1
+### -cc1 should be round-tripped so there is no more `-cc1` in the extra args. Look for `-triple` which is a cc1 flag.
+#       CHECK:  SwiftASTContextForExpressions(module: "a", cu: "main.swift")::LogConfiguration() --     -triple
+#       CHECK-NOT: -cc1
diff --git a/lldb/test/Shell/Swift/caching.test b/lldb/test/Shell/Swift/caching.test
@@ -0,0 +1,30 @@
+# REQUIRES: swift
+# REQUIRES: system-darwin
+
+# RUN: rm -rf %t && mkdir %t
+# RUN: split-file %s %t
+# RUN: %target-swiftc -g -Onone -save-temps \
+# RUN:          -module-cache-path %t/cache %t/main.swift \
+# RUN:          -cache-compile-job -cas-path %t/cas -explicit-module-build \
+# RUN:          -module-name main -o %t/main
+
+# RUN: %lldb %t/main -s %t/lldb.script 2>&1 | FileCheck %s
+# CHECK:         LogConfiguration() --   Extra clang arguments
+# CHECK-COUNT-1: LogConfiguration() --     -triple
+# CHECK:         (Int) ${{.*}} = 1
+
+//--- main.swift
+func test() {
+  print("break here")
+}
+test()
+
+//--- lldb.script
+# Force loading from interface to simulate no binary module available.
+settings set symbols.swift-module-loading-mode prefer-interface
+log enable lldb types
+b test
+run
+# Create a SwiftASTContext
+expr 1
+quit
diff --git a/lldb/unittests/Symbol/TestSwiftASTContext.cpp b/lldb/unittests/Symbol/TestSwiftASTContext.cpp
@@ -234,15 +234,15 @@ const std::vector<std::string> uniqued_flags = {
 TEST_F(ClangArgs, UniquingCollisionWithExistingFlags) {
   const std::vector<std::string> source = duplicated_flags;
   std::vector<std::string> dest = uniqued_flags;
-  SwiftASTContext::AddExtraClangArgs(source, dest, false);
+  SwiftASTContext::AddExtraClangArgs(source, dest);
 
   EXPECT_EQ(dest, uniqued_flags);
 }
 
 TEST_F(ClangArgs, UniquingCollisionWithAddedFlags) {
   const std::vector<std::string> source = duplicated_flags;
   std::vector<std::string> dest;
-  SwiftASTContext::AddExtraClangArgs(source, dest, false);
+  SwiftASTContext::AddExtraClangArgs(source, dest);
 
   EXPECT_EQ(dest, uniqued_flags);
 }
@@ -251,7 +251,7 @@ TEST_F(ClangArgs, DoubleDash) {
   // -v with all currently ignored arguments following.
   const std::vector<std::string> source{"-v", "--", "-Werror", ""};
   std::vector<std::string> dest;
-  SwiftASTContext::AddExtraClangArgs(source, dest, false);
+  SwiftASTContext::AddExtraClangArgs(source, dest);
 
   // Check that all ignored arguments got removed.
   EXPECT_EQ(dest, std::vector<std::string>({"-v"}));

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+import ClangA`
	`2`	`+let obj = A()`
	`3`	`+print("break here \(obj)")`