This revision was automatically updated to reflect the committed changes. Closed by commit rGa114ec0c6dc0: [lldb] Change the way we pick a platform for fat binaries (authored by JDevlieghere). Herald added a project: LLDB.
Changed prior to commit: https://reviews.llvm.org/D122684?vs=419005&id=419280#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122684/new/ https://reviews.llvm.org/D122684 Files: lldb/include/lldb/Target/Platform.h lldb/source/Target/Platform.cpp lldb/source/Target/TargetList.cpp lldb/unittests/Platform/CMakeLists.txt lldb/unittests/Platform/PlatformTest.cpp
Index: lldb/unittests/Platform/PlatformTest.cpp =================================================================== --- /dev/null +++ lldb/unittests/Platform/PlatformTest.cpp @@ -0,0 +1,162 @@ +//===-- PlatformTest.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 "gtest/gtest.h" + +#include "Plugins/Platform/POSIX/PlatformPOSIX.h" +#include "TestingSupport/SubsystemRAII.h" +#include "lldb/Core/PluginManager.h" +#include "lldb/Host/FileSystem.h" +#include "lldb/Host/HostInfo.h" +#include "lldb/Target/Platform.h" + +using namespace lldb; +using namespace lldb_private; + +class TestPlatform : public PlatformPOSIX { +public: + TestPlatform() : PlatformPOSIX(false) {} + using Platform::Clear; +}; + +class PlatformArm : public TestPlatform { +public: + PlatformArm() = default; + + std::vector<ArchSpec> + GetSupportedArchitectures(const ArchSpec &process_host_arch) override { + return {ArchSpec("arm64-apple-ps4")}; + } + + llvm::StringRef GetPluginName() override { return "arm"; } + llvm::StringRef GetDescription() override { return "arm"; } +}; + +class PlatformIntel : public TestPlatform { +public: + PlatformIntel() = default; + + std::vector<ArchSpec> + GetSupportedArchitectures(const ArchSpec &process_host_arch) override { + return {ArchSpec("x86_64-apple-ps4")}; + } + + llvm::StringRef GetPluginName() override { return "intel"; } + llvm::StringRef GetDescription() override { return "intel"; } +}; + +class PlatformThumb : public TestPlatform { +public: + static void Initialize() { + PluginManager::RegisterPlugin("thumb", "thumb", + PlatformThumb::CreateInstance); + } + static void Terminate() { + PluginManager::UnregisterPlugin(PlatformThumb::CreateInstance); + } + + static PlatformSP CreateInstance(bool force, const ArchSpec *arch) { + return std::make_shared<PlatformThumb>(); + } + + std::vector<ArchSpec> + GetSupportedArchitectures(const ArchSpec &process_host_arch) override { + return {ArchSpec("thumbv7-apple-ps4"), ArchSpec("thumbv7f-apple-ps4")}; + } + + llvm::StringRef GetPluginName() override { return "thumb"; } + llvm::StringRef GetDescription() override { return "thumb"; } +}; + +class PlatformTest : public ::testing::Test { + SubsystemRAII<FileSystem, HostInfo> subsystems; + void SetUp() override { TestPlatform::Clear(); } +}; + +TEST_F(PlatformTest, GetPlatformForArchitecturesHost) { + const PlatformSP host_platform_sp = std::make_shared<PlatformArm>(); + Platform::SetHostPlatform(host_platform_sp); + ASSERT_EQ(Platform::GetHostPlatform(), host_platform_sp); + + const std::vector<ArchSpec> archs = {ArchSpec("arm64-apple-ps4"), + ArchSpec("arm64e-apple-ps4")}; + std::vector<PlatformSP> candidates; + + // The host platform matches all architectures. + PlatformSP platform_sp = + Platform::GetPlatformForArchitectures(archs, {}, {}, candidates); + ASSERT_TRUE(platform_sp); + EXPECT_EQ(platform_sp, host_platform_sp); +} + +TEST_F(PlatformTest, GetPlatformForArchitecturesSelected) { + const PlatformSP host_platform_sp = std::make_shared<PlatformIntel>(); + Platform::SetHostPlatform(host_platform_sp); + ASSERT_EQ(Platform::GetHostPlatform(), host_platform_sp); + + const std::vector<ArchSpec> archs = {ArchSpec("arm64-apple-ps4"), + ArchSpec("arm64e-apple-ps4")}; + std::vector<PlatformSP> candidates; + + // The host platform matches no architectures. No selected platform. + PlatformSP platform_sp = + Platform::GetPlatformForArchitectures(archs, {}, {}, candidates); + ASSERT_FALSE(platform_sp); + + // The selected platform matches all architectures. + const PlatformSP selected_platform_sp = std::make_shared<PlatformArm>(); + platform_sp = Platform::GetPlatformForArchitectures( + archs, {}, selected_platform_sp, candidates); + ASSERT_TRUE(platform_sp); + EXPECT_EQ(platform_sp, selected_platform_sp); +} + +TEST_F(PlatformTest, GetPlatformForArchitecturesSelectedOverHost) { + const PlatformSP host_platform_sp = std::make_shared<PlatformIntel>(); + Platform::SetHostPlatform(host_platform_sp); + ASSERT_EQ(Platform::GetHostPlatform(), host_platform_sp); + + const std::vector<ArchSpec> archs = {ArchSpec("arm64-apple-ps4"), + ArchSpec("x86_64-apple-ps4")}; + std::vector<PlatformSP> candidates; + + // The host platform matches one architecture. No selected platform. + PlatformSP platform_sp = + Platform::GetPlatformForArchitectures(archs, {}, {}, candidates); + ASSERT_TRUE(platform_sp); + EXPECT_EQ(platform_sp, host_platform_sp); + + // The selected and host platform each match one architecture. + // The selected platform is preferred. + const PlatformSP selected_platform_sp = std::make_shared<PlatformArm>(); + platform_sp = Platform::GetPlatformForArchitectures( + archs, {}, selected_platform_sp, candidates); + ASSERT_TRUE(platform_sp); + EXPECT_EQ(platform_sp, selected_platform_sp); +} + +TEST_F(PlatformTest, GetPlatformForArchitecturesCandidates) { + PlatformThumb::Initialize(); + + const PlatformSP host_platform_sp = std::make_shared<PlatformIntel>(); + Platform::SetHostPlatform(host_platform_sp); + ASSERT_EQ(Platform::GetHostPlatform(), host_platform_sp); + const PlatformSP selected_platform_sp = std::make_shared<PlatformArm>(); + + const std::vector<ArchSpec> archs = {ArchSpec("thumbv7-apple-ps4"), + ArchSpec("thumbv7f-apple-ps4")}; + std::vector<PlatformSP> candidates; + + // The host platform matches one architecture. No selected platform. + PlatformSP platform_sp = Platform::GetPlatformForArchitectures( + archs, {}, selected_platform_sp, candidates); + ASSERT_TRUE(platform_sp); + EXPECT_EQ(platform_sp->GetName(), "thumb"); + + PlatformThumb::Terminate(); +} Index: lldb/unittests/Platform/CMakeLists.txt =================================================================== --- lldb/unittests/Platform/CMakeLists.txt +++ lldb/unittests/Platform/CMakeLists.txt @@ -3,6 +3,7 @@ PlatformDarwinTest.cpp PlatformMacOSXTest.cpp PlatformSiginfoTest.cpp + PlatformTest.cpp LINK_LIBS lldbPluginPlatformFreeBSD Index: lldb/source/Target/TargetList.cpp =================================================================== --- lldb/source/Target/TargetList.cpp +++ lldb/source/Target/TargetList.cpp @@ -171,80 +171,31 @@ } else { // Fat binary. No architecture specified, check if there is // only one platform for all of the architectures. - PlatformSP host_platform_sp = Platform::GetHostPlatform(); - std::vector<PlatformSP> platforms; - for (size_t i = 0; i < num_specs; ++i) { - ModuleSpec module_spec; - if (module_specs.GetModuleSpecAtIndex(i, module_spec)) { - // First consider the platform specified by the user, if any, and - // the selected platform otherwise. - if (platform_sp) { - if (platform_sp->IsCompatibleArchitecture( - module_spec.GetArchitecture(), {}, false, nullptr)) { - platforms.push_back(platform_sp); - continue; - } - } - - // Now consider the host platform if it is different from the - // specified/selected platform. - if (host_platform_sp && - (!platform_sp || - host_platform_sp->GetName() != platform_sp->GetName())) { - if (host_platform_sp->IsCompatibleArchitecture( - module_spec.GetArchitecture(), {}, false, nullptr)) { - platforms.push_back(host_platform_sp); - continue; - } - } - - // Finally find a platform that matches the architecture in the - // executable file. - PlatformSP fallback_platform_sp( - Platform::GetPlatformForArchitecture( - module_spec.GetArchitecture())); - if (fallback_platform_sp) { - platforms.push_back(fallback_platform_sp); - } - } - } - - Platform *platform_ptr = nullptr; - bool more_than_one_platforms = false; - for (const auto &the_platform_sp : platforms) { - if (platform_ptr) { - if (platform_ptr->GetName() != the_platform_sp->GetName()) { - more_than_one_platforms = true; - platform_ptr = nullptr; - break; - } - } else { - platform_ptr = the_platform_sp.get(); - } - } - - if (platform_ptr) { - // All platforms for all modules in the executable match, so we can - // select this platform. - platform_sp = platforms.front(); - } else if (!more_than_one_platforms) { - // No platforms claim to support this file. + std::vector<PlatformSP> candidates; + std::vector<ArchSpec> archs; + for (const ModuleSpec &spec : module_specs.ModuleSpecs()) + archs.push_back(spec.GetArchitecture()); + if (PlatformSP platform_for_archs_sp = + Platform::GetPlatformForArchitectures(archs, {}, platform_sp, + candidates)) { + platform_sp = platform_for_archs_sp; + } else if (candidates.empty()) { error.SetErrorString("no matching platforms found for this file"); return error; } else { // More than one platform claims to support this file. StreamString error_strm; - std::set<Platform *> platform_set; + std::set<llvm::StringRef> platform_set; error_strm.Printf( "more than one platform supports this executable ("); - for (const auto &the_platform_sp : platforms) { - if (platform_set.find(the_platform_sp.get()) == - platform_set.end()) { - if (!platform_set.empty()) - error_strm.PutCString(", "); - error_strm.PutCString(the_platform_sp->GetName()); - platform_set.insert(the_platform_sp.get()); - } + for (const auto &candidate : candidates) { + llvm::StringRef platform_name = candidate->GetName(); + if (platform_set.count(platform_name)) + continue; + if (!platform_set.empty()) + error_strm.PutCString(", "); + error_strm.PutCString(platform_name); + platform_set.insert(platform_name); } error_strm.Printf("), specify an architecture to disambiguate"); error.SetErrorString(error_strm.GetString()); Index: lldb/source/Target/Platform.cpp =================================================================== --- lldb/source/Target/Platform.cpp +++ lldb/source/Target/Platform.cpp @@ -38,6 +38,7 @@ #include "lldb/Utility/Log.h" #include "lldb/Utility/Status.h" #include "lldb/Utility/StructuredData.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" @@ -156,6 +157,8 @@ } } +void Platform::Clear() { GetPlatformList().clear(); } + PlatformProperties &Platform::GetGlobalPlatformProperties() { static PlatformProperties g_settings; return g_settings; @@ -1216,6 +1219,61 @@ return platform_sp; } +lldb::PlatformSP Platform::GetPlatformForArchitectures( + std::vector<ArchSpec> archs, const ArchSpec &process_host_arch, + lldb::PlatformSP selected_platform_sp, + std::vector<lldb::PlatformSP> &candidates) { + candidates.clear(); + candidates.reserve(archs.size()); + + if (archs.empty()) + return nullptr; + + PlatformSP host_platform_sp = Platform::GetHostPlatform(); + + // Prefer the selected platform if it matches at least one architecture. + if (selected_platform_sp) { + for (const ArchSpec &arch : archs) { + if (selected_platform_sp->IsCompatibleArchitecture( + arch, process_host_arch, false, nullptr)) + return selected_platform_sp; + } + } + + // Prefer the host platform if it matches at least one architecture. + if (host_platform_sp) { + for (const ArchSpec &arch : archs) { + if (host_platform_sp->IsCompatibleArchitecture(arch, process_host_arch, + false, nullptr)) + return host_platform_sp; + } + } + + // Collect a list of candidate platforms for the architectures. + for (const ArchSpec &arch : archs) { + if (PlatformSP platform = + Platform::GetPlatformForArchitecture(arch, process_host_arch)) + candidates.push_back(platform); + } + + // The selected or host platform didn't match any of the architectures. If + // the same platform supports all architectures then that's the obvious next + // best thing. + if (candidates.size() == archs.size()) { + if (std::all_of(candidates.begin(), candidates.end(), + [&](const PlatformSP &p) -> bool { + return p->GetName() == candidates.front()->GetName(); + })) { + return candidates.front(); + } + } + + // At this point we either have no platforms that match the given + // architectures or multiple platforms with no good way to disambiguate + // between them. + return nullptr; +} + std::vector<ArchSpec> Platform::CreateArchList(llvm::ArrayRef<llvm::Triple::ArchType> archs, llvm::Triple::OSType os) { Index: lldb/include/lldb/Target/Platform.h =================================================================== --- lldb/include/lldb/Target/Platform.h +++ lldb/include/lldb/Target/Platform.h @@ -99,6 +99,24 @@ const ArchSpec &process_host_arch = {}, ArchSpec *platform_arch_ptr = nullptr); + /// Get the platform for the given list of architectures. + /// + /// The algorithm works a follows: + /// + /// 1. Returns the selected platform if it matches any of the architectures. + /// 2. Returns the host platform if it matches any of the architectures. + /// 3. Returns the platform that matches all the architectures. + /// + /// If none of the above apply, this function returns a default platform. The + /// candidates output argument differentiates between either no platforms + /// supporting the given architecture or multiple platforms supporting the + /// given architecture. + static lldb::PlatformSP + GetPlatformForArchitectures(std::vector<ArchSpec> archs, + const ArchSpec &process_host_arch, + lldb::PlatformSP selected_platform_sp, + std::vector<lldb::PlatformSP> &candidates); + static const char *GetHostPlatformName(); static void SetHostPlatform(const lldb::PlatformSP &platform_sp); @@ -871,6 +889,9 @@ virtual CompilerType GetSiginfoType(const llvm::Triple &triple); protected: + /// For unit testing purposes only. + static void Clear(); + /// Create a list of ArchSpecs with the given OS and a architectures. The /// vendor field is left as an "unspecified unknown". static std::vector<ArchSpec>
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
