[lldb] Improve error message for unrecognized executables #97490
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently, LLDB prints out a rather unhelpful error message when passed a file that it doesn't recognize as an executable. error: '/path/to/file' doesn't contain any 'host' platform architectures: arm64, armv7, armv7f, armv7k, armv7s, armv7m, armv7em, armv6m, armv6, armv5, armv4, arm, thumbv7, thumbv7k, thumbv7s, thumbv7f, thumbv7m, thumbv7em, thumbv6m, thumbv6, thumbv5, thumbv4t, thumb, x86_64, x86_64, arm64, arm64e, arm64, arm64e I did a quick search internally and found at least 24 instances of users being confused by this. This patch improves the error message when it doesn't recognize the file as an executable, but keeps the existing error message otherwise, i.e. when it's an object file we understand, but the current platform doesn't support.
|
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesCurrently, LLDB prints out a rather unhelpful error message when passed a file that it doesn't recognize as an executable. > error: '/path/to/file' doesn't contain any 'host' platform I did a quick search internally and found at least 24 instances of users being confused by this. This patch improves the error message when it doesn't recognize the file as an executable, but keeps the existing error message otherwise, i.e. when it's an object file we understand, but the current platform doesn't support. Full diff: https://github.com/llvm/llvm-project/pull/97490.diff 4 Files Affected:
diff --git a/lldb/include/lldb/Symbol/ObjectFile.h b/lldb/include/lldb/Symbol/ObjectFile.h
index 6348d8103f85d..8592323322e38 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -178,6 +178,7 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
lldb::offset_t file_offset,
lldb::offset_t file_size,
lldb_private::ModuleSpecList &specs);
+ static bool IsObjectFile(lldb_private::FileSpec file_spec);
/// Split a path into a file path with object name.
///
/// For paths like "/tmp/foo.a(bar.o)" we often need to split a path up into
diff --git a/lldb/source/Symbol/ObjectFile.cpp b/lldb/source/Symbol/ObjectFile.cpp
index d890ad92e8312..2608a9c5fb79a 100644
--- a/lldb/source/Symbol/ObjectFile.cpp
+++ b/lldb/source/Symbol/ObjectFile.cpp
@@ -184,6 +184,15 @@ ObjectFileSP ObjectFile::FindPlugin(const lldb::ModuleSP &module_sp,
return object_file_sp;
}
+bool ObjectFile::IsObjectFile(lldb_private::FileSpec file_spec) {
+ DataBufferSP data_sp;
+ offset_t data_offset = 0;
+ ModuleSP module_sp = std::make_shared<Module>(file_spec);
+ return static_cast<bool>(ObjectFile::FindPlugin(
+ module_sp, &file_spec, 0, FileSystem::Instance().GetByteSize(file_spec),
+ data_sp, data_offset));
+}
+
size_t ObjectFile::GetModuleSpecifications(const FileSpec &file,
lldb::offset_t file_offset,
lldb::offset_t file_size,
diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index b3116545b91d1..d57f644be5611 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -766,7 +766,6 @@ Status
Platform::ResolveExecutable(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).
@@ -775,55 +774,57 @@ Platform::ResolveExecutable(const ModuleSpec &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 (!FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()) &&
+ !module_spec.GetUUID().IsValid())
+ return Status::createWithFormat("'{0}' does not exist",
+ resolved_module_spec.GetFileSpec());
+
+ if (resolved_module_spec.GetArchitecture().IsValid() ||
+ resolved_module_spec.GetUUID().IsValid()) {
+ Status 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.
+ // 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;
+ llvm::ListSeparator LS;
+ ArchSpec process_host_arch;
+ Status error;
+ for (const ArchSpec &arch : GetSupportedArchitectures(process_host_arch)) {
+ resolved_module_spec.GetArchitecture() = arch;
+ error =
+ ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
+ module_search_paths_ptr, nullptr, nullptr);
+ if (error.Success()) {
if (exe_module_sp && exe_module_sp->GetObjectFile())
- return error;
- exe_module_sp.reset();
+ break;
+ error.SetErrorToGenericError();
}
- // No valid architecture was specified or the exact arch wasn't found.
- // 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;
- llvm::ListSeparator LS;
- ArchSpec process_host_arch;
- for (const ArchSpec &arch : GetSupportedArchitectures(process_host_arch)) {
- resolved_module_spec.GetArchitecture() = arch;
- error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
- module_search_paths_ptr, nullptr,
- nullptr);
- if (error.Success()) {
- if (exe_module_sp && exe_module_sp->GetObjectFile())
- break;
- error.SetErrorToGenericError();
- }
- arch_names << LS << arch.GetArchitectureName();
- }
+ arch_names << LS << arch.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",
+ if (exe_module_sp && error.Success())
+ return {};
+
+ if (!FileSystem::Instance().Readable(resolved_module_spec.GetFileSpec()))
+ return Status::createWithFormat("'{0}' is not readable",
resolved_module_spec.GetFileSpec());
- }
- return error;
+ if (!ObjectFile::IsObjectFile(resolved_module_spec.GetFileSpec()))
+ return Status::createWithFormat("'{0}' is not a valid executable",
+ resolved_module_spec.GetFileSpec());
+
+ return Status::createWithFormat(
+ "'{0}' doesn't contain any '{1}' platform architectures: {2}",
+ resolved_module_spec.GetFileSpec(), GetPluginName(),
+ arch_names.GetData());
}
Status Platform::ResolveSymbolFile(Target &target, const ModuleSpec &sym_spec,
diff --git a/lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml b/lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml
index 389261ad9b10a..e6564661b96fd 100644
--- a/lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml
+++ b/lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml
@@ -4,7 +4,7 @@
# RUN: yaml2obj %s -o %t.exe
# RUN: %lldb %t.exe 2>&1 | FileCheck %s
-# CHECK: error: '{{.*}}' doesn't contain any {{.*}} platform architectures
+# CHECK: error: '{{.*}}' is not a valid executable
--- !COFF
OptionalHeader:
AddressOfEntryPoint: 4096
|
jasonmolenda
approved these changes
Jul 3, 2024
labath
approved these changes
Jul 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, LLDB prints out a rather unhelpful error message when passed a file that it doesn't recognize as an executable.
I did a quick search internally and found at least 24 instances of users being confused by this. This patch improves the error message when it doesn't recognize the file as an executable, but keeps the existing error message otherwise, i.e. when it's an object file we understand, but the current platform doesn't support.