Make the clang module cache setting available without a target

[aprantl]

Summary

It turns out that setting the clang module cache after LLDB has a Target can be too late. In particular, the Swift language plugin needs to know the setting without having access to a Target. This patch moves the setting into the *LLDB* module cache, where it is a global setting that is available before any Target is created and more importantly, is shared between all Targets.

rdar://problem/37944432

[jingham]

This looks fine. Did you have a way to test it?

[aprantl]

The entire testsuite depends on being able to set this option (see the change in packages/Python/lldbsuite/test/lldbtest.py), but we don't verify that the directory is being used. I'll see if I can come up with a dedicated test.

[aprantl]

Added a testcase.

[zturner]

This change has introduced a dependency from Core -> clang Driver (due to
#include "clang/Driver/Driver.h" in ModuleList.cpp). Can you please try to
find a way to remove this dependency?

lldb/trunk/include/lldb/Core/ModuleList.h

 //===-- ModuleList.h --------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_ModuleList_h_
 #define liblldb_ModuleList_h_
 
 #include "lldb/Core/Address.h"     // for Address
 #include "lldb/Core/ModuleSpec.h"  // for ModuleSpec
+#include "lldb/Core/UserSettingsController.h"
 #include "lldb/Utility/FileSpec.h" // for FileSpec
 #include "lldb/Utility/Iterable.h"
 #include "lldb/Utility/Status.h" // for Status
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 
 #include "llvm/ADT/DenseSet.h"

 class UUID;
 }
 namespace lldb_private {
 class VariableList;
 }
 
 namespace lldb_private {
 
+class ModuleListProperties : public Properties {
+public:
+  ModuleListProperties();
+
+  FileSpec GetClangModulesCachePath() const;
+  bool SetClangModulesCachePath(llvm::StringRef path);
+}; 
+
 //----------------------------------------------------------------------
 /// @class ModuleList ModuleList.h "lldb/Core/ModuleList.h"
 /// @brief A collection class for Module objects.
 ///
 /// Modules in the module collection class are stored as reference
 /// counted shared pointers to Module objects.
 //----------------------------------------------------------------------
 class ModuleList {
public:
   ///     The number of modules in the module list.
   //------------------------------------------------------------------
   size_t GetSize() const;
 
   bool LoadScriptingResourcesInTarget(Target *target, std::list<Status> &errors,
                                       Stream *feedback_stream = nullptr,
                                       bool continue_on_error = true);
 
+  static ModuleListProperties &GetGlobalModuleListProperties();
+
   static bool ModuleIsInCache(const Module *module_ptr);
 
   static Status GetSharedModule(const ModuleSpec &module_spec,
                                 lldb::ModuleSP &module_sp,
                                 const FileSpecList *module_search_paths_ptr,
                                 lldb::ModuleSP *old_module_sp_ptr,
                                 bool *did_create_ptr,
                                 bool always_create = false);
 
   static bool RemoveSharedModule(lldb::ModuleSP &module_sp);
 
   static size_t FindSharedModules(const ModuleSpec &module_spec,
                                   ModuleList &matching_module_list);
 
   static size_t RemoveOrphanSharedModules(bool mandatory);
 
   static bool RemoveSharedModuleIfOrphaned(const Module *module_ptr);
   
   void ForEach(std::function<bool(const lldb::ModuleSP &module_sp)> const
                    &callback) const;
 
 protected:
   //------------------------------------------------------------------
   // Class typedefs.
   //------------------------------------------------------------------
   typedef std::vector<lldb::ModuleSP>

lldb/trunk/include/lldb/Target/Target.h

public:
   bool GetSkipPrologue() const;
 
   PathMappingList &GetSourcePathMap() const;
 
   FileSpecList &GetExecutableSearchPaths();
 
   FileSpecList &GetDebugFileSearchPaths();
 
-  FileSpec &GetClangModulesCachePath();
-
   FileSpecList &GetClangModuleSearchPaths();
 
   bool GetEnableAutoImportClangModules() const;
 
   bool GetEnableAutoApplyFixIts() const;
 
   bool GetEnableNotifyAboutFixIts() const;
 

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile

+LEVEL = ../../../make
+OBJC_SOURCES := main.m
+include $(LEVEL)/Makefile.rules
+CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(SRCDIR)

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/TestClangModulesCache.py

+"""Test that the clang modules cache directory can be controlled."""
+
+from __future__ import print_function
+
+
+import unittest2
+import os
+import time
+import platform
+import shutil
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ObjCModulesTestCase(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        TestBase.setUp(self)
+
+    def test_expr(self):
+        self.build()
+        self.main_source_file = lldb.SBFileSpec("main.m")
+        self.runCmd("settings set target.auto-import-clang-modules true")
+        mod_cache = self.getBuildArtifact("my-clang-modules-cache")
+        if os.path.isdir(mod_cache):
+          shutil.rmtree(mod_cache)
+        self.assertFalse(os.path.isdir(mod_cache),
+                         "module cache should not exist")
+        self.runCmd('settings set clang.modules-cache-path "%s"' % mod_cache)
+        self.runCmd('settings set target.clang-module-search-paths "%s"'
+                    % self.getSourceDir())
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+            self, "Set breakpoint here", self.main_source_file)
+        self.runCmd("expr @import Darwin")
+        self.assertTrue(os.path.isdir(mod_cache), "module cache exists")

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/f.h

+void f() {}

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m

+@import Foo;
+int main() {
+  f(); // Set breakpoint here.
+  return 0;
+}

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/module.modulemap

+module Foo {
+  header "f.h"
+}

lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py

def setUp(self):
         # Works with the test driver to conditionally skip tests via
         # decorators.
         Base.setUp(self)
 
         # Set the clang modules cache path.
         if self.child:
             assert(self.getDebugInfo() == 'default')
             mod_cache = os.path.join(self.getBuildDir(), "module-cache")
-            self.runCmd("settings set target.clang-modules-cache-path "
-                        + mod_cache)
+            self.runCmd('settings set clang.modules-cache-path "%s"'
+                        % mod_cache)
 
 
         if "LLDB_MAX_LAUNCH_COUNT" in os.environ:
             self.maxLaunchCount = int(os.environ["LLDB_MAX_LAUNCH_COUNT"])
 
         if "LLDB_TIME_WAIT_NEXT_LAUNCH" in os.environ:
             self.timeWaitNextLaunch = float(
                 os.environ["LLDB_TIME_WAIT_NEXT_LAUNCH"])

lldb/trunk/source/Core/Debugger.cpp

 //===-- Debugger.cpp --------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Core/Debugger.h"
 
 #include "lldb/Breakpoint/Breakpoint.h" // for Breakpoint, Brea...
 #include "lldb/Core/Event.h"            // for Event, EventData...
 #include "lldb/Core/FormatEntity.h"
 #include "lldb/Core/Listener.h" // for Listener
 #include "lldb/Core/Mangled.h"  // for Mangled
+#include "lldb/Core/ModuleList.h"  // for Mangled
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/State.h"
 #include "lldb/Core/StreamAsynchronousIO.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/DataFormatters/DataVisualization.h"
 #include "lldb/Expression/REPL.h"
 #include "lldb/Host/File.h" // for File, File::kInv...
 #include "lldb/Host/HostInfo.h"
Debugger::Debugger(lldb::LogOutputCallback log_callback, void *baton)
   m_collection_sp->Initialize(g_properties);
   m_collection_sp->AppendProperty(
       ConstString("target"),
       ConstString("Settings specify to debugging targets."), true,
       Target::GetGlobalProperties()->GetValueProperties());
   m_collection_sp->AppendProperty(
       ConstString("platform"), ConstString("Platform settings."), true,
       Platform::GetGlobalPlatformProperties()->GetValueProperties());
+  m_collection_sp->AppendProperty(
+      ConstString("clang"), ConstString("Settings specific to Clang."), true,
+      ModuleList::GetGlobalModuleListProperties().GetValueProperties());
   if (m_command_interpreter_ap) {
     m_collection_sp->AppendProperty(
         ConstString("interpreter"),
         ConstString("Settings specify to the debugger's command interpreter."),
         true, m_command_interpreter_ap->GetValueProperties());
   }
   OptionValueSInt64 *term_width =
       m_collection_sp->GetPropertyAtIndexAsOptionValueSInt64(

lldb/trunk/source/Core/ModuleList.cpp

 //===-- ModuleList.cpp ------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Core/ModuleList.h"
 #include "lldb/Core/FileSpecList.h" // for FileSpecList
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Symbols.h"
+#include "lldb/Interpreter/OptionValueProperties.h"
+#include "lldb/Interpreter/OptionValueFileSpec.h"
+#include "lldb/Interpreter/Property.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolContext.h" // for SymbolContextList, SymbolCon...
 #include "lldb/Symbol/VariableList.h"
 #include "lldb/Utility/ArchSpec.h"    // for ArchSpec
 #include "lldb/Utility/ConstString.h" // for ConstString
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Logging.h" // for GetLogIfAnyCategoriesSet
 #include "lldb/Utility/UUID.h"    // for UUID, operator!=, operator==
 #include "lldb/lldb-defines.h"    // for LLDB_INVALID_INDEX32
 
 #if defined(LLVM_ON_WIN32)
 #include "lldb/Host/windows/PosixApi.h" // for PATH_MAX
 #endif
 
 #include "llvm/ADT/StringRef.h" // for StringRef
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h" // for fs
+#include "clang/Driver/Driver.h"
 
 #include <chrono> // for operator!=, time_point
 #include <memory> // for shared_ptr
 #include <mutex>
 #include <string>  // for string
 #include <utility> // for distance
 
 namespace lldb_private {

 }
 namespace lldb_private {
 class TypeList;
 }
 
 using namespace lldb;
 using namespace lldb_private;
 
+namespace {
+
+PropertyDefinition g_properties[] = {
+    {"modules-cache-path", OptionValue::eTypeFileSpec, true, 0, nullptr,
+     nullptr,
+     "The path to the clang modules cache directory (-fmodules-cache-path)."},
+    {nullptr, OptionValue::eTypeInvalid, false, 0, nullptr, nullptr, nullptr}};
+
+enum { ePropertyClangModulesCachePath };
+
+} // namespace
+
+ModuleListProperties::ModuleListProperties() {
+  m_collection_sp.reset(new OptionValueProperties(ConstString("clang")));
+  m_collection_sp->Initialize(g_properties);
+
+  llvm::SmallString<128> path;
+  clang::driver::Driver::getDefaultModuleCachePath(path);
+  SetClangModulesCachePath(path);
+}
+
+FileSpec ModuleListProperties::GetClangModulesCachePath() const {
+  return m_collection_sp
+      ->GetPropertyAtIndexAsOptionValueFileSpec(nullptr, false,
+                                                ePropertyClangModulesCachePath)
+      ->GetCurrentValue();
+}
+
+bool ModuleListProperties::SetClangModulesCachePath(llvm::StringRef path) {
+  return m_collection_sp->SetPropertyAtIndexAsString(
+      nullptr, ePropertyClangModulesCachePath, path);
+}
+
+
 ModuleList::ModuleList()
     : m_modules(), m_modules_mutex(), m_notifier(nullptr) {}
 
 ModuleList::ModuleList(const ModuleList &rhs)
     : m_modules(), m_modules_mutex(), m_notifier(nullptr) {
   std::lock_guard<std::recursive_mutex> lhs_guard(m_modules_mutex);
   std::lock_guard<std::recursive_mutex> rhs_guard(rhs.m_modules_mutex);
   m_modules = rhs.m_modules;
if (module) {
     for (pos = begin; pos != end; ++pos) {
       if ((*pos).get() == module)
         return std::distance(begin, pos);
     }
   }
   return LLDB_INVALID_INDEX32;
 }
 
-static ModuleList &GetSharedModuleList() {
-  static ModuleList *g_shared_module_list = nullptr;
+namespace {
+struct SharedModuleListInfo {
+  ModuleList module_list;
+  ModuleListProperties module_list_properties;
+};
+}
+static SharedModuleListInfo &GetSharedModuleListInfo()
+{
+  static SharedModuleListInfo *g_shared_module_list_info = nullptr;
   static llvm::once_flag g_once_flag;
   llvm::call_once(g_once_flag, []() {
     // NOTE: Intentionally leak the module list so a program doesn't have to
     // cleanup all modules and object files as it exits. This just wastes time
     // doing a bunch of cleanup that isn't required.
-    if (g_shared_module_list == nullptr)
-      g_shared_module_list = new ModuleList(); // <--- Intentional leak!!!
+    if (g_shared_module_list_info == nullptr)
+      g_shared_module_list_info = new SharedModuleListInfo();
   });
-  return *g_shared_module_list;
+  return *g_shared_module_list_info;
+}
+
+static ModuleList &GetSharedModuleList() {
+  return GetSharedModuleListInfo().module_list;
+}
+
+ModuleListProperties &ModuleList::GetGlobalModuleListProperties() {
+  return GetSharedModuleListInfo().module_list_properties;
 }
 
 bool ModuleList::ModuleIsInCache(const Module *module_ptr) {
   if (module_ptr) {
     ModuleList &shared_module_list = GetSharedModuleList();
     return shared_module_list.FindModule(module_ptr).get() != nullptr;
   }
   return false;

lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

 //===-- ClangModulesDeclVendor.cpp ------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 // C Includes
 // C++ Includes
 #include <mutex>
 
 // Other libraries and framework includes
 #include "clang/Basic/TargetInfo.h"
-#include "clang/Driver/Driver.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Parse/Parser.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Serialization/ASTReader.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Threading.h"
 
 // Project includes
 #include "ClangModulesDeclVendor.h"
 
+#include "lldb/Core/ModuleList.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
ClangModulesDeclVendor::Create(Target &target) {
   target.GetPlatform()->AddClangModuleCompilationOptions(
       &target, compiler_invocation_arguments);
 
   compiler_invocation_arguments.push_back(ModuleImportBufferName);
 
   // Add additional search paths with { "-I", path } or { "-F", path } here.
 
   {
-    llvm::SmallString<128> Path;
-    target.GetClangModulesCachePath().GetPath(Path);
-    if (Path.empty())
-      clang::driver::Driver::getDefaultModuleCachePath(Path);
+    llvm::SmallString<128> path;
+    auto props = ModuleList::GetGlobalModuleListProperties();
+    props.GetClangModulesCachePath().GetPath(path);
     std::string module_cache_argument("-fmodules-cache-path=");
-    module_cache_argument.append(Path.str());
+    module_cache_argument.append(path.str());
     compiler_invocation_arguments.push_back(module_cache_argument);
   }
 
   FileSpecList &module_search_paths = target.GetClangModuleSearchPaths();
 
   for (size_t spi = 0, spe = module_search_paths.GetSize(); spi < spe; ++spi) {
     const FileSpec &search_path = module_search_paths.GetFileSpecAtIndex(spi);
 

lldb/trunk/source/Target/Target.cpp

static PropertyDefinition g_properties[] = {
     {"display-expression-in-crashlogs", OptionValue::eTypeBoolean, false, false,
      nullptr, nullptr, "Expressions that crash will show up in crash logs if "
                        "the host system supports executable specific crash log "
                        "strings and this setting is set to true."},
     {"trap-handler-names", OptionValue::eTypeArray, true,
      OptionValue::eTypeString, nullptr, nullptr,
      "A list of trap handler function names, e.g. a common Unix user process "
      "one is _sigtramp."},
-    {"clang-modules-cache-path",
-     OptionValue::eTypeFileSpec, false, 0, nullptr, nullptr,
-     "The path to the clang modules cache directory (-fmodules-cache-path)."},
     {"display-runtime-support-values", OptionValue::eTypeBoolean, false, false,
      nullptr, nullptr, "If true, LLDB will show variables that are meant to "
                        "support the operation of a language's runtime "
                        "support."},
     {"non-stop-mode", OptionValue::eTypeBoolean, false, 0, nullptr, nullptr,
      "Disable lock-step debugging, instead control threads independently."},
     {nullptr, OptionValue::eTypeInvalid, false, 0, nullptr, nullptr, nullptr}};
 
enum {
   ePropertyUseHexImmediates,
   ePropertyHexImmediateStyle,
   ePropertyUseFastStepping,
   ePropertyLoadScriptFromSymbolFile,
   ePropertyLoadCWDlldbinitFile,
   ePropertyMemoryModuleLoadLevel,
   ePropertyDisplayExpressionsInCrashlogs,
   ePropertyTrapHandlerNames,
-  ePropertyClangModulesCachePath,
   ePropertyDisplayRuntimeSupportValues,
   ePropertyNonStopModeEnabled,
   ePropertyExperimental
 };
 
 class TargetOptionValueProperties : public OptionValueProperties {
 public:
   TargetOptionValueProperties(const ConstString &name)
FileSpecList &TargetProperties::GetDebugFileSearchPaths() {
   const uint32_t idx = ePropertyDebugFileSearchPaths;
   OptionValueFileSpecList *option_value =
       m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpecList(nullptr,
                                                                    false, idx);
   assert(option_value);
   return option_value->GetCurrentValue();
 }
 
-FileSpec &TargetProperties::GetClangModulesCachePath() {
-  const uint32_t idx = ePropertyClangModulesCachePath;
-  OptionValueFileSpec *option_value =
-      m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpec(nullptr, false,
-                                                               idx);
-  assert(option_value);
-  return option_value->GetCurrentValue();
-}
-
 FileSpecList &TargetProperties::GetClangModuleSearchPaths() {
   const uint32_t idx = ePropertyClangModuleSearchPaths;
   OptionValueFileSpecList *option_value =
       m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpecList(nullptr,
                                                                    false, idx);
   assert(option_value);
   return option_value->GetCurrentValue();
 }