[Lldb-commits] [lldb] 457eb05 - [lldb/PlatformMacOSX] Re-implement GetDeveloperDirectory

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Fri Mar 27 12:37:16 PDT 2020


Author: Jonas Devlieghere
Date: 2020-03-27T12:36:56-07:00
New Revision: 457eb05db67d201f34920e762a90d0deb670becb

URL: https://github.com/llvm/llvm-project/commit/457eb05db67d201f34920e762a90d0deb670becb
DIFF: https://github.com/llvm/llvm-project/commit/457eb05db67d201f34920e762a90d0deb670becb.diff

LOG: [lldb/PlatformMacOSX] Re-implement GetDeveloperDirectory

GetDeveloperDirectory returns a const char* which is NULL when we cannot
find the developer directory. This crashes in
PlatformDarwinKernel::CollectKextAndKernelDirectories because we're
unconditionally assigning it to a std::string. Coincidentally I just
refactored a bunch of code in PlatformMacOSX so instead of a ad-hoc fix
I've reimplemented the method based on GetXcodeContentsDirectory.

The change is mostly NFC. Obviously it fixes the crash, but it also
removes support for finding the Xcode directory through he legacy
$XCODE_SELECT_PREFIX_DIR/usr/share/xcode-select/xcode_dir_path.

Differential revision: https://reviews.llvm.org/D76938

Added: 
    

Modified: 
    lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
    lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp
    lldb/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.cpp
    lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
    lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
    lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
    lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
    lldb/source/Plugins/Platform/MacOSX/PlatformiOSSimulator.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
