maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[lldb] Refactor Platform::ResolveExecutable 

Module resolution is probably the most complex piece of lldb [citation
needed], with numerous levels of abstraction, each one implementing
various retry and fallback strategies.

It is also a very repetitive, with only small differences between
"host", "remote-and-connected" and "remote-but-not-(yet)-connected"
scenarios.

The goal of this patch (first in series) is to reduce the number of
abstractions, and deduplicate the code.

One of the reasons for this complexity is the tension between the desire
to offload the process of module resolution to the remote platform
instance (that's how most other platform methods work), and the desire
to keep it local to the outer platform class (its easier to subclass the
outer class, and it generally makes more sense).

This patch resolves that conflict in favour of doing everything in the
outer class. The gdb-remote (our only remote platform) implementation of
ResolveExecutable was not doing anything gdb-specific, and was rather
similar to the other implementations of that method (any divergence is
most likely the result of fixes not being applied everywhere rather than
intentional).

It does this by excising the remote platform out of the resolution
codepath. The gdb-remote implementation of ResolveExecutable is moved to
Platform::ResolveRemoteExecutable, and the (only) call site is
redirected to that. On its own, this does not achieve (much), but it
creates new opportunities for layer peeling and code sharing, since all
of the code now lives closer together.

Differential Revision: https://reviews.llvm.org/D113487
This commit is contained in:
Pavel Labath 2021-11-09 16:32:57 +01:00
parent 6c48f6aafe
commit 098c01c132
6 changed files with 110 additions and 113 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
lldb/include/lldb/Target/Platform.h
View File

@ -943,8 +943,7 @@ protected:
virtual void CalculateTrapHandlerSymbolNames() = 0;
Status GetCachedExecutable(ModuleSpec &module_spec, lldb::ModuleSP &module_sp,
const FileSpecList *module_search_paths_ptr,
Platform &remote_platform);
const FileSpecList *module_search_paths_ptr);
virtual Status DownloadModuleSlice(const FileSpec &src_file_spec,
const uint64_t src_offset,
@ -956,6 +955,11 @@ protected:
virtual const char *GetCacheHostname();
virtual Status
ResolveRemoteExecutable(const ModuleSpec &module_spec,
lldb::ModuleSP &exe_module_sp,
const FileSpecList *module_search_paths_ptr);
private:
typedef std::function<Status(const ModuleSpec &)> ModuleResolver;
@ -969,8 +973,7 @@ private:
Status LoadCachedExecutable(const ModuleSpec &module_spec,
lldb::ModuleSP &module_sp,
const FileSpecList *module_search_paths_ptr,
Platform &remote_platform);
const FileSpecList *module_search_paths_ptr);
FileSpec GetModuleCacheRoot();
};

70
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
View File

