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

[Jason Molenda via Phabricator via lldb-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;



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;

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits