[Lldb-commits] [PATCH] D150621: lldb PlatformDarwinKernel, delay local filesystem scan until needed

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added a reviewer: bulbazord.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150621

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h


Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
===================================================================
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
@@ -158,6 +158,8 @@
   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.
@@ -194,6 +196,9 @@
 
   LazyBool m_ios_debug_session;
 
+  std::mutex m_kext_scan_mutex;
+  bool m_did_kext_scan;
+
   PlatformDarwinKernel(const PlatformDarwinKernel &) = delete;
   const PlatformDarwinKernel &operator=(const PlatformDarwinKernel &) = delete;
 };
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
===================================================================
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
@@ -229,12 +229,8 @@
       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_mutex(), m_did_kext_scan(false) {}
 
 /// Destructor.
 ///
@@ -243,6 +239,7 @@
 PlatformDarwinKernel::~PlatformDarwinKernel() = default;
 
 void PlatformDarwinKernel::GetStatus(Stream &strm) {
+  UpdateKextandKernelsLocalScan();
   Platform::GetStatus(strm);
   strm.Printf(" Debug session type: ");
   if (m_ios_debug_session == eLazyBoolYes)
@@ -709,6 +706,15 @@
   return results;
 }
 
+void PlatformDarwinKernel::UpdateKextandKernelsLocalScan() {
+  std::lock_guard<std::mutex> guard(m_kext_scan_mutex);
+  if (!m_did_kext_scan) {
+    CollectKextAndKernelDirectories();
+    SearchForKextsAndKernelsRecursively();
+    m_did_kext_scan = true;
+  }
+}
+
 Status PlatformDarwinKernel::GetSharedModule(
     const ModuleSpec &module_spec, Process *process, ModuleSP &module_sp,
     const FileSpecList *module_search_paths_ptr,
@@ -789,6 +795,7 @@
     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) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150621.522353.patch
Type: text/x-patch
Size: 2844 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20230515/7ec5e205/attachment.bin>