@ -89,76 +89,6 @@ llvm::StringRef PlatformRemoteGDBServer::GetDescription() {
return GetDescriptionStatic();
}
Status PlatformRemoteGDBServer::ResolveExecutable(
const ModuleSpec &module_spec, lldb::ModuleSP &exe_module_sp,
const FileSpecList *module_search_paths_ptr) {
// copied from PlatformRemoteiOS
Status error;
// Nothing special to do here, just use the actual file and architecture
ModuleSpec resolved_module_spec(module_spec);
// Resolve any executable within an apk on Android?
// Host::ResolveExecutableInBundle (resolved_module_spec.GetFileSpec());
if (FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()) ||
module_spec.GetUUID().IsValid()) {
if (resolved_module_spec.GetArchitecture().IsValid() ||
resolved_module_spec.GetUUID().IsValid()) {
error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
module_search_paths_ptr, nullptr,
nullptr);
if (exe_module_sp && exe_module_sp->GetObjectFile())
return error;
exe_module_sp.reset();
}
// No valid architecture was specified or the exact arch wasn't found so
// ask the platform for the architectures that we should be using (in the
// correct order) and see if we can find a match that way
StreamString arch_names;
for (uint32_t idx = 0; GetSupportedArchitectureAtIndex(
idx, resolved_module_spec.GetArchitecture());
++idx) {
error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
module_search_paths_ptr, nullptr,
nullptr);
// Did we find an executable using one of the
if (error.Success()) {
if (exe_module_sp && exe_module_sp->GetObjectFile())
break;
else
error.SetErrorToGenericError();
}
if (idx > 0)
arch_names.PutCString(", ");
arch_names.PutCString(
resolved_module_spec.GetArchitecture().GetArchitectureName());
}
if (error.Fail() || !exe_module_sp) {
if (FileSystem::Instance().Readable(resolved_module_spec.GetFileSpec())) {
error.SetErrorStringWithFormatv(
"'{0}' doesn't contain any '{1}' platform architectures: {2}",
resolved_module_spec.GetFileSpec(), GetPluginName(),
arch_names.GetData());
} else {
error.SetErrorStringWithFormat(
"'%s' is not readable",
resolved_module_spec.GetFileSpec().GetPath().c_str());
}
}
} else {
error.SetErrorStringWithFormat(
"'%s' does not exist",
resolved_module_spec.GetFileSpec().GetPath().c_str());
}
return error;
}
bool PlatformRemoteGDBServer::GetModuleSpec(const FileSpec &module_file_spec,
const ArchSpec &arch,
ModuleSpec &module_spec) {

4
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
View File

@ -40,10 +40,6 @@ public:
llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
// lldb_private::Platform functions
Status
ResolveExecutable(const ModuleSpec &module_spec, lldb::ModuleSP &module_sp,
const FileSpecList *module_search_paths_ptr) override;
bool GetModuleSpec(const FileSpec &module_file_spec, const ArchSpec &arch,
ModuleSpec &module_spec) override;

104
lldb/source/Target/Platform.cpp
View File

@ -833,6 +833,7 @@ Platform::ResolveExecutable(const ModuleSpec &module_spec,
lldb::ModuleSP &exe_module_sp,
const FileSpecList *module_search_paths_ptr) {
Status error;
if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
if (module_spec.GetArchitecture().IsValid()) {
error = ModuleList::GetSharedModule(module_spec, exe_module_sp,
@ -855,12 +856,80 @@ Platform::ResolveExecutable(const ModuleSpec &module_spec,
}
}
} else {
error.SetErrorStringWithFormat("'%s' does not exist",
module_spec.GetFileSpec().GetPath().c_str());
error.SetErrorStringWithFormat(
"'%s' does not exist", module_spec.GetFileSpec().GetPath().c_str());
}
return error;
}
Status
Platform::ResolveRemoteExecutable(const ModuleSpec &module_spec,
lldb::ModuleSP &exe_module_sp,
const FileSpecList *module_search_paths_ptr) {
Status error;
// We may connect to a process and use the provided executable (Don't use
// local $PATH).
ModuleSpec resolved_module_spec(module_spec);
// Resolve any executable within a bundle on MacOSX
Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
if (FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()) ||
module_spec.GetUUID().IsValid()) {
if (resolved_module_spec.GetArchitecture().IsValid() ||
resolved_module_spec.GetUUID().IsValid()) {
error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
module_search_paths_ptr, nullptr,
nullptr);
if (exe_module_sp && exe_module_sp->GetObjectFile())
return error;
exe_module_sp.reset();
}
// No valid architecture was specified or the exact arch wasn't found so
// ask the platform for the architectures that we should be using (in the
// correct order) and see if we can find a match that way
StreamString arch_names;
for (uint32_t idx = 0; GetSupportedArchitectureAtIndex(
idx, resolved_module_spec.GetArchitecture());
++idx) {
error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
module_search_paths_ptr, nullptr,
nullptr);
// Did we find an executable using one of the
if (error.Success()) {
if (exe_module_sp && exe_module_sp->GetObjectFile())
break;
else
error.SetErrorToGenericError();
}
if (idx > 0)
arch_names.PutCString(", ");
arch_names.PutCString(
resolved_module_spec.GetArchitecture().GetArchitectureName());
}
if (error.Fail() || !exe_module_sp) {
if (FileSystem::Instance().Readable(resolved_module_spec.GetFileSpec())) {
error.SetErrorStringWithFormatv(
"'{0}' doesn't contain any '{1}' platform architectures: {2}",
resolved_module_spec.GetFileSpec(), GetPluginName(),
arch_names.GetData());
} else {
error.SetErrorStringWithFormatv("'{0}' is not readable",
resolved_module_spec.GetFileSpec());
}
}
} else {
error.SetErrorStringWithFormatv("'{0}' does not exist",
resolved_module_spec.GetFileSpec());
}
return error;
}
Status Platform::ResolveSymbolFile(Target &target, const ModuleSpec &sym_spec,
FileSpec &sym_file) {
Status error;
@ -1507,12 +1576,13 @@ const std::vector<ConstString> &Platform::GetTrapHandlerSymbolNames() {
return m_trap_handlers;
}
Status Platform::GetCachedExecutable(
ModuleSpec &module_spec, lldb::ModuleSP &module_sp,
const FileSpecList *module_search_paths_ptr, Platform &remote_platform) {
Status
Platform::GetCachedExecutable(ModuleSpec &module_spec,
lldb::ModuleSP &module_sp,
const FileSpecList *module_search_paths_ptr) {
const auto platform_spec = module_spec.GetFileSpec();
const auto error = LoadCachedExecutable(
module_spec, module_sp, module_search_paths_ptr, remote_platform);
const auto error =
LoadCachedExecutable(module_spec, module_sp, module_search_paths_ptr);
if (error.Success()) {
module_spec.GetFileSpec() = module_sp->GetFileSpec();
module_spec.GetPlatformFileSpec() = platform_spec;
@ -1521,15 +1591,17 @@ Status Platform::GetCachedExecutable(
return error;
}
Status Platform::LoadCachedExecutable(
const ModuleSpec &module_spec, lldb::ModuleSP &module_sp,
const FileSpecList *module_search_paths_ptr, Platform &remote_platform) {
return GetRemoteSharedModule(module_spec, nullptr, module_sp,
[&](const ModuleSpec &spec) {
return remote_platform.ResolveExecutable(
spec, module_sp, module_search_paths_ptr);
},
nullptr);
Status
Platform::LoadCachedExecutable(const ModuleSpec &module_spec,
lldb::ModuleSP &module_sp,
const FileSpecList *module_search_paths_ptr) {
return GetRemoteSharedModule(
module_spec, nullptr, module_sp,
[&](const ModuleSpec &spec) {
return ResolveRemoteExecutable(spec, module_sp,
module_search_paths_ptr);
},
nullptr);
}
Status Platform::GetRemoteSharedModule(const ModuleSpec &module_spec,

3
lldb/source/Target/RemoteAwarePlatform.cpp
View File

@ -72,8 +72,7 @@ Status RemoteAwarePlatform::ResolveExecutable(
} else {
if (m_remote_platform_sp) {
return GetCachedExecutable(resolved_module_spec, exe_module_sp,
module_search_paths_ptr,
*m_remote_platform_sp);
module_search_paths_ptr);
}
// We may connect to a process and use the provided executable (Don't use

31
lldb/unittests/Target/RemoteAwarePlatformTest.cpp
View File

@ -31,6 +31,17 @@ public:
ProcessSP(ProcessAttachInfo &, Debugger &, Target *, Status &));
MOCK_METHOD0(CalculateTrapHandlerSymbolNames, void());
MOCK_METHOD2(ResolveRemoteExecutable,
std::pair<Status, ModuleSP>(const ModuleSpec &,
const FileSpecList *));
Status ResolveRemoteExecutable(
const ModuleSpec &module_spec, lldb::ModuleSP &exe_module_sp,
const FileSpecList *module_search_paths_ptr) /*override*/ {
auto pair = ResolveRemoteExecutable(module_spec, module_search_paths_ptr);
exe_module_sp = pair.second;
return pair.first;
}
void SetRemotePlatform(lldb::PlatformSP platform) {
m_remote_platform_sp = platform;
}
@ -47,18 +58,6 @@ public:
ProcessSP(ProcessAttachInfo &, Debugger &, Target *, Status &));
MOCK_METHOD0(CalculateTrapHandlerSymbolNames, void());
MOCK_METHOD0(GetUserIDResolver, UserIDResolver &());
MOCK_METHOD2(ResolveExecutable,
std::pair<Status, ModuleSP>(const ModuleSpec &,
const FileSpecList *));
Status
ResolveExecutable(const ModuleSpec &module_spec,
lldb::ModuleSP &exe_module_sp,
const FileSpecList *module_search_paths_ptr) /*override*/ {
auto pair = ResolveExecutable(module_spec, module_search_paths_ptr);
exe_module_sp = pair.second;
return pair.first;
}
};
namespace {
@ -73,15 +72,13 @@ TEST_F(RemoteAwarePlatformTest, TestResolveExecutabelOnClientByPlatform) {
ModuleSpec executable_spec;
ModuleSP expected_executable(new Module(executable_spec));
auto platform_sp = std::make_shared<TargetPlatformTester>(false);
EXPECT_CALL(*platform_sp, ResolveExecutable(_, _))
.WillRepeatedly(Return(std::make_pair(Status(), expected_executable)));
RemoteAwarePlatformTester platform(false);
EXPECT_CALL(platform, GetSupportedArchitectureAtIndex(_, _))
.WillRepeatedly(Return(false));
EXPECT_CALL(platform, ResolveRemoteExecutable(_, _))
.WillRepeatedly(Return(std::make_pair(Status(), expected_executable)));
platform.SetRemotePlatform(platform_sp);
platform.SetRemotePlatform(std::make_shared<TargetPlatformTester>(false));
ModuleSP resolved_sp;
lldb_private::Status status =