Skip to content

Commit a114ec0

Browse files
committed
[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
1 parent b97aa41 commit a114ec0

File tree

5 files changed

+260
-67
lines changed

5 files changed

+260
-67
lines changed

lldb/include/lldb/Target/Platform.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,24 @@ class Platform : public PluginInterface {
9999
const ArchSpec &process_host_arch = {},
100100
ArchSpec *platform_arch_ptr = nullptr);
101101

102+
/// Get the platform for the given list of architectures.
103+
///
104+
/// The algorithm works a follows:
105+
///
106+
/// 1. Returns the selected platform if it matches any of the architectures.
107+
/// 2. Returns the host platform if it matches any of the architectures.
108+
/// 3. Returns the platform that matches all the architectures.
109+
///
110+
/// If none of the above apply, this function returns a default platform. The
111+
/// candidates output argument differentiates between either no platforms
112+
/// supporting the given architecture or multiple platforms supporting the
113+
/// given architecture.
114+
static lldb::PlatformSP
115+
GetPlatformForArchitectures(std::vector<ArchSpec> archs,
116+
const ArchSpec &process_host_arch,
117+
lldb::PlatformSP selected_platform_sp,
118+
std::vector<lldb::PlatformSP> &candidates);
119+
102120
static const char *GetHostPlatformName();
103121

104122
static void SetHostPlatform(const lldb::PlatformSP &platform_sp);
@@ -871,6 +889,9 @@ class Platform : public PluginInterface {
871889
virtual CompilerType GetSiginfoType(const llvm::Triple &triple);
872890

873891
protected:
892+
/// For unit testing purposes only.
893+
static void Clear();
894+
874895
/// Create a list of ArchSpecs with the given OS and a architectures. The
875896
/// vendor field is left as an "unspecified unknown".
876897
static std::vector<ArchSpec>

lldb/source/Target/Platform.cpp

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
#include "lldb/Utility/Log.h"
3939
#include "lldb/Utility/Status.h"
4040
#include "lldb/Utility/StructuredData.h"
41+
#include "llvm/ADT/STLExtras.h"
4142
#include "llvm/Support/FileSystem.h"
4243
#include "llvm/Support/Path.h"
4344

@@ -156,6 +157,8 @@ void Platform::Terminate() {
156157
}
157158
}
158159

160+
void Platform::Clear() { GetPlatformList().clear(); }
161+
159162
PlatformProperties &Platform::GetGlobalPlatformProperties() {
160163
static PlatformProperties g_settings;
161164
return g_settings;
@@ -1216,6 +1219,61 @@ Platform::GetPlatformForArchitecture(const ArchSpec &arch,
12161219
return platform_sp;
12171220
}
12181221

1222+
lldb::PlatformSP Platform::GetPlatformForArchitectures(
1223+
std::vector<ArchSpec> archs, const ArchSpec &process_host_arch,
1224+
lldb::PlatformSP selected_platform_sp,
1225+
std::vector<lldb::PlatformSP> &candidates) {
1226+
candidates.clear();
1227+
candidates.reserve(archs.size());
1228+
1229+
if (archs.empty())
1230+
return nullptr;
1231+
1232+
PlatformSP host_platform_sp = Platform::GetHostPlatform();
1233+
1234+
// Prefer the selected platform if it matches at least one architecture.
1235+
if (selected_platform_sp) {
1236+
for (const ArchSpec &arch : archs) {
1237+
if (selected_platform_sp->IsCompatibleArchitecture(
1238+
arch, process_host_arch, false, nullptr))
1239+
return selected_platform_sp;
1240+
}
1241+
}
1242+
1243+
// Prefer the host platform if it matches at least one architecture.
1244+
if (host_platform_sp) {
1245+
for (const ArchSpec &arch : archs) {
1246+
if (host_platform_sp->IsCompatibleArchitecture(arch, process_host_arch,
1247+
false, nullptr))
1248+
return host_platform_sp;
1249+
}
1250+
}
1251+
1252+
// Collect a list of candidate platforms for the architectures.
1253+
for (const ArchSpec &arch : archs) {
1254+
if (PlatformSP platform =
1255+
Platform::GetPlatformForArchitecture(arch, process_host_arch))
1256+
candidates.push_back(platform);
1257+
}
1258+
1259+
// The selected or host platform didn't match any of the architectures. If
1260+
// the same platform supports all architectures then that's the obvious next
1261+
// best thing.
1262+
if (candidates.size() == archs.size()) {
1263+
if (std::all_of(candidates.begin(), candidates.end(),
1264+
[&](const PlatformSP &p) -> bool {
1265+
return p->GetName() == candidates.front()->GetName();
1266+
})) {
1267+
return candidates.front();
1268+
}
1269+
}
1270+
1271+
// At this point we either have no platforms that match the given
1272+
// architectures or multiple platforms with no good way to disambiguate
1273+
// between them.
1274+
return nullptr;
1275+
}
1276+
12191277
std::vector<ArchSpec>
12201278
Platform::CreateArchList(llvm::ArrayRef<llvm::Triple::ArchType> archs,
12211279
llvm::Triple::OSType os) {

lldb/source/Target/TargetList.cpp

Lines changed: 18 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -171,80 +171,31 @@ Status TargetList::CreateTargetInternal(
171171
} else {
172172
// Fat binary. No architecture specified, check if there is
173173
// only one platform for all of the architectures.
174-
PlatformSP host_platform_sp = Platform::GetHostPlatform();
175-
std::vector<PlatformSP> platforms;
176-
for (size_t i = 0; i < num_specs; ++i) {
177-
ModuleSpec module_spec;
178-
if (module_specs.GetModuleSpecAtIndex(i, module_spec)) {
179-
// First consider the platform specified by the user, if any, and
180-
// the selected platform otherwise.
181-
if (platform_sp) {
182-
if (platform_sp->IsCompatibleArchitecture(
183-
module_spec.GetArchitecture(), {}, false, nullptr)) {
184-
platforms.push_back(platform_sp);
185-
continue;
186-
}
187-
}
188-
189-
// Now consider the host platform if it is different from the
190-
// specified/selected platform.
191-
if (host_platform_sp &&
192-
(!platform_sp ||
193-
host_platform_sp->GetName() != platform_sp->GetName())) {
194-
if (host_platform_sp->IsCompatibleArchitecture(
195-
module_spec.GetArchitecture(), {}, false, nullptr)) {
196-
platforms.push_back(host_platform_sp);
197-
continue;
198-
}
199-
}
200-
201-
// Finally find a platform that matches the architecture in the
202-
// executable file.
203-
PlatformSP fallback_platform_sp(
204-
Platform::GetPlatformForArchitecture(
205-
module_spec.GetArchitecture()));
206-
if (fallback_platform_sp) {
207-
platforms.push_back(fallback_platform_sp);
208-
}
209-
}
210-
}
211-
212-
Platform *platform_ptr = nullptr;
213-
bool more_than_one_platforms = false;
214-
for (const auto &the_platform_sp : platforms) {
215-
if (platform_ptr) {
216-
if (platform_ptr->GetName() != the_platform_sp->GetName()) {
217-
more_than_one_platforms = true;
218-
platform_ptr = nullptr;
219-
break;
220-
}
221-
} else {
222-
platform_ptr = the_platform_sp.get();
223-
}
224-
}
225-
226-
if (platform_ptr) {
227-
// All platforms for all modules in the executable match, so we can
228-
// select this platform.
229-
platform_sp = platforms.front();
230-
} else if (!more_than_one_platforms) {
231-
// No platforms claim to support this file.
174+
std::vector<PlatformSP> candidates;
175+
std::vector<ArchSpec> archs;
176+
for (const ModuleSpec &spec : module_specs.ModuleSpecs())
177+
archs.push_back(spec.GetArchitecture());
178+
if (PlatformSP platform_for_archs_sp =
179+
Platform::GetPlatformForArchitectures(archs, {}, platform_sp,
180+
candidates)) {
181+
platform_sp = platform_for_archs_sp;
182+
} else if (candidates.empty()) {
232183
error.SetErrorString("no matching platforms found for this file");
233184
return error;
234185
} else {
235186
// More than one platform claims to support this file.
236187
StreamString error_strm;
237-
std::set<Platform *> platform_set;
188+
std::set<llvm::StringRef> platform_set;
238189
error_strm.Printf(
239190
"more than one platform supports this executable (");
240-
for (const auto &the_platform_sp : platforms) {
241-
if (platform_set.find(the_platform_sp.get()) ==
242-
platform_set.end()) {
243-
if (!platform_set.empty())
244-
error_strm.PutCString(", ");
245-
error_strm.PutCString(the_platform_sp->GetName());
246-
platform_set.insert(the_platform_sp.get());
247-
}
191+
for (const auto &candidate : candidates) {
192+
llvm::StringRef platform_name = candidate->GetName();
193+
if (platform_set.count(platform_name))
194+
continue;
195+
if (!platform_set.empty())
196+
error_strm.PutCString(", ");
197+
error_strm.PutCString(platform_name);
198+
platform_set.insert(platform_name);
248199
}
249200
error_strm.Printf("), specify an architecture to disambiguate");
250201
error.SetErrorString(error_strm.GetString());

lldb/unittests/Platform/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ add_lldb_unittest(LLDBPlatformTests
33
PlatformDarwinTest.cpp
44
PlatformMacOSXTest.cpp
55
PlatformSiginfoTest.cpp
6+
PlatformTest.cpp
67

78
LINK_LIBS
89
lldbPluginPlatformFreeBSD
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
//===-- PlatformTest.cpp --------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "gtest/gtest.h"
10+
11+
#include "Plugins/Platform/POSIX/PlatformPOSIX.h"
12+
#include "TestingSupport/SubsystemRAII.h"
13+
#include "lldb/Core/PluginManager.h"
14+
#include "lldb/Host/FileSystem.h"
15+
#include "lldb/Host/HostInfo.h"
16+
#include "lldb/Target/Platform.h"
17+
18+
using namespace lldb;
19+
using namespace lldb_private;
20+
21+
class TestPlatform : public PlatformPOSIX {
22+
public:
23+
TestPlatform() : PlatformPOSIX(false) {}
24+
using Platform::Clear;
25+
};
26+
27+
class PlatformArm : public TestPlatform {
28+
public:
29+
PlatformArm() = default;
30+
31+
std::vector<ArchSpec>
32+
GetSupportedArchitectures(const ArchSpec &process_host_arch) override {
33+
return {ArchSpec("arm64-apple-ps4")};
34+
}
35+
36+
llvm::StringRef GetPluginName() override { return "arm"; }
37+
llvm::StringRef GetDescription() override { return "arm"; }
38+
};
39+
40+
class PlatformIntel : public TestPlatform {
41+
public:
42+
PlatformIntel() = default;
43+
44+
std::vector<ArchSpec>
45+
GetSupportedArchitectures(const ArchSpec &process_host_arch) override {
46+
return {ArchSpec("x86_64-apple-ps4")};
47+
}
48+
49+
llvm::StringRef GetPluginName() override { return "intel"; }
50+
llvm::StringRef GetDescription() override { return "intel"; }
51+
};
52+
53+
class PlatformThumb : public TestPlatform {
54+
public:
55+
static void Initialize() {
56+
PluginManager::RegisterPlugin("thumb", "thumb",
57+
PlatformThumb::CreateInstance);
58+
}
59+
static void Terminate() {
60+
PluginManager::UnregisterPlugin(PlatformThumb::CreateInstance);
61+
}
62+
63+
static PlatformSP CreateInstance(bool force, const ArchSpec *arch) {
64+
return std::make_shared<PlatformThumb>();
65+
}
66+
67+
std::vector<ArchSpec>
68+
GetSupportedArchitectures(const ArchSpec &process_host_arch) override {
69+
return {ArchSpec("thumbv7-apple-ps4"), ArchSpec("thumbv7f-apple-ps4")};
70+
}
71+
72+
llvm::StringRef GetPluginName() override { return "thumb"; }
73+
llvm::StringRef GetDescription() override { return "thumb"; }
74+
};
75+
76+
class PlatformTest : public ::testing::Test {
77+
SubsystemRAII<FileSystem, HostInfo> subsystems;
78+
void SetUp() override { TestPlatform::Clear(); }
79+
};
80+
81+
TEST_F(PlatformTest, GetPlatformForArchitecturesHost) {
82+
const PlatformSP host_platform_sp = std::make_shared<PlatformArm>();
83+
Platform::SetHostPlatform(host_platform_sp);
84+
ASSERT_EQ(Platform::GetHostPlatform(), host_platform_sp);
85+
86+
const std::vector<ArchSpec> archs = {ArchSpec("arm64-apple-ps4"),
87+
ArchSpec("arm64e-apple-ps4")};
88+
std::vector<PlatformSP> candidates;
89+
90+
// The host platform matches all architectures.
91+
PlatformSP platform_sp =
92+
Platform::GetPlatformForArchitectures(archs, {}, {}, candidates);
93+
ASSERT_TRUE(platform_sp);
94+
EXPECT_EQ(platform_sp, host_platform_sp);
95+
}
96+
97+
TEST_F(PlatformTest, GetPlatformForArchitecturesSelected) {
98+
const PlatformSP host_platform_sp = std::make_shared<PlatformIntel>();
99+
Platform::SetHostPlatform(host_platform_sp);
100+
ASSERT_EQ(Platform::GetHostPlatform(), host_platform_sp);
101+
102+
const std::vector<ArchSpec> archs = {ArchSpec("arm64-apple-ps4"),
103+
ArchSpec("arm64e-apple-ps4")};
104+
std::vector<PlatformSP> candidates;
105+
106+
// The host platform matches no architectures. No selected platform.
107+
PlatformSP platform_sp =
108+
Platform::GetPlatformForArchitectures(archs, {}, {}, candidates);
109+
ASSERT_FALSE(platform_sp);
110+
111+
// The selected platform matches all architectures.
112+
const PlatformSP selected_platform_sp = std::make_shared<PlatformArm>();
113+
platform_sp = Platform::GetPlatformForArchitectures(
114+
archs, {}, selected_platform_sp, candidates);
115+
ASSERT_TRUE(platform_sp);
116+
EXPECT_EQ(platform_sp, selected_platform_sp);
117+
}
118+
119+
TEST_F(PlatformTest, GetPlatformForArchitecturesSelectedOverHost) {
120+
const PlatformSP host_platform_sp = std::make_shared<PlatformIntel>();
121+
Platform::SetHostPlatform(host_platform_sp);
122+
ASSERT_EQ(Platform::GetHostPlatform(), host_platform_sp);
123+
124+
const std::vector<ArchSpec> archs = {ArchSpec("arm64-apple-ps4"),
125+
ArchSpec("x86_64-apple-ps4")};
126+
std::vector<PlatformSP> candidates;
127+
128+
// The host platform matches one architecture. No selected platform.
129+
PlatformSP platform_sp =
130+
Platform::GetPlatformForArchitectures(archs, {}, {}, candidates);
131+
ASSERT_TRUE(platform_sp);
132+
EXPECT_EQ(platform_sp, host_platform_sp);
133+
134+
// The selected and host platform each match one architecture.
135+
// The selected platform is preferred.
136+
const PlatformSP selected_platform_sp = std::make_shared<PlatformArm>();
137+
platform_sp = Platform::GetPlatformForArchitectures(
138+
archs, {}, selected_platform_sp, candidates);
139+
ASSERT_TRUE(platform_sp);
140+
EXPECT_EQ(platform_sp, selected_platform_sp);
141+
}
142+
143+
TEST_F(PlatformTest, GetPlatformForArchitecturesCandidates) {
144+
PlatformThumb::Initialize();
145+
146+
const PlatformSP host_platform_sp = std::make_shared<PlatformIntel>();
147+
Platform::SetHostPlatform(host_platform_sp);
148+
ASSERT_EQ(Platform::GetHostPlatform(), host_platform_sp);
149+
const PlatformSP selected_platform_sp = std::make_shared<PlatformArm>();
150+
151+
const std::vector<ArchSpec> archs = {ArchSpec("thumbv7-apple-ps4"),
152+
ArchSpec("thumbv7f-apple-ps4")};
153+
std::vector<PlatformSP> candidates;
154+
155+
// The host platform matches one architecture. No selected platform.
156+
PlatformSP platform_sp = Platform::GetPlatformForArchitectures(
157+
archs, {}, selected_platform_sp, candidates);
158+
ASSERT_TRUE(platform_sp);
159+
EXPECT_EQ(platform_sp->GetName(), "thumb");
160+
161+
PlatformThumb::Terminate();
162+
}

0 commit comments

Comments
 (0)