[lldb] Improve platform handling in CreateTargetInternal

Currently, `target create` has no --platform option. However,
TargetList::CreateTargetInternal which is called under the hood, will
return an error when either no platform or multiple matching platforms
are found, saying that a platform should be specified with --platform.

This patch adds the platform option, but that doesn't solve either of
these errors.

 - If more than one platform matches, specifying the platform isn't
   going to fix that. The current code will only look at the
   architecture instead. I've updated the error message to ask the user
   to specify an architecture.

 - If no architecture is found, specifying a new one via platform isn't
   going to change that either because we already try to find one that
   matches the given architecture.

Differential revision: https://reviews.llvm.org/D84809
This commit is contained in:
Jonas Devlieghere 2020-07-29 08:59:57 -07:00
parent e7196bdf81
commit 4add853647
4 changed files with 338 additions and 125 deletions

View File

@ -23,6 +23,7 @@
#include "lldb/Interpreter/OptionGroupBoolean.h"
#include "lldb/Interpreter/OptionGroupFile.h"
#include "lldb/Interpreter/OptionGroupFormat.h"
#include "lldb/Interpreter/OptionGroupPlatform.h"
#include "lldb/Interpreter/OptionGroupString.h"
#include "lldb/Interpreter/OptionGroupUInt64.h"
#include "lldb/Interpreter/OptionGroupUUID.h"
@ -214,6 +215,7 @@ public:
"Create a target using the argument as the main executable.",
nullptr),
m_option_group(), m_arch_option(),
m_platform_options(true), // Include the --platform option.
m_core_file(LLDB_OPT_SET_1, false, "core", 'c', 0, eArgTypeFilename,
"Fullpath to a core file to use for this target."),
m_symbol_file(LLDB_OPT_SET_1, false, "symfile", 's', 0,
@ -240,6 +242,7 @@ public:
m_arguments.push_back(arg);
m_option_group.Append(&m_arch_option, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
m_option_group.Append(&m_platform_options, LLDB_OPT_SET_ALL, 1);
m_option_group.Append(&m_core_file, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
m_option_group.Append(&m_symbol_file, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
m_option_group.Append(&m_remote_file, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
@ -311,7 +314,8 @@ protected:
llvm::StringRef arch_cstr = m_arch_option.GetArchitectureName();
Status error(debugger.GetTargetList().CreateTarget(
debugger, file_path, arch_cstr,
m_add_dependents.m_load_dependent_files, nullptr, target_sp));
m_add_dependents.m_load_dependent_files, &m_platform_options,
target_sp));
if (target_sp) {
// Only get the platform after we create the target because we might
@ -442,6 +446,7 @@ protected:
private:
OptionGroupOptions m_option_group;
OptionGroupArchitecture m_arch_option;
OptionGroupPlatform m_platform_options;
OptionGroupFile m_core_file;
OptionGroupFile m_symbol_file;
OptionGroupFile m_remote_file;

View File

@ -75,55 +75,49 @@ Status TargetList::CreateTargetInternal(
const OptionGroupPlatform *platform_options, TargetSP &target_sp,
bool is_dummy_target) {
Status error;
PlatformSP platform_sp;
// This is purposely left empty unless it is specified by triple_cstr. If not
// initialized via triple_cstr, then the currently selected platform will set
// the architecture correctly.
// Let's start by looking at the selected platform.
PlatformSP platform_sp = debugger.GetPlatformList().GetSelectedPlatform();
// This variable corresponds to the architecture specified by the triple
// string. If that string was empty the currently selected platform will
// determine the architecture.
const ArchSpec arch(triple_str);
if (!triple_str.empty()) {
if (!arch.IsValid()) {
error.SetErrorStringWithFormat("invalid triple '%s'",
triple_str.str().c_str());
return error;
}
if (!triple_str.empty() && !arch.IsValid()) {
error.SetErrorStringWithFormat("invalid triple '%s'",
triple_str.str().c_str());
return error;
}
ArchSpec platform_arch(arch);
bool prefer_platform_arch = false;
CommandInterpreter &interpreter = debugger.GetCommandInterpreter();
// let's see if there is already an existing platform before we go creating
// another...
platform_sp = debugger.GetPlatformList().GetSelectedPlatform();
if (platform_options && platform_options->PlatformWasSpecified()) {
// Create a new platform if it doesn't match the selected platform
if (!platform_options->PlatformMatches(platform_sp)) {
const bool select_platform = true;
platform_sp = platform_options->CreatePlatformWithOptions(
interpreter, arch, select_platform, error, platform_arch);
if (!platform_sp)
return error;
}
// Create a new platform if a platform was specified in the platform options
// and doesn't match the selected platform.
if (platform_options && platform_options->PlatformWasSpecified() &&
!platform_options->PlatformMatches(platform_sp)) {
const bool select_platform = true;
platform_sp = platform_options->CreatePlatformWithOptions(
debugger.GetCommandInterpreter(), arch, select_platform, error,
platform_arch);
if (!platform_sp)
return error;
}
if (!user_exe_path.empty()) {
ModuleSpecList module_specs;
ModuleSpec module_spec;
module_spec.GetFileSpec().SetFile(user_exe_path, FileSpec::Style::native);
FileSystem::Instance().Resolve(module_spec.GetFileSpec());
bool prefer_platform_arch = false;
if (!user_exe_path.empty()) {
ModuleSpec module_spec(FileSpec(user_exe_path, FileSpec::Style::native));
FileSystem::Instance().Resolve(module_spec.GetFileSpec());
// Resolve the executable in case we are given a path to a application
// bundle like a .app bundle on MacOSX
// bundle like a .app bundle on MacOSX.
Host::ResolveExecutableInBundle(module_spec.GetFileSpec());
lldb::offset_t file_offset = 0;
lldb::offset_t file_size = 0;
ModuleSpecList module_specs;
const size_t num_specs = ObjectFile::GetModuleSpecifications(
module_spec.GetFileSpec(), file_offset, file_size, module_specs);
if (num_specs > 0) {
ModuleSpec matching_module_spec;
@ -134,7 +128,7 @@ Status TargetList::CreateTargetInternal(
matching_module_spec.GetArchitecture())) {
// If the OS or vendor weren't specified, then adopt the module's
// architecture so that the platform matching can be more
// accurate
// accurate.
if (!platform_arch.TripleOSWasSpecified() ||
!platform_arch.TripleVendorWasSpecified()) {
prefer_platform_arch = true;
@ -155,113 +149,107 @@ Status TargetList::CreateTargetInternal(
return error;
}
} else {
// Only one arch and none was specified
// Only one arch and none was specified.
prefer_platform_arch = true;
platform_arch = matching_module_spec.GetArchitecture();
}
}
} else if (arch.IsValid()) {
// A (valid) architecture was specified.
module_spec.GetArchitecture() = arch;
if (module_specs.FindMatchingModuleSpec(module_spec,
matching_module_spec)) {
prefer_platform_arch = true;
platform_arch = matching_module_spec.GetArchitecture();
}
} else {
if (arch.IsValid()) {
module_spec.GetArchitecture() = arch;
if (module_specs.FindMatchingModuleSpec(module_spec,
matching_module_spec)) {
prefer_platform_arch = true;
platform_arch = matching_module_spec.GetArchitecture();
}
} else {
// No architecture specified, check if there is only one platform for
// all of the architectures.
typedef std::vector<PlatformSP> PlatformList;
PlatformList platforms;
PlatformSP host_platform_sp = Platform::GetHostPlatform();
for (size_t i = 0; i < num_specs; ++i) {
ModuleSpec module_spec;
if (module_specs.GetModuleSpecAtIndex(i, module_spec)) {
// See if there was a selected platform and check that first
// since the user may have specified it.
if (platform_sp) {
if (platform_sp->IsCompatibleArchitecture(
module_spec.GetArchitecture(), false, nullptr)) {
platforms.push_back(platform_sp);
continue;
}
}
// Next check the host platform it if wasn't already checked
// above
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;
}
}
// Just find a platform that matches the architecture in the
// executable file
PlatformSP fallback_platform_sp(
Platform::GetPlatformForArchitecture(
module_spec.GetArchitecture(), nullptr));
if (fallback_platform_sp) {
platforms.push_back(fallback_platform_sp);
// 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;
}
}
}
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;
// 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;
}
} else {
platform_ptr = the_platform_sp.get();
}
// Finally find a platform that matches the architecture in the
// executable file.
PlatformSP fallback_platform_sp(
Platform::GetPlatformForArchitecture(
module_spec.GetArchitecture(), nullptr));
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) {
// 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
error.SetErrorString("No matching platforms found for this file, "
"specify one with the --platform option");
return error;
} else {
// More than one platform claims to support this file, so the
// --platform option must be specified
StreamString error_strm;
std::set<Platform *> 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().GetCString());
platform_set.insert(the_platform_sp.get());
}
if (platform_ptr->GetName() != the_platform_sp->GetName()) {
more_than_one_platforms = true;
platform_ptr = nullptr;
break;
}
error_strm.Printf(
"), use the --platform option to specify a platform");
error.SetErrorString(error_strm.GetString());
return error;
} 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.
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;
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().GetCString());
platform_set.insert(the_platform_sp.get());
}
}
error_strm.Printf("), specify an architecture to disambiguate");
error.SetErrorString(error_strm.GetString());
return error;
}
}
}
}
// If we have a valid architecture, make sure the current platform is
// compatible with that architecture
// compatible with that architecture.
if (!prefer_platform_arch && arch.IsValid()) {
if (!platform_sp->IsCompatibleArchitecture(arch, false, &platform_arch)) {
platform_sp = Platform::GetPlatformForArchitecture(arch, &platform_arch);
@ -269,8 +257,8 @@ Status TargetList::CreateTargetInternal(
debugger.GetPlatformList().SetSelectedPlatform(platform_sp);
}
} else if (platform_arch.IsValid()) {
// if "arch" isn't valid, yet "platform_arch" is, it means we have an
// executable file with a single architecture which should be used
// If "arch" isn't valid, yet "platform_arch" is, it means we have an
// executable file with a single architecture which should be used.
ArchSpec fixed_platform_arch;
if (!platform_sp->IsCompatibleArchitecture(platform_arch, false,
&fixed_platform_arch)) {
@ -284,10 +272,9 @@ Status TargetList::CreateTargetInternal(
if (!platform_arch.IsValid())
platform_arch = arch;
error = TargetList::CreateTargetInternal(
return TargetList::CreateTargetInternal(
debugger, user_exe_path, platform_arch, load_dependent_files, platform_sp,
target_sp, is_dummy_target);
return error;
}
lldb::TargetSP TargetList::GetDummyTarget(lldb_private::Debugger &debugger) {

View File

@ -115,6 +115,33 @@ class targetCommandTestCase(TestBase):
self.runCmd("target list")
@no_debug_info_test
def test_target_create_invalid_arch(self):
exe = self.getBuildArtifact("a.out")
self.expect("target create {} --arch doesntexist".format(exe), error=True,
patterns=["error: invalid triple 'doesntexist'"])
@no_debug_info_test
def test_target_create_platform(self):
self.buildB()
exe = self.getBuildArtifact("b.out")
self.expect("target create {} --platform host".format(exe))
@no_debug_info_test
def test_target_create_unsupported_platform(self):
yaml = os.path.join(self.getSourceDir(), "bogus.yaml")
exe = self.getBuildArtifact("bogus")
self.yaml2obj(yaml, exe)
self.expect("target create {}".format(exe), error=True,
patterns=['error: no matching platforms found for this file'])
@no_debug_info_test
def test_target_create_invalid_platform(self):
self.buildB()
exe = self.getBuildArtifact("b.out")
self.expect("target create {} --platform doesntexist".format(exe), error=True,
patterns=['error: unable to find a plug-in for the platform named "doesntexist"'])
def do_target_variable_command(self, exe_name):
"""Exercise 'target variable' command before and after starting the inferior."""
self.runCmd("file " + self.getBuildArtifact(exe_name),

View File

@ -0,0 +1,194 @@
--- !fat-mach-o
FatHeader:
magic: 0xCAFEBABE
nfat_arch: 3
FatArchs:
- cputype: 0x0000000C
cpusubtype: 0x00000009
offset: 0x0000000000004000
size: 200
align: 14
- cputype: 0x0000000C
cpusubtype: 0x0000000B
offset: 0x0000000000008000
size: 200
align: 14
- cputype: 0x0100000C
cpusubtype: 0x00000000
offset: 0x000000000000C000
size: 332
align: 14
Slices:
- !mach-o
FileHeader:
magic: 0xFEEDFACE
# Bogus
cputype: 0x00000003
cpusubtype: 0x00000009
filetype: 0x00000001
ncmds: 4
sizeofcmds: 112
flags: 0x00002000
LoadCommands:
- cmd: LC_SEGMENT
cmdsize: 56
segname: ''
vmaddr: 0
vmsize: 0
fileoff: 0
filesize: 0
maxprot: 7
initprot: 7
nsects: 0
flags: 0
- cmd: LC_SYMTAB
cmdsize: 24
symoff: 172
nsyms: 1
stroff: 184
strsize: 16
- cmd: LC_VERSION_MIN_IPHONEOS
cmdsize: 16
version: 327680
sdk: 0
- cmd: LC_DATA_IN_CODE
cmdsize: 16
dataoff: 172
datasize: 0
LinkEditData:
NameList:
- n_strx: 4
n_type: 0x01
n_sect: 0
n_desc: 512
n_value: 4
StringTable:
- ''
- ''
- ''
- ''
- _armv7_var
- ''
- !mach-o
FileHeader:
magic: 0xFEEDFACE
# Bogus
cputype: 0x00000002
cpusubtype: 0x0000000B
filetype: 0x00000001
ncmds: 4
sizeofcmds: 112
flags: 0x00002000
LoadCommands:
- cmd: LC_SEGMENT
cmdsize: 56
segname: ''
vmaddr: 0
vmsize: 0
fileoff: 0
filesize: 0
maxprot: 7
initprot: 7
nsects: 0
flags: 0
- cmd: LC_SYMTAB
cmdsize: 24
symoff: 172
nsyms: 1
stroff: 184
strsize: 16
- cmd: LC_VERSION_MIN_IPHONEOS
cmdsize: 16
version: 327680
sdk: 0
- cmd: LC_DATA_IN_CODE
cmdsize: 16
dataoff: 172
datasize: 0
LinkEditData:
NameList:
- n_strx: 4
n_type: 0x01
n_sect: 0
n_desc: 512
n_value: 4
StringTable:
- ''
- ''
- ''
- ''
- _armv7s_var
- !mach-o
FileHeader:
magic: 0xFEEDFACF
# Bogus
cputype: 0x00000001
cpusubtype: 0x00000000
filetype: 0x00000001
ncmds: 4
sizeofcmds: 208
flags: 0x00002000
reserved: 0x00000000
LoadCommands:
- cmd: LC_SEGMENT_64
cmdsize: 152
segname: ''
vmaddr: 0
vmsize: 0
fileoff: 272
filesize: 0
maxprot: 7
initprot: 7
nsects: 1
flags: 0
Sections:
- sectname: __text
segname: __TEXT
addr: 0x0000000000000000
size: 0
offset: 0x00000110
align: 0
reloff: 0x00000000
nreloc: 0
flags: 0x80000400
reserved1: 0x00000000
reserved2: 0x00000000
reserved3: 0x00000000
content: ''
- cmd: LC_SYMTAB
cmdsize: 24
symoff: 276
nsyms: 2
stroff: 308
strsize: 24
- cmd: LC_VERSION_MIN_IPHONEOS
cmdsize: 16
version: 327680
sdk: 0
- cmd: LC_DATA_IN_CODE
cmdsize: 16
dataoff: 276
datasize: 0
LinkEditData:
NameList:
- n_strx: 15
n_type: 0x0E
n_sect: 1
n_desc: 0
n_value: 0
- n_strx: 4
n_type: 0x01
n_sect: 0
n_desc: 512
n_value: 4
StringTable:
- ''
- ''
- ''
- ''
- _arm64_var
- ltmp0
- ''
- ''
- ''
...