lldb PlatformDarwinKernel, delay local filesystem scan until needed

[jasonmolenda]

Summary

The darwin kernel debugging platform plugin has a feature where it will scan a list of directories on the local filesystem for kernel binaries and kexts, loadable kernel extensions. When it finds a kernel/kext with a given UUID, it looks over the list of binaries it found, and loads them automatically.

In https://reviews.llvm.org/D133680 I added a feature to PlatformDarwinKernel where the platform is force-created, and then a method is called on it with an address in memory. If the address is a Mach-O fileset, it looks for a kernel in the fileset and returns the address of the kernel binary. This allows for generic code (live debugging, corefile loading) to handle this platform-specific case in a generic way.

In the ctor for PlatformDarwinKernel, I was doing this local filesystem scan. And if a generic bit of code is force creating this platform to run that method for each binary, we could end up iterating over the local filesystem multiple times. In extreme cases, it was a noticeable perf hit.

This patch moves the filesystem scan out of the ctor and into a method which is called in the two places where this platform actually needs the list of binaries. It grabs a mutex before doing the scan, to avoid a race if multiple threads print the platform status or try to find a kext/kernel on the local filesystem. There isn't any change in behavior beyond the performance difference when this platform is force-created multiple times in a debug session.

[JDevlieghere]

You can do this simpler with a single std::once_flag.

lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp

710–715

lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h

199–200

[bulbazord]

In D150621#4344243, @JDevlieghere wrote:

You can do this simpler with a single std::once_flag.

+1

I like the idea of this patch a lot! LGTM otherwise.

[jasonmolenda]

Ah, makes sense, will update.

[jasonmolenda]

Update patch to use a std::once operation instead of hand-rolling it, as suggested by Jonas and Alex.

[bulbazord]

LGTM

lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h

protected:
 
   Status ExamineKextForMatchingUUID(const FileSpec &kext_bundle_path,
                                     const UUID &uuid, const ArchSpec &arch,
                                     lldb::ModuleSP &exe_module_sp);
 
   bool LoadPlatformBinaryAndSetup(Process *process, lldb::addr_t addr,
                                   bool notify) override;
 
+  void UpdateKextandKernelsLocalScan();
+
   // Most of the ivars are assembled under FileSystem::EnumerateDirectory calls
   // where the function being called for each file/directory must be static.
   // We'll pass a this pointer as a baton and access the ivars directly.
   // Toss-up whether this should just be a struct at this point.
 
 public:
   /// Multimap of CFBundleID to FileSpec on local filesystem, kexts with dSYMs
   /// next to them.
public:
   /// List of kernel dsyms with no binaries next to them.
   KernelBinaryCollection m_kernel_dsyms_no_binaries;
 
   /// List of kernel .dSYM.yaa files.
   KernelBinaryCollection m_kernel_dsyms_yaas;
 
   LazyBool m_ios_debug_session;
 
+  std::once_flag m_kext_scan_flag;
+
   PlatformDarwinKernel(const PlatformDarwinKernel &) = delete;
   const PlatformDarwinKernel &operator=(const PlatformDarwinKernel &) = delete;
 };
 
 #else // __APPLE__
 
 // Since DynamicLoaderDarwinKernel is compiled in for all systems, and relies
 // on PlatformDarwinKernel for the plug-in name, we compile just the plug-in

lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp

 /// Default Constructor
 PlatformDarwinKernel::PlatformDarwinKernel(
     lldb_private::LazyBool is_ios_debug_session)
     : PlatformDarwin(false), // This is a remote platform
       m_name_to_kext_path_map_with_dsyms(),
       m_name_to_kext_path_map_without_dsyms(), m_search_directories(),
       m_search_directories_no_recursing(), m_kernel_binaries_with_dsyms(),
       m_kernel_binaries_without_dsyms(), m_kernel_dsyms_no_binaries(),
-      m_kernel_dsyms_yaas(), m_ios_debug_session(is_ios_debug_session)
-
-{
-  CollectKextAndKernelDirectories();
-  SearchForKextsAndKernelsRecursively();
-}
+      m_kernel_dsyms_yaas(), m_ios_debug_session(is_ios_debug_session),
+      m_kext_scan_flag() {}
 
 /// Destructor.
 ///
 /// The destructor is virtual since this class is designed to be
 /// inherited from by the plug-in instance.
 PlatformDarwinKernel::~PlatformDarwinKernel() = default;
 
 void PlatformDarwinKernel::GetStatus(Stream &strm) {
+  UpdateKextandKernelsLocalScan();
   Platform::GetStatus(strm);
   strm.Printf(" Debug session type: ");
   if (m_ios_debug_session == eLazyBoolYes)
     strm.Printf("iOS kernel debugging\n");
   else if (m_ios_debug_session == eLazyBoolNo)
     strm.Printf("Mac OS X kernel debugging\n");
   else
     strm.Printf("unknown kernel debugging\n");
PlatformDarwinKernel::GetDWARFBinaryInDSYMBundle(const FileSpec &dsym_bundle) {
   binary_filepath += filename;
 
   FileSpec binary_fspec(binary_filepath);
   if (FileSystem::Instance().Exists(binary_fspec))
     results.push_back(binary_fspec);
   return results;
 }
 
+void PlatformDarwinKernel::UpdateKextandKernelsLocalScan() {
+  std::call_once(m_kext_scan_flag, [this]() {
+    CollectKextAndKernelDirectories();
+    SearchForKextsAndKernelsRecursively();
+  });
+}
+
 Status PlatformDarwinKernel::GetSharedModule(
     const ModuleSpec &module_spec, Process *process, ModuleSP &module_sp,
     const FileSpecList *module_search_paths_ptr,
     llvm::SmallVectorImpl<ModuleSP> *old_modules, bool *did_create_ptr) {
   Status error;
   module_sp.reset();
   const FileSpec &platform_file = module_spec.GetFileSpec();
 

 }
 
 Status PlatformDarwinKernel::GetSharedModuleKernel(
     const ModuleSpec &module_spec, Process *process, ModuleSP &module_sp,
     const FileSpecList *module_search_paths_ptr,
     llvm::SmallVectorImpl<ModuleSP> *old_modules, bool *did_create_ptr) {
   Status error;
   module_sp.reset();
+  UpdateKextandKernelsLocalScan();
 
   // First try all kernel binaries that have a dSYM next to them
   for (auto possible_kernel : m_kernel_binaries_with_dsyms) {
     if (FileSystem::Instance().Exists(possible_kernel)) {
       ModuleSpec kern_spec(possible_kernel);
       kern_spec.GetUUID() = module_spec.GetUUID();
       module_sp.reset(new Module(kern_spec));
       if (module_sp && module_sp->GetObjectFile() &&