This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
JDevlieghere marked 3 inline comments as done.
Closed by commit rG4add853647b3: [lldb] Improve platform handling in
CreateTargetInternal (authored by JDevlieghere).
Herald added a project: LLDB.
Changed prior to commit:
https://reviews.llvm.org/D84809?vs=281421&id=281659#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84809/new/
https://reviews.llvm.org/D84809
Files:
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Target/TargetList.cpp
lldb/test/API/commands/target/basic/TestTargetCommand.py
lldb/test/API/commands/target/basic/bogus.yaml
Index: lldb/test/API/commands/target/basic/bogus.yaml
===================================================================
--- /dev/null
+++ lldb/test/API/commands/target/basic/bogus.yaml
@@ -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
+ - ''
+ - ''
+ - ''
+...
Index: lldb/test/API/commands/target/basic/TestTargetCommand.py
===================================================================
--- lldb/test/API/commands/target/basic/TestTargetCommand.py
+++ lldb/test/API/commands/target/basic/TestTargetCommand.py
@@ -115,6 +115,33 @@
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),
Index: lldb/source/Target/TargetList.cpp
===================================================================
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -75,55 +75,49 @@
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;
}
+ bool prefer_platform_arch = false;
+
if (!user_exe_path.empty()) {
- ModuleSpecList module_specs;
- ModuleSpec module_spec;
- module_spec.GetFileSpec().SetFile(user_exe_path, FileSpec::Style::native);
+ 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 @@
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 @@
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;
- }
+ // 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;
}
+ }
- // 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);
+ // 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;
}
}
- }
- 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();
+ // 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;
+ if (platform_ptr->GetName() != the_platform_sp->GetName()) {
+ more_than_one_platforms = true;
+ platform_ptr = nullptr;
+ break;
+ }
} 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());
- }
+ 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(
- "), use the --platform option to specify a platform");
- error.SetErrorString(error_strm.GetString());
- return error;
}
+ 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 @@
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 @@
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) {
Index: lldb/source/Commands/CommandObjectTarget.cpp
===================================================================
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -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 @@
"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 @@
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 @@
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 @@
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;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
