[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

diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
index 41c6999..cb6fbce 100644 (file)
--- 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 7e3e7c9..fa5f93b 100644 (file)
--- 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 e249236..6cd2016 100644 (file)
--- 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 350043f..e163300 100644 (file)
--- 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 f6729c5..5bdc61c 100644 (file)
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -104,6 +104,7 @@ public:
   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 @@ protected:
                                              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 @@ protected:
                                          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 5859856..6d9f20a 100644 (file)
--- 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 bfb3f1e..40dd903 100644 (file)
--- 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 375ddd0..b09c405 100644 (file)
--- 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;