[Lldb-commits] [lldb] [lldb] Improve error message for unrecognized executables (PR #97490)
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Tue Jul 2 16:28:45 PDT 2024
https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/97490
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
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.
>From a4e291994cc1f2357829292d043fa05edf9f04af Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Tue, 2 Jul 2024 16:22:34 -0700
Subject: [PATCH] [lldb] Improve error message for unrecognized executables
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.
---
lldb/include/lldb/Symbol/ObjectFile.h | 1 +
lldb/source/Symbol/ObjectFile.cpp | 9 ++
lldb/source/Target/Platform.cpp | 87 ++++++++++---------
.../PECOFF/invalid-export-table.yaml | 2 +-
4 files changed, 55 insertions(+), 44 deletions(-)
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
More information about the lldb-commits
mailing list