Introduce a setting to disable Spotlight while running the test suite

[aprantl]

Summary

This is a more principled approach to disabling Spotlight .dSYM lookups while running the testsuite, most importantly it also works for the LIT-based tests, which I overlooked in my initial fix (renaming the test build dir to lldb-tests.noindex).

[labath]

Does that mean we can remove the .noindex thingy from the build folder name? (In any case, the change seems fine to me).

[clayborg]

I would rather see this done in a more generic fashion. Any platform should be able to enable/disable external symbol lookups. Maybe make this setting:

(lldb) settings set symbols.enable-external-lookups false

On Darwin this would mean dSYM. Any other platform can skip any symbol lookups that aren't part of the object file or specified inside the object file.

[aprantl]

In D44342#1034514, @labath wrote:

Does that mean we can remove the .noindex thingy from the build folder name? (In any case, the change seems fine to me).

Yes, but we could still keep it for performance reasons. Perhaps only on Darwin.

[aprantl]

Renamed option to something more general.

[aprantl]

Thanks for the suggestion, Greg! I renamed to option to use a more generic name and removed the #if APPLE guards.

[clayborg]

source/Core/ModuleList.cpp

70 ↗

(On Diff #138082)

Remove this comment? I can apply to any platform in the future that implements symbol lookups right?

73–74 ↗

(On Diff #138082)

Maybe restate this a bit? Maybe something like:

"Disable using external tools or linked libraries to locate symbol files."

[aprantl]

More review feedback.

[vsk]

Thanks @aprantl!

[jingham]

This is great. I think I'm supposed to complain that there isn't a positive test that this flag works. This is in the testsuite, but that will just give a negative signal - that we aren't getting random failures that are maybe due to leaving built test products around on your indexed volumes somewhere.

If we weren't building in a .noindex hierarchy this would be as simple as building the dSYM variant, then moving the dSYM to a subdirectory and making sure we still don't read in the dSYM. But that won't work in a .noindex build. Not sure we should be copying files to people's desktops. /tmp/ won't work because Spotlight doesn't index there. We talked briefly about having an indexed directory for test products as well as a no index one, which you could then use for that purpose.

But I'm not sure that this switch is worth that effort. This is fine by me as is.

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

 namespace lldb_private {
 
 class ModuleListProperties : public Properties {
 public:
   ModuleListProperties();
 
   FileSpec GetClangModulesCachePath() const;
   bool SetClangModulesCachePath(llvm::StringRef path);
+  bool GetEnableExternalLookup() const;
 }; 
 
 //----------------------------------------------------------------------
 /// @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.

lldb/trunk/lit/lit-lldb-init

+# LLDB init file for the LIT tests.
+settings set symbols.enable-external-lookup false

lldb/trunk/lit/lit.cfg

 
 # Propagate PYTHON_EXECUTABLE into the environment
 config.environment['PYTHON_EXECUTABLE'] = getattr(config, 'python_executable', '')
 
 # Register substitutions
 config.substitutions.append(('%python', config.python_executable))
 
 debugserver = lit.util.which('debugserver', lldb_tools_dir)
-lldb = lit.util.which('lldb', lldb_tools_dir)
+lldb = "%s -S %s/lit-lldb-init" % (lit.util.which('lldb', lldb_tools_dir),
+                               config.test_source_root)
 
 if not os.path.exists(config.cc):
     config.cc = lit.util.which(config.cc, config.environment['PATH'])
 
 if not os.path.exists(config.cxx):
     config.cxx = lit.util.which(config.cxx, config.environment['PATH'])
 
 if platform.system() in ['Darwin']:

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

class TestBase(Base):
     def setUp(self):
         #import traceback
         # traceback.print_stack()
 
         # Works with the test driver to conditionally skip tests via
         # decorators.
         Base.setUp(self)
 
-        # Set the clang modules cache path.
         if self.child:
+            # Set the clang modules cache path.
             assert(self.getDebugInfo() == 'default')
             mod_cache = os.path.join(self.getBuildDir(), "module-cache")
             self.runCmd('settings set symbols.clang-modules-cache-path "%s"'
                         % mod_cache)
 
+            # Disable Spotlight lookup. The testsuite creates
+            # different binaries with the same UUID, because they only
+            # differ in the debug info, which is not being hashed.
+            self.runCmd('settings set symbols.enable-external-lookup false')
+
 
         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/ModuleList.cpp

 }
 
 using namespace lldb;
 using namespace lldb_private;
 
 namespace {
 
 PropertyDefinition g_properties[] = {
+    {"enable-external-lookup", OptionValue::eTypeBoolean, true, true, nullptr,
+     nullptr,
+     "Control the use of external tools or libraries to locate symbol files. "
+     "On macOS, Spotlight is used to locate a matching .dSYM bundle based on "
+     "the UUID of the executable."},
     {"clang-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 };
+enum { ePropertyEnableExternalLookup, ePropertyClangModulesCachePath };
 
 } // namespace
 
 ModuleListProperties::ModuleListProperties() {
   m_collection_sp.reset(new OptionValueProperties(ConstString("symbols")));
   m_collection_sp->Initialize(g_properties);
 
   llvm::SmallString<128> path;
   clang::driver::Driver::getDefaultModuleCachePath(path);
   SetClangModulesCachePath(path);
 }
 
+bool ModuleListProperties::GetEnableExternalLookup() const {
+  const uint32_t idx = ePropertyEnableExternalLookup;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+      nullptr, idx, g_properties[idx].default_uint_value != 0);
+}
+
 FileSpec ModuleListProperties::GetClangModulesCachePath() const {
   return m_collection_sp
       ->GetPropertyAtIndexAsOptionValueFileSpec(nullptr, false,
                                                 ePropertyClangModulesCachePath)
       ->GetCurrentValue();
 }
 
 bool ModuleListProperties::SetClangModulesCachePath(llvm::StringRef path) {

lldb/trunk/source/Host/macosx/Symbols.cpp

 // Other libraries and framework includes
 #include <CoreFoundation/CoreFoundation.h>
 
 // Project includes
 #include "Host/macosx/cfcpp/CFCBundle.h"
 #include "Host/macosx/cfcpp/CFCData.h"
 #include "Host/macosx/cfcpp/CFCReleaser.h"
 #include "Host/macosx/cfcpp/CFCString.h"
+#include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"

 
 CFURLRef DBGCopyFullDSYMURLForUUID(CFUUIDRef uuid, CFURLRef exec_url);
 CFDictionaryRef DBGCopyDSYMPropertyLists(CFURLRef dsym_url);
 }
 #endif
 
 int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
                                        ModuleSpec &return_module_spec) {
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+  if (!ModuleList::GetGlobalModuleListProperties().GetEnableExternalLookup()) {
+    if (log)
+      log->Printf("Spotlight lookup for .dSYM bundles is disabled.");
+    return 0;
+  }
+  
   return_module_spec = module_spec;
   return_module_spec.GetFileSpec().Clear();
   return_module_spec.GetSymbolFileSpec().Clear();
 
   int items_found = 0;
 
 #if !defined(__arm__) && !defined(__arm64__) &&                                \
     !defined(__aarch64__) // No DebugSymbols on the iOS devices
if (module_uuid != NULL) {
           NULL, module_uuid[0], module_uuid[1], module_uuid[2], module_uuid[3],
           module_uuid[4], module_uuid[5], module_uuid[6], module_uuid[7],
           module_uuid[8], module_uuid[9], module_uuid[10], module_uuid[11],
           module_uuid[12], module_uuid[13], module_uuid[14], module_uuid[15]));
 
       if (module_uuid_ref.get()) {
         CFCReleaser<CFURLRef> exec_url;
         const FileSpec *exec_fspec = module_spec.GetFileSpecPtr();
-        Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
         if (exec_fspec) {
           char exec_cf_path[PATH_MAX];
           if (exec_fspec->GetPath(exec_cf_path, sizeof(exec_cf_path)))
             exec_url.reset(::CFURLCreateFromFileSystemRepresentation(
                 NULL, (const UInt8 *)exec_cf_path, strlen(exec_cf_path),
                 FALSE));
         }