Skip to content

Revert "[lldb][HostInfoMacOSX] Try to use DW_AT_LLVM_sysroot instead of xcrun when looking up SDK" #129621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ class HostInfoMacOSX : public HostInfoPosix {
static FileSpec GetXcodeDeveloperDirectory();

/// Query xcrun to find an Xcode SDK directory.
///
/// Note, this is an expensive operation if the SDK we're querying
/// does not exist in an Xcode installation path on the host.
static llvm::Expected<llvm::StringRef> GetSDKRoot(SDKOptions options);
static llvm::Expected<llvm::StringRef> FindSDKTool(XcodeSDK sdk,
llvm::StringRef tool);
Expand Down
5 changes: 0 additions & 5 deletions lldb/include/lldb/Utility/XcodeSDK.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#ifndef LLDB_UTILITY_SDK_H
#define LLDB_UTILITY_SDK_H

#include "lldb/Utility/FileSpec.h"
#include "lldb/lldb-forward.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/VersionTuple.h"
Expand All @@ -24,7 +23,6 @@ namespace lldb_private {
/// An abstraction for Xcode-style SDKs that works like \ref ArchSpec.
class XcodeSDK {
std::string m_name;
FileSpec m_sysroot;

public:
/// Different types of Xcode SDKs.
Expand Down Expand Up @@ -64,8 +62,6 @@ class XcodeSDK {
/// directory component of a path one would pass to clang's -isysroot
/// parameter. For example, "MacOSX.10.14.sdk".
XcodeSDK(std::string &&name) : m_name(std::move(name)) {}
XcodeSDK(std::string name, FileSpec sysroot)
: m_name(std::move(name)), m_sysroot(std::move(sysroot)) {}
static XcodeSDK GetAnyMacOS() { return XcodeSDK("MacOSX.sdk"); }

/// The merge function follows a strict order to maintain monotonicity:
Expand All @@ -83,7 +79,6 @@ class XcodeSDK {
llvm::VersionTuple GetVersion() const;
Type GetType() const;
llvm::StringRef GetString() const;
const FileSpec &GetSysroot() const;
/// Whether this Xcode SDK supports Swift.
bool SupportsSwift() const;

Expand Down
3 changes: 0 additions & 3 deletions lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1425,9 +1425,6 @@ PlatformDarwin::ResolveSDKPathFromDebugInfo(Module &module) {

auto [sdk, _] = std::move(*sdk_or_err);

if (FileSystem::Instance().Exists(sdk.GetSysroot()))
return sdk.GetSysroot().GetPath();

auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk});
if (!path_or_err)
return llvm::createStringError(
Expand Down
29 changes: 12 additions & 17 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -993,26 +993,21 @@ XcodeSDK SymbolFileDWARF::ParseXcodeSDK(CompileUnit &comp_unit) {
const char *sdk = cu_die.GetAttributeValueAsString(DW_AT_APPLE_sdk, nullptr);
if (!sdk)
return {};
std::string sysroot =
const char *sysroot =
cu_die.GetAttributeValueAsString(DW_AT_LLVM_sysroot, "");
// Register the sysroot path remapping with the module belonging to
// the CU as well as the one belonging to the symbol file. The two
// would be different if this is an OSO object and module is the
// corresponding debug map, in which case both should be updated.
ModuleSP module_sp = comp_unit.GetModule();
if (module_sp)
module_sp->RegisterXcodeSDK(sdk, sysroot);

// RegisterXcodeSDK calls into xcrun which is not aware of CLT, which is
// expensive.
if (sysroot.find("/Library/Developer/CommandLineTools/SDKs") != 0) {
// Register the sysroot path remapping with the module belonging to
// the CU as well as the one belonging to the symbol file. The two
// would be different if this is an OSO object and module is the
// corresponding debug map, in which case both should be updated.
ModuleSP module_sp = comp_unit.GetModule();
if (module_sp)
module_sp->RegisterXcodeSDK(sdk, sysroot);

ModuleSP local_module_sp = m_objfile_sp->GetModule();
if (local_module_sp && local_module_sp != module_sp)
local_module_sp->RegisterXcodeSDK(sdk, sysroot);
}
ModuleSP local_module_sp = m_objfile_sp->GetModule();
if (local_module_sp && local_module_sp != module_sp)
local_module_sp->RegisterXcodeSDK(sdk, sysroot);

return {sdk, FileSpec{std::move(sysroot)}};
return {sdk};
}

size_t SymbolFileDWARF::ParseFunctions(CompileUnit &comp_unit) {
Expand Down
17 changes: 4 additions & 13 deletions lldb/source/Utility/XcodeSDK.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,6 @@ XcodeSDK::Type XcodeSDK::GetType() const {

llvm::StringRef XcodeSDK::GetString() const { return m_name; }

const FileSpec &XcodeSDK::GetSysroot() const { return m_sysroot; }

bool XcodeSDK::Info::operator<(const Info &other) const {
return std::tie(type, version, internal) <
std::tie(other.type, other.version, other.internal);
Expand All @@ -155,24 +153,17 @@ bool XcodeSDK::Info::operator==(const Info &other) const {
}

void XcodeSDK::Merge(const XcodeSDK &other) {
auto add_internal_sdk_suffix = [](llvm::StringRef sdk) {
return (sdk.substr(0, sdk.size() - 3) + "Internal.sdk").str();
};

// The "bigger" SDK always wins.
auto l = Parse();
auto r = other.Parse();
if (l < r)
*this = other;
else {
// The Internal flag always wins.
if (!l.internal && r.internal) {
if (llvm::StringRef(m_name).ends_with(".sdk"))
m_name = add_internal_sdk_suffix(m_name);

if (m_sysroot.GetFileNameExtension() == ".sdk")
m_sysroot.SetFilename(add_internal_sdk_suffix(m_sysroot.GetFilename()));
}
if (llvm::StringRef(m_name).ends_with(".sdk"))
if (!l.internal && r.internal)
m_name =
m_name.substr(0, m_name.size() - 3) + std::string("Internal.sdk");
}
}

Expand Down
19 changes: 2 additions & 17 deletions lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -307,28 +307,13 @@ SDKPathParsingTestData sdkPathParsingTestCases[] = {
.expect_internal_sdk = true,
.expect_sdk_path_pattern = "Internal.sdk"},

/// Two CUs with a public (non-CommandLineTools) SDK each
{.input_sdk_paths = {"/Path/To/SDKs/iPhoneOS14.1.sdk",
"/Path/To/SDKs/MacOSX11.3.sdk"},
.expect_mismatch = false,
.expect_internal_sdk = false,
.expect_sdk_path_pattern = "iPhoneOS14.1.sdk"},

/// One CU with CommandLineTools and the other a public SDK
/// Two CUs with an internal SDK each
{.input_sdk_paths =
{"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.1.sdk",
"/Path/To/SDKs/MacOSX11.3.sdk"},
"/Library/Developer/CommandLineTools/SDKs/MacOSX11.3.sdk"},
.expect_mismatch = false,
.expect_internal_sdk = false,
.expect_sdk_path_pattern = "iPhoneOS14.1.sdk"},

/// One CU with CommandLineTools and the other an internal SDK
{.input_sdk_paths =
{"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.1.sdk",
"/Path/To/SDKs/MacOSX11.3.Internal.sdk"},
.expect_mismatch = false,
.expect_internal_sdk = true,
.expect_sdk_path_pattern = "iPhoneOS14.1.Internal.sdk"},
};

INSTANTIATE_TEST_SUITE_P(SDKPathParsingTests, SDKPathParsingMultiparamTests,
Expand Down
13 changes: 0 additions & 13 deletions lldb/unittests/Utility/XcodeSDKTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,12 @@ TEST(XcodeSDKTest, ParseTest) {
EXPECT_EQ(XcodeSDK("MacOSX10.9.sdk").GetVersion(), llvm::VersionTuple(10, 9));
EXPECT_EQ(XcodeSDK("MacOSX10.15.4.sdk").GetVersion(), llvm::VersionTuple(10, 15));
EXPECT_EQ(XcodeSDK("MacOSX.sdk").IsAppleInternalSDK(), false);
EXPECT_EQ(
XcodeSDK("MacOSX.sdk", FileSpec{"/Path/To/MacOSX.sdk"}).GetSysroot(),
FileSpec("/Path/To/MacOSX.sdk"));
EXPECT_EQ(XcodeSDK("MacOSX10.15.Internal.sdk").GetType(), XcodeSDK::MacOSX);
EXPECT_EQ(XcodeSDK("MacOSX10.15.Internal.sdk").GetVersion(),
llvm::VersionTuple(10, 15));
EXPECT_EQ(XcodeSDK("MacOSX10.15.Internal.sdk").IsAppleInternalSDK(), true);
EXPECT_FALSE(XcodeSDK("MacOSX10.15.Internal.sdk").GetSysroot());
EXPECT_EQ(XcodeSDK().GetType(), XcodeSDK::unknown);
EXPECT_EQ(XcodeSDK().GetVersion(), llvm::VersionTuple());
EXPECT_FALSE(XcodeSDK().GetSysroot());
}

TEST(XcodeSDKTest, MergeTest) {
Expand All @@ -65,14 +60,6 @@ TEST(XcodeSDKTest, MergeTest) {
XcodeSDK empty;
empty.Merge(XcodeSDK("MacOSX10.14.Internal.sdk"));
EXPECT_EQ(empty.GetString(), llvm::StringRef("MacOSX10.14.Internal.sdk"));
EXPECT_FALSE(empty.GetSysroot());
empty.Merge(XcodeSDK("MacOSX9.5.Internal.sdk", FileSpec{"/Path/To/9.5.sdk"}));
EXPECT_FALSE(empty.GetSysroot());
empty.Merge(XcodeSDK("MacOSX12.5.sdk", FileSpec{"/Path/To/12.5.sdk"}));
EXPECT_EQ(empty.GetSysroot(), FileSpec{"/Path/To/12.5.sdk"});
empty.Merge(XcodeSDK("MacOSX11.5.Internal.sdk",
FileSpec{"/Path/To/12.5.Internal.sdk"}));
EXPECT_EQ(empty.GetSysroot(), FileSpec{"/Path/To/12.5.Internal.sdk"});
}

#ifndef _WIN32
Expand Down
Loading