[Lldb-commits] [lldb] ed7e468 - [lldb] Improve error message for unrecognized executables (#97490)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Jul 8 09:29:06 PDT 2024
Author: Jonas Devlieghere
Date: 2024-07-08T09:29:01-07:00
New Revision: ed7e46877dc7f09b5d194f87c87bfc2bebfcf27a
URL: https://github.com/llvm/llvm-project/commit/ed7e46877dc7f09b5d194f87c87bfc2bebfcf27a
DIFF: https://github.com/llvm/llvm-project/commit/ed7e46877dc7f09b5d194f87c87bfc2bebfcf27a.diff
LOG: [lldb] Improve error message for unrecognized executables (#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.
Added:
Modified:
lldb/include/lldb/Symbol/ObjectFile.h
lldb/source/Symbol/ObjectFile.cpp
lldb/source/Target/Platform.cpp
lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml
Removed:
################################################################################
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 c06abd3070f31..bb90c377d86b2 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -732,7 +732,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).
@@ -741,55 +740,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