[Lldb-commits] [lldb] a114ec0 - [lldb] Change the way we pick a platform for fat binaries
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Wed Mar 30 15:30:19 PDT 2022
Author: Jonas Devlieghere
Date: 2022-03-30T15:30:05-07:00
New Revision: a114ec0c6dc052832ec3dc1f65c9e221e3272925
URL: https://github.com/llvm/llvm-project/commit/a114ec0c6dc052832ec3dc1f65c9e221e3272925
DIFF: https://github.com/llvm/llvm-project/commit/a114ec0c6dc052832ec3dc1f65c9e221e3272925.diff
LOG: [lldb] Change the way we pick a platform for fat binaries
Currently, when creating a target for a fat binary, we error out if more
than one platforms can support the different architectures in the
binary. There are situations where it makes sense for multiple platforms
to support the same architectures: for example the host and
remote-macosx platform on Darwin.
The only way to currently disambiguate between them is to specify the
architecture. This patch changes that to take into account the selected
and host platform. The new algorithm works a follows:
1. Pick the selected platform if it matches any of the architectures.
2. Pick the host platform if it matches any of the architectures.
3. If there's one platform that works for all architectures, pick that.
If none of the above apply then we either have no platform supporting
the architectures in the fat binary or multiple platforms with no good
way to disambiguate between them.
I've added a bunch of unit tests to codify this new behavior.
rdar://90360204
Differential revision: https://reviews.llvm.org/D122684
Added:
lldb/unittests/Platform/PlatformTest.cpp
Modified:
lldb/include/lldb/Target/Platform.h
lldb/source/Target/Platform.cpp
lldb/source/Target/TargetList.cpp
lldb/unittests/Platform/CMakeLists.txt
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index 9136989fea0bb..eee7e3116beea 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -99,6 +99,24 @@ class Platform : public PluginInterface {
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
diff erentiates 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 @@ class Platform : public PluginInterface {
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>
diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index 2c3bdf3a6bbb1..74e3e38953608 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/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::Terminate() {
}
}
+void Platform::Clear() { GetPlatformList().clear(); }
+
PlatformProperties &Platform::GetGlobalPlatformProperties() {
static PlatformProperties g_settings;
return g_settings;
@@ -1216,6 +1219,61 @@ Platform::GetPlatformForArchitecture(const ArchSpec &arch,
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) {
diff --git a/lldb/source/Target/TargetList.cpp b/lldb/source/Target/TargetList.cpp
index 7481231197016..8d980d000253a 100644
--- a/lldb/source/Target/TargetList.cpp
+++ b/lldb/source/Target/TargetList.cpp
@@ -171,80 +171,31 @@ Status TargetList::CreateTargetInternal(
} 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
diff erent 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());
diff --git a/lldb/unittests/Platform/CMakeLists.txt b/lldb/unittests/Platform/CMakeLists.txt
index 3c23c46916d86..963975602d671 100644
--- a/lldb/unittests/Platform/CMakeLists.txt
+++ b/lldb/unittests/Platform/CMakeLists.txt
@@ -3,6 +3,7 @@ add_lldb_unittest(LLDBPlatformTests
PlatformDarwinTest.cpp
PlatformMacOSXTest.cpp
PlatformSiginfoTest.cpp
+ PlatformTest.cpp
LINK_LIBS
lldbPluginPlatformFreeBSD
diff --git a/lldb/unittests/Platform/PlatformTest.cpp b/lldb/unittests/Platform/PlatformTest.cpp
new file mode 100644
index 0000000000000..e5e94e04ee072
--- /dev/null
+++ b/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();
+}
More information about the lldb-commits
mailing list