[PATCH] D48302: Search for kext variants is searching from parent directory when it should not be

[Jason Molenda via Phabricator via llvm-commits]

jasonmolenda created this revision.
jasonmolenda added a reviewer: JDevlieghere.
Herald added a subscriber: llvm-commits.

There's a small perf issue after the changes to r334205; the patch is intended to search all the executables in a kext bundle to discover variants, e.g.

/System/Library/Extensions/AppleUSB.kext

everything under the AppleUSB.kext subdir should be searched.  But the patch used the FileSpec's GetDirectory method, so we ended up recursively searching all the kexts in the parent directory.

Functionally, these are equivalent - recursively searching from the parent dir will yield the same result because we require a binary's UUID match before we use it.  But it has a high performance cost if we search from the wrong base directory.

rdar://problem/41227170


Repository:
  rL LLVM

https://reviews.llvm.org/D48302

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


Index: source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
===================================================================
--- source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
+++ source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
@@ -128,7 +128,7 @@
                                       bool recurse);
 
   static std::vector<lldb_private::FileSpec>
-  SearchForExecutablesRecursively(const lldb_private::ConstString &dir);
+  SearchForExecutablesRecursively(const std::string &dir);
 
   static void AddKextToMap(PlatformDarwinKernel *thisp,
                            const lldb_private::FileSpec &file_spec);
Index: source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
===================================================================
--- source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
+++ source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
@@ -780,10 +780,10 @@
 }
 
 std::vector<lldb_private::FileSpec>
-PlatformDarwinKernel::SearchForExecutablesRecursively(const ConstString &dir) {
+PlatformDarwinKernel::SearchForExecutablesRecursively(const std::string &dir) {
   std::vector<FileSpec> executables;
   std::error_code EC;
-  for (llvm::sys::fs::recursive_directory_iterator it(dir.GetStringRef(), EC),
+  for (llvm::sys::fs::recursive_directory_iterator it(dir.c_str(), EC),
        end;
        it != end && !EC; it.increment(EC)) {
     auto status = it->status();
@@ -800,7 +800,7 @@
     const FileSpec &kext_bundle_path, const lldb_private::UUID &uuid,
     const ArchSpec &arch, ModuleSP &exe_module_sp) {
   for (const auto &exe_file :
-       SearchForExecutablesRecursively(kext_bundle_path.GetDirectory())) {
+       SearchForExecutablesRecursively(kext_bundle_path.GetPath())) {
     if (exe_file.Exists()) {
       ModuleSpec exe_spec(exe_file);
       exe_spec.GetUUID() = uuid;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D48302.151809.patch
Type: text/x-patch
Size: 1838 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180618/78e1451e/attachment.bin>