JDevlieghere created this revision. JDevlieghere added a reviewer: aprantl. Herald added a subscriber: mgorny.
The current implementation isn't very resilient when it comes to the output of `xcrun`. Currently it cannot deal with: - Trailing newlines. - Leading newlines and errors/warnings before the Xcode path. - Xcode not being named `Xcode.app`. This extract the logic into a helper in PlatformMacOSX and fixes those issues. Repository: rLLDB LLDB https://reviews.llvm.org/D76261 Files: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h lldb/unittests/Platform/CMakeLists.txt lldb/unittests/Platform/PlatformMacOSXTest.cpp
Index: lldb/unittests/Platform/PlatformMacOSXTest.cpp =================================================================== --- /dev/null +++ lldb/unittests/Platform/PlatformMacOSXTest.cpp @@ -0,0 +1,66 @@ +//===-- PlatformMacOSXTest.cpp --------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Plugins/Platform/MacOSX/PlatformMacOSX.h" +#include "llvm/ADT/StringRef.h" +#include "gtest/gtest.h" +#include <tuple> + +using namespace lldb; +using namespace lldb_private; + +struct PlatformMacOSXTester : public PlatformMacOSX { +public: + using PlatformMacOSX::ComputeContentsPath; +}; + +TEST(PlatformMacOSXTest, ComputeContentsPath) { + PlatformMacOSX::XcodeSDK standard( + "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/" + "Developer/SDKs/MacOSX10.15.sdk"); + EXPECT_FALSE(static_cast<bool>(standard)); + EXPECT_TRUE(PlatformMacOSXTester::ComputeContentsPath(standard)); + EXPECT_TRUE(static_cast<bool>(standard)); + EXPECT_EQ(standard.default_sdk, + "/Applications/Xcode.app/Contents/Developer/Platforms/" + "MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk"); + EXPECT_EQ(standard.contents_path, "/Applications/Xcode.app/Contents"); + + PlatformMacOSX::XcodeSDK beta( + "/Applications/Xcode-beta.app/Contents/Developer/Platforms/" + "MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk"); + EXPECT_FALSE(static_cast<bool>(beta)); + EXPECT_TRUE(PlatformMacOSXTester::ComputeContentsPath(beta)); + EXPECT_TRUE(static_cast<bool>(beta)); + EXPECT_EQ(beta.default_sdk, + "/Applications/Xcode-beta.app/Contents/Developer/Platforms/" + "MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk"); + EXPECT_EQ(beta.contents_path, "/Applications/Xcode-beta.app/Contents"); + + PlatformMacOSX::XcodeSDK no_app( + "/Applications/Xcode/Contents/Developer/Platforms/MacOSX.platform/" + "Developer/SDKs/MacOSX10.15.sdk"); + EXPECT_FALSE(static_cast<bool>(no_app)); + EXPECT_FALSE(PlatformMacOSXTester::ComputeContentsPath(no_app)); + EXPECT_FALSE(static_cast<bool>(no_app)); + EXPECT_EQ(no_app.default_sdk, + "/Applications/Xcode/Contents/Developer/Platforms/MacOSX.platform/" + "Developer/SDKs/MacOSX10.15.sdk"); + EXPECT_EQ(no_app.contents_path, ""); + + PlatformMacOSX::XcodeSDK no_contents( + "/Applications/Xcode.app/Developer/Platforms/MacOSX.platform/Developer/" + "SDKs/MacOSX10.15.sdk"); + EXPECT_FALSE(static_cast<bool>(no_contents)); + EXPECT_FALSE(PlatformMacOSXTester::ComputeContentsPath(no_contents)); + EXPECT_FALSE(static_cast<bool>(no_contents)); + EXPECT_EQ(no_contents.default_sdk, + "/Applications/Xcode.app/Developer/Platforms/MacOSX.platform/" + "Developer/SDKs/MacOSX10.15.sdk"); + EXPECT_EQ(no_contents.contents_path, ""); +} Index: lldb/unittests/Platform/CMakeLists.txt =================================================================== --- lldb/unittests/Platform/CMakeLists.txt +++ lldb/unittests/Platform/CMakeLists.txt @@ -1,5 +1,6 @@ add_lldb_unittest(LLDBPlatformTests PlatformDarwinTest.cpp + PlatformMacOSXTest.cpp LINK_LIBS lldbPluginPlatformMacOSX Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h =================================================================== --- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h +++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h @@ -76,6 +76,22 @@ target, options, PlatformDarwin::SDKType::MacOSX); } + struct XcodeSDK { + std::string default_sdk; + std::string contents_path; + XcodeSDK(std::string sdk = {}) + : default_sdk(std::move(sdk)), contents_path() {} + explicit operator bool() { + return !default_sdk.empty() && !contents_path.empty(); + } + }; + + /// Get the SDK path from xcrun. + static XcodeSDK GetXcodeSDK(bool use_xcrun = true); + +protected: + static bool ComputeContentsPath(XcodeSDK &sdk); + private: DISALLOW_COPY_AND_ASSIGN(PlatformMacOSX); }; Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp =================================================================== --- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp +++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp @@ -183,70 +183,101 @@ /// inherited from by the plug-in instance. PlatformMacOSX::~PlatformMacOSX() {} +bool PlatformMacOSX::ComputeContentsPath(XcodeSDK &sdk) { + auto begin = llvm::sys::path::begin(sdk.default_sdk); + auto end = llvm::sys::path::end(sdk.default_sdk); + + // Iterate over the path components until we find something that ends with + // .app. If the next component is Contents then we've found the Xcode content + // directory. + for (auto it = begin; it != end; ++it) { + if (it->endswith(".app")) { + auto next = it; + if (++next != end && *next == "Contents") { + llvm::SmallString<128> buffer; + llvm::sys::path::append(buffer, begin, ++next); + sdk.contents_path = buffer.str().str(); + return true; + } + } + } + + return false; +} + +PlatformMacOSX::XcodeSDK PlatformMacOSX::GetXcodeSDK(bool use_xcrun) { + if (FileSpec fspec = HostInfo::GetShlibDir()) { + if (FileSystem::Instance().Exists(fspec)) { + XcodeSDK sdk(fspec.GetPath()); + if (ComputeContentsPath(sdk)) + return sdk; + } + } + + // If we cannot use xcrun then we're done. + if (!use_xcrun) + return {}; + + int status = 0; + int signo = 0; + std::string output_str; + lldb_private::Status error = Host::RunShellCommand( + "xcrun -sdk macosx --show-sdk-path", FileSpec(), &status, &signo, + &output_str, std::chrono::seconds(3)); + + // Check that xcrun return something. + if (status != 0 || output_str.empty()) + return {}; + + // Convert to a StringRef so we can manipulate the string without modifying + // the underlying data. + llvm::StringRef output(output_str); + + // Remove any trailing newline characters. + output = output.rtrim(); + + // Strip any leading newline characters and everything before them. + const size_t last_newline = output.rfind('\n'); + if (last_newline != llvm::StringRef::npos) + output = output.substr(last_newline + 1); + + // Whatever is left in output should be a valid path. If it's not we're done. + if (!FileSystem::Instance().Exists(output)) + return {}; + + XcodeSDK sdk(output.str()); + if (ComputeContentsPath(sdk)) + return sdk; + + return {}; +} + ConstString PlatformMacOSX::GetSDKDirectory(lldb_private::Target &target) { ModuleSP exe_module_sp(target.GetExecutableModule()); if (exe_module_sp) { ObjectFile *objfile = exe_module_sp->GetObjectFile(); if (objfile) { - std::string xcode_contents_path; - std::string default_xcode_sdk; + XcodeSDK sdk; FileSpec fspec; llvm::VersionTuple version = objfile->GetSDKVersion(); if (!version.empty()) { - fspec = HostInfo::GetShlibDir(); - if (fspec) { - std::string path; - xcode_contents_path = fspec.GetPath(); - size_t pos = xcode_contents_path.find("/Xcode.app/Contents/"); - if (pos != std::string::npos) { - // LLDB.framework is inside an Xcode app bundle, we can locate the - // SDK from here - xcode_contents_path.erase(pos + strlen("/Xcode.app/Contents/")); - } else { - xcode_contents_path.clear(); - // Use the selected Xcode - int status = 0; - int signo = 0; - std::string output; - const char *command = "xcrun -sdk macosx --show-sdk-path"; - lldb_private::Status error = RunShellCommand( - command, // shell command to run - FileSpec(), // current working directory - &status, // Put the exit status of the process in here - &signo, // Put the signal that caused the process to exit in - // here - &output, // Get the output from the command and place it in this - // string - std::chrono::seconds(3)); - if (status == 0 && !output.empty()) { - size_t first_non_newline = output.find_last_not_of("\r\n"); - if (first_non_newline != std::string::npos) - output.erase(first_non_newline + 1); - default_xcode_sdk = output; - - pos = default_xcode_sdk.find("/Xcode.app/Contents/"); - if (pos != std::string::npos) - xcode_contents_path = default_xcode_sdk.substr( - 0, pos + strlen("/Xcode.app/Contents/")); - } - } - } + sdk = GetXcodeSDK(); - if (!xcode_contents_path.empty()) { + if (!sdk.contents_path.empty()) { StreamString sdk_path; - sdk_path.Printf("%sDeveloper/Platforms/MacOSX.platform/Developer/" + sdk_path.Printf("%s/Developer/Platforms/MacOSX.platform/Developer/" "SDKs/MacOSX%u.%u.sdk", - xcode_contents_path.c_str(), version.getMajor(), + sdk.contents_path.c_str(), version.getMajor(), version.getMinor().getValue()); fspec.SetFile(sdk_path.GetString(), FileSpec::Style::native); if (FileSystem::Instance().Exists(fspec)) return ConstString(sdk_path.GetString()); } - if (!default_xcode_sdk.empty()) { - fspec.SetFile(default_xcode_sdk, FileSpec::Style::native); + if (!sdk.default_sdk.empty()) { + fspec.SetFile(sdk.default_sdk, FileSpec::Style::native); if (FileSystem::Instance().Exists(fspec)) - return ConstString(default_xcode_sdk); + return ConstString(sdk.default_sdk); } } }
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