index 41c699975909..cb6fbce19e58 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
@@ -77,9 +77,10 @@ void PlatformAppleSimulator::GetStatus(Stream &strm) {
   // simulator
   PlatformAppleSimulator::LoadCoreSimulator();
 
+  std::string developer_dir = GetXcodeDeveloperDirectory().GetPath();
   CoreSimulatorSupport::DeviceSet devices =
       CoreSimulatorSupport::DeviceSet::GetAvailableDevices(
-          GetDeveloperDirectory());
+          developer_dir.c_str());
   const size_t num_devices = devices.GetNumDevices();
   if (num_devices) {
     strm.Printf("Available devices:\n");
@@ -123,9 +124,10 @@ Status PlatformAppleSimulator::ConnectRemote(Args &args) {
     const char *arg_cstr = args.GetArgumentAtIndex(0);
     if (arg_cstr) {
       std::string arg_str(arg_cstr);
+      std::string developer_dir = GetXcodeDeveloperDirectory().GetPath();
       CoreSimulatorSupport::DeviceSet devices =
           CoreSimulatorSupport::DeviceSet::GetAvailableDevices(
-              GetDeveloperDirectory());
+              developer_dir.c_str());
       devices.ForEach(
           [this, &arg_str](const CoreSimulatorSupport::Device &device) -> bool {
             if (arg_str == device.GetUDID() || arg_str == device.GetName()) {
@@ -212,12 +214,12 @@ FileSpec PlatformAppleSimulator::GetCoreSimulatorPath() {
 #if defined(__APPLE__)
   std::lock_guard<std::mutex> guard(m_core_sim_path_mutex);
   if (!m_core_simulator_framework_path.hasValue()) {
-    const char *developer_dir = GetDeveloperDirectory();
-    if (developer_dir) {
+    if (FileSpec fspec = GetXcodeDeveloperDirectory()) {
+      std::string developer_dir = fspec.GetPath();
       StreamString cs_path;
       cs_path.Printf(
           "%s/Library/PrivateFrameworks/CoreSimulator.framework/CoreSimulator",
-          developer_dir);
+          developer_dir.c_str());
       m_core_simulator_framework_path = FileSpec(cs_path.GetData());
       FileSystem::Instance().Resolve(*m_core_simulator_framework_path);
     }
@@ -245,8 +247,9 @@ CoreSimulatorSupport::Device PlatformAppleSimulator::GetSimulatorDevice() {
   if (!m_device.hasValue()) {
     const CoreSimulatorSupport::DeviceType::ProductFamilyID dev_id =
         CoreSimulatorSupport::DeviceType::ProductFamilyID::iPhone;
+    std::string developer_dir = GetXcodeDeveloperDirectory().GetPath();
     m_device = CoreSimulatorSupport::DeviceSet::GetAvailableDevices(
-                   GetDeveloperDirectory())
+                   developer_dir.c_str())
                    .GetFanciest(dev_id);
   }
 

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp
index 7e3e7c92f180..fa5f93ba1488 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp
@@ -255,14 +255,14 @@ EnumerateDirectoryCallback(void *baton, llvm::sys::fs::file_type ft,
 const char *PlatformAppleTVSimulator::GetSDKDirectoryAsCString() {
   std::lock_guard<std::mutex> guard(m_sdk_dir_mutex);
   if (m_sdk_directory.empty()) {
-    const char *developer_dir = GetDeveloperDirectory();
-    if (developer_dir) {
+    if (FileSpec fspec = GetXcodeDeveloperDirectory()) {
+      std::string developer_dir = fspec.GetPath();
       char sdks_directory[PATH_MAX];
       char sdk_dirname[PATH_MAX];
       sdk_dirname[0] = '\0';
       snprintf(sdks_directory, sizeof(sdks_directory),
                "%s/Platforms/AppleTVSimulator.platform/Developer/SDKs",
-               developer_dir);
+               developer_dir.c_str());
       FileSpec simulator_sdk_spec;
       bool find_directories = true;
       bool find_files = false;

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.cpp
index e24923609d4a..6cd20164ae78 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.cpp
@@ -255,14 +255,14 @@ EnumerateDirectoryCallback(void *baton, llvm::sys::fs::file_type ft,
 const char *PlatformAppleWatchSimulator::GetSDKDirectoryAsCString() {
   std::lock_guard<std::mutex> guard(m_sdk_dir_mutex);
   if (m_sdk_directory.empty()) {
-    const char *developer_dir = GetDeveloperDirectory();
-    if (developer_dir) {
+    if (FileSpec fspec = GetXcodeDeveloperDirectory()) {
+      std::string developer_dir = fspec.GetPath();
       char sdks_directory[PATH_MAX];
       char sdk_dirname[PATH_MAX];
       sdk_dirname[0] = '\0';
       snprintf(sdks_directory, sizeof(sdks_directory),
                "%s/Platforms/AppleWatchSimulator.platform/Developer/SDKs",
-               developer_dir);
+               developer_dir.c_str());
       FileSpec simulator_sdk_spec;
       bool find_directories = true;
       bool find_files = false;

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 350043f8d4e9..e1633007f23f 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -48,9 +48,7 @@ using namespace lldb;
 using namespace lldb_private;
 
 /// Default Constructor
-PlatformDarwin::PlatformDarwin(bool is_host)
-    : PlatformPOSIX(is_host), // This is the local host platform
-      m_developer_directory() {}
+PlatformDarwin::PlatformDarwin(bool is_host) : PlatformPOSIX(is_host) {}
 
 /// Destructor.
 ///
@@ -1135,88 +1133,17 @@ static FileSpec GetXcodeSelectPath() {
   return g_xcode_select_filespec;
 }
 
-// Return a directory path like /Applications/Xcode.app/Contents/Developer
-const char *PlatformDarwin::GetDeveloperDirectory() {
-  std::lock_guard<std::mutex> guard(m_mutex);
-  if (m_developer_directory.empty()) {
-    bool developer_dir_path_valid = false;
-    char developer_dir_path[PATH_MAX];
-
-    // Get the lldb framework's file path, and if it exists, truncate some
-    // components to only the developer directory path.
-    FileSpec temp_file_spec = HostInfo::GetShlibDir();
-    if (temp_file_spec) {
-      if (temp_file_spec.GetPath(developer_dir_path,
-                                 sizeof(developer_dir_path))) {
-        // e.g.
-        // /Applications/Xcode.app/Contents/SharedFrameworks/LLDB.framework
-        char *shared_frameworks =
-            strstr(developer_dir_path, "/SharedFrameworks/LLDB.framework");
-        if (shared_frameworks) {
-          shared_frameworks[0] = '\0';  // truncate developer_dir_path at this point
-          strncat (developer_dir_path, "/Developer", sizeof (developer_dir_path) - 1); // add /Developer on
-          developer_dir_path_valid = true;
-        } else {
-          // e.g.
-          // /Applications/Xcode.app/Contents/Developer/Toolchains/iOS11.2.xctoolchain/System/Library/PrivateFrameworks/LLDB.framework
-          char *developer_toolchains =
-            strstr(developer_dir_path, "/Contents/Developer/Toolchains/");
-          if (developer_toolchains) {
-            developer_toolchains += sizeof ("/Contents/Developer") - 1;
-            developer_toolchains[0] = '\0'; // truncate developer_dir_path at this point
-            developer_dir_path_valid = true;
-          }
-        }
-      }
-    }
-
-    if (!developer_dir_path_valid) {
-      std::string xcode_dir_path;
-      const char *xcode_select_prefix_dir = getenv("XCODE_SELECT_PREFIX_DIR");
-      if (xcode_select_prefix_dir)
-        xcode_dir_path.append(xcode_select_prefix_dir);
-      xcode_dir_path.append("/usr/share/xcode-select/xcode_dir_path");
-      temp_file_spec.SetFile(xcode_dir_path, FileSpec::Style::native);
-      auto dir_buffer =
-          FileSystem::Instance().CreateDataBuffer(temp_file_spec.GetPath());
-      if (dir_buffer && dir_buffer->GetByteSize() > 0) {
-        llvm::StringRef path_ref(dir_buffer->GetChars());
-        // Trim tailing newlines and make sure there is enough room for a null
-        // terminator.
-        path_ref =
-            path_ref.rtrim("\r\n").take_front(sizeof(developer_dir_path) - 1);
-        ::memcpy(developer_dir_path, path_ref.data(), path_ref.size());
-        developer_dir_path[path_ref.size()] = '\0';
-        developer_dir_path_valid = true;
-      }
-    }
-
-    if (!developer_dir_path_valid) {
-      FileSpec devel_dir = GetXcodeSelectPath();
-      if (FileSystem::Instance().IsDirectory(devel_dir)) {
-        devel_dir.GetPath(&developer_dir_path[0], sizeof(developer_dir_path));
-        developer_dir_path_valid = true;
-      }
-    }
-
-    if (developer_dir_path_valid) {
-      temp_file_spec.SetFile(developer_dir_path, FileSpec::Style::native);
-      if (FileSystem::Instance().Exists(temp_file_spec)) {
-        m_developer_directory.assign(developer_dir_path);
-        return m_developer_directory.c_str();
-      }
+lldb_private::FileSpec PlatformDarwin::GetXcodeDeveloperDirectory() {
+  static lldb_private::FileSpec g_developer_directory;
+  static llvm::once_flag g_once_flag;
+  llvm::call_once(g_once_flag, []() {
+    if (FileSpec fspec = GetXcodeContentsDirectory()) {
+      fspec.AppendPathComponent("Developer");
+      if (FileSystem::Instance().Exists(fspec))
+        g_developer_directory = fspec;
     }
-    // Assign a single NULL character so we know we tried to find the device
-    // support directory and we don't keep trying to find it over and over.
-    m_developer_directory.assign(1, '\0');
-  }
-
-  // We should have put a single NULL character into m_developer_directory or
-  // it should have a valid path if the code gets here
-  assert(m_developer_directory.empty() == false);
-  if (m_developer_directory[0])
-    return m_developer_directory.c_str();
-  return nullptr;
+  });
+  return g_developer_directory;
 }
 
 BreakpointSP PlatformDarwin::SetThreadCreationBreakpoint(Target &target) {

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
index f6729c508f00..5bdc61c98ae5 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -104,6 +104,7 @@ class PlatformDarwin : public PlatformPOSIX {
   static llvm::StringRef GetSDKNameForType(SDKType type);
   static lldb_private::FileSpec GetXcodeSDK(SDKType type);
   static lldb_private::FileSpec GetXcodeContentsDirectory();
+  static lldb_private::FileSpec GetXcodeDeveloperDirectory();
 
   /// Return the toolchain directroy the current LLDB instance is located in.
   static lldb_private::FileSpec GetCurrentToolchainDirectory();
@@ -176,7 +177,6 @@ class PlatformDarwin : public PlatformPOSIX {
                                              std::vector<std::string> &options,
                                              SDKType sdk_type);
 
-  const char *GetDeveloperDirectory();
 
   lldb_private::Status FindBundleBinaryInExecSearchPaths(
       const lldb_private::ModuleSpec &module_spec,
@@ -188,7 +188,6 @@ class PlatformDarwin : public PlatformPOSIX {
                                          llvm::StringRef component);
   static std::string FindXcodeContentsDirectoryInPath(llvm::StringRef path);
 
-  std::string m_developer_directory;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(PlatformDarwin);

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
index 5859856a4e6d..6d9f20a77369 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
@@ -327,7 +327,7 @@ void PlatformDarwinKernel::CollectKextAndKernelDirectories() {
 
   // DeveloperDirectory is something like
   // "/Applications/Xcode.app/Contents/Developer"
-  std::string developer_dir = GetDeveloperDirectory();
+  std::string developer_dir = GetXcodeDeveloperDirectory().GetPath();
   if (developer_dir.empty())
     developer_dir = "/Applications/Xcode.app/Contents/Developer";
 

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
index bfb3f1e0f6b3..40dd90320151 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
@@ -342,9 +342,8 @@ PlatformRemoteDarwinDevice::GetSDKDirectoryForLatestOSVersion() {
 const char *PlatformRemoteDarwinDevice::GetDeviceSupportDirectory() {
   std::string platform_dir = "/Platforms/" + GetPlatformName() + "/DeviceSupport";
   if (m_device_support_directory.empty()) {
-    const char *device_support_dir = GetDeveloperDirectory();
-    if (device_support_dir) {
-      m_device_support_directory.assign(device_support_dir);
+    if (FileSpec fspec = GetXcodeDeveloperDirectory()) {
+      m_device_support_directory = fspec.GetPath();
       m_device_support_directory.append(platform_dir.c_str());
     } else {
       // Assign a single NULL character so we know we tried to find the device

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformiOSSimulator.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformiOSSimulator.cpp
index 375ddd072262..b09c4051e285 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformiOSSimulator.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformiOSSimulator.cpp
@@ -260,14 +260,14 @@ EnumerateDirectoryCallback(void *baton, llvm::sys::fs::file_type ft,
 const char *PlatformiOSSimulator::GetSDKDirectoryAsCString() {
   std::lock_guard<std::mutex> guard(m_sdk_dir_mutex);
   if (m_sdk_directory.empty()) {
-    const char *developer_dir = GetDeveloperDirectory();
-    if (developer_dir) {
+    if (FileSpec fspec = GetXcodeDeveloperDirectory()) {
+      std::string developer_dir = fspec.GetPath();
       char sdks_directory[PATH_MAX];
       char sdk_dirname[PATH_MAX];
       sdk_dirname[0] = '\0';
       snprintf(sdks_directory, sizeof(sdks_directory),
                "%s/Platforms/iPhoneSimulator.platform/Developer/SDKs",
-               developer_dir);
+               developer_dir.c_str());
       FileSpec simulator_sdk_spec;
       bool find_directories = true;
       bool find_files = false;


        


More information about the lldb-commits mailing list