[Lldb-commits] [lldb] [lldb] Make ObjectFile::GetModuleSpecifications *return* module specifications (PR #188509)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Mar 25 08:24:37 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
<details>
<summary>Changes</summary>
For consistency with #<!-- -->188276 (and better readability?).
---
Patch is 20.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/188509.diff
11 Files Affected:
- (modified) lldb/include/lldb/Symbol/ObjectFile.h (+7-8)
- (modified) lldb/source/API/SBModuleSpec.cpp (+1-1)
- (modified) lldb/source/Commands/CommandObjectTarget.cpp (+4-3)
- (modified) lldb/source/Core/Module.cpp (+3-4)
- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp (+3-3)
- (modified) lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp (+22-28)
- (modified) lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp (+10-10)
- (modified) lldb/source/Symbol/ObjectFile.cpp (+16-25)
- (modified) lldb/source/Target/Platform.cpp (+2-4)
- (modified) lldb/source/Target/TargetList.cpp (+4-5)
- (modified) lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp (+9-7)
``````````diff
diff --git a/lldb/include/lldb/Symbol/ObjectFile.h b/lldb/include/lldb/Symbol/ObjectFile.h
index 2cdcadd262622..dfcf33f385d93 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -174,17 +174,16 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
lldb::addr_t header_addr,
lldb::WritableDataBufferSP file_data_sp);
- static size_t
+ static ModuleSpecList
GetModuleSpecifications(const FileSpec &file, lldb::offset_t file_offset,
- lldb::offset_t file_size, ModuleSpecList &specs,
+ lldb::offset_t file_size,
lldb::DataExtractorSP = lldb::DataExtractorSP());
- static size_t GetModuleSpecifications(const lldb_private::FileSpec &file,
- lldb::DataExtractorSP &extractor_sp,
- lldb::offset_t data_offset,
- lldb::offset_t file_offset,
- lldb::offset_t file_size,
- lldb_private::ModuleSpecList &specs);
+ static ModuleSpecList
+ GetModuleSpecifications(const lldb_private::FileSpec &file,
+ lldb::DataExtractorSP &extractor_sp,
+ lldb::offset_t data_offset,
+ lldb::offset_t file_offset, lldb::offset_t file_size);
static bool IsObjectFile(lldb_private::FileSpec file_spec);
/// Split a path into a file path with object name.
///
diff --git a/lldb/source/API/SBModuleSpec.cpp b/lldb/source/API/SBModuleSpec.cpp
index 8371106dcbf9e..7b59538f70161 100644
--- a/lldb/source/API/SBModuleSpec.cpp
+++ b/lldb/source/API/SBModuleSpec.cpp
@@ -213,7 +213,7 @@ SBModuleSpecList SBModuleSpecList::GetModuleSpecifications(const char *path) {
FileSpec file_spec(path);
FileSystem::Instance().Resolve(file_spec);
Host::ResolveExecutableInBundle(file_spec);
- ObjectFile::GetModuleSpecifications(file_spec, 0, 0, *specs.m_opaque_up);
+ *specs.m_opaque_up = ObjectFile::GetModuleSpecifications(file_spec, 0, 0);
return specs;
}
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index 64b9500b23634..ea68d5b600a08 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -4303,9 +4303,10 @@ class CommandObjectTargetSymbolsAdd : public CommandObjectParsed {
ModuleList matching_modules;
// First extract all module specs from the symbol file
- lldb_private::ModuleSpecList symfile_module_specs;
- if (ObjectFile::GetModuleSpecifications(module_spec.GetSymbolFileSpec(),
- 0, 0, symfile_module_specs)) {
+ lldb_private::ModuleSpecList symfile_module_specs =
+ ObjectFile::GetModuleSpecifications(module_spec.GetSymbolFileSpec(), 0,
+ 0);
+ if (symfile_module_specs.GetSize() > 0) {
// Now extract the module spec that matches the target architecture
ModuleSpec target_arch_module_spec;
ModuleSpec symfile_module_spec;
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index e34e639392f1a..d82f97a328a60 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -157,10 +157,9 @@ Module::Module(const ModuleSpec &module_spec)
// First extract all module specifications from the file using the local file
// path. If there are no specifications, then don't fill anything in
- ModuleSpecList modules_specs;
- if (ObjectFile::GetModuleSpecifications(module_spec.GetFileSpec(), 0,
- file_size, modules_specs,
- extractor_sp) == 0)
+ ModuleSpecList modules_specs = ObjectFile::GetModuleSpecifications(
+ module_spec.GetFileSpec(), 0, file_size, extractor_sp);
+ if (modules_specs.GetSize() == 0)
return;
// Now make sure that one of the module specifications matches what we just
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index 1beec8bf87155..16ded2c657d54 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -1353,9 +1353,9 @@ GDBRemoteCommunicationServerCommon::GetModuleInfo(llvm::StringRef module_path,
const ModuleSpec module_spec(actual_module_path_spec, arch);
- ModuleSpecList module_specs;
- if (!ObjectFile::GetModuleSpecifications(actual_module_path_spec, file_offset,
- file_size, module_specs))
+ ModuleSpecList module_specs = ObjectFile::GetModuleSpecifications(
+ actual_module_path_spec, file_offset, file_size);
+ if (module_specs.GetSize() == 0)
return ModuleSpec();
ModuleSpec matched_module_spec;
diff --git a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
index e209ddeb1cc2d..3947036538f20 100644
--- a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
+++ b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
@@ -258,7 +258,6 @@ std::optional<ModuleSpec> SymbolLocatorDebugSymbols::LocateExecutableObjectFile(
path);
FileSpec file_spec(path);
FileSystem::Instance().Resolve(file_spec);
- ModuleSpecList module_specs;
ModuleSpec matched_module_spec;
using namespace llvm::sys::fs;
switch (get_file_type(file_spec.GetPath())) {
@@ -274,12 +273,11 @@ std::optional<ModuleSpec> SymbolLocatorDebugSymbols::LocateExecutableObjectFile(
sizeof(path) - 1)) {
FileSpec bundle_exe_file_spec(path);
FileSystem::Instance().Resolve(bundle_exe_file_spec);
- if (ObjectFile::GetModuleSpecifications(
- bundle_exe_file_spec, 0, 0, module_specs) &&
+ if (ModuleSpecList module_specs =
+ ObjectFile::GetModuleSpecifications(
+ bundle_exe_file_spec, 0, 0);
module_specs.FindMatchingModuleSpec(
- module_spec, matched_module_spec))
-
- {
+ module_spec, matched_module_spec)) {
++items_found;
return_module_spec.GetFileSpec() = bundle_exe_file_spec;
LLDB_LOGF(log,
@@ -302,12 +300,10 @@ std::optional<ModuleSpec> SymbolLocatorDebugSymbols::LocateExecutableObjectFile(
case file_type::symlink_file:
case file_type::block_file:
case file_type::character_file:
- if (ObjectFile::GetModuleSpecifications(file_spec, 0, 0,
- module_specs) &&
- module_specs.FindMatchingModuleSpec(module_spec,
- matched_module_spec))
-
- {
+ if (ModuleSpecList module_specs =
+ ObjectFile::GetModuleSpecifications(file_spec, 0, 0);
+ module_specs.FindMatchingModuleSpec(
+ module_spec, matched_module_spec)) {
++items_found;
return_module_spec.GetFileSpec() = file_spec;
LLDB_LOGF(log,
@@ -348,8 +344,9 @@ std::optional<FileSpec> SymbolLocatorDebugSymbols::FindSymbolFileInBundle(
continue;
FileSpec dsym_fspec(Iter->path());
- ModuleSpecList module_specs;
- if (ObjectFile::GetModuleSpecifications(dsym_fspec, 0, 0, module_specs)) {
+ ModuleSpecList module_specs =
+ ObjectFile::GetModuleSpecifications(dsym_fspec, 0, 0);
+ if (module_specs.GetSize() > 0) {
ModuleSpec spec;
for (size_t i = 0; i < module_specs.GetSize(); ++i) {
bool got_spec = module_specs.GetModuleSpecAtIndex(i, spec);
@@ -372,8 +369,9 @@ std::optional<FileSpec> SymbolLocatorDebugSymbols::FindSymbolFileInBundle(
static bool FileAtPathContainsArchAndUUID(const FileSpec &file_fspec,
const ArchSpec *arch,
const lldb_private::UUID *uuid) {
- ModuleSpecList module_specs;
- if (ObjectFile::GetModuleSpecifications(file_fspec, 0, 0, module_specs)) {
+ ModuleSpecList module_specs =
+ ObjectFile::GetModuleSpecifications(file_fspec, 0, 0);
+ if (module_specs.GetSize() > 0) {
ModuleSpec spec;
for (size_t i = 0; i < module_specs.GetSize(); ++i) {
bool got_spec = module_specs.GetModuleSpecAtIndex(i, spec);
@@ -700,7 +698,6 @@ static int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
path);
FileSpec file_spec(path);
FileSystem::Instance().Resolve(file_spec);
- ModuleSpecList module_specs;
ModuleSpec matched_module_spec;
using namespace llvm::sys::fs;
switch (get_file_type(file_spec.GetPath())) {
@@ -716,12 +713,11 @@ static int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
sizeof(path) - 1)) {
FileSpec bundle_exe_file_spec(path);
FileSystem::Instance().Resolve(bundle_exe_file_spec);
- if (ObjectFile::GetModuleSpecifications(
- bundle_exe_file_spec, 0, 0, module_specs) &&
+ if (ModuleSpecList module_specs =
+ ObjectFile::GetModuleSpecifications(
+ bundle_exe_file_spec, 0, 0);
module_specs.FindMatchingModuleSpec(
- module_spec, matched_module_spec))
-
- {
+ module_spec, matched_module_spec)) {
++items_found;
return_module_spec.GetFileSpec() = bundle_exe_file_spec;
LLDB_LOGF(log,
@@ -744,12 +740,10 @@ static int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
case file_type::symlink_file:
case file_type::block_file:
case file_type::character_file:
- if (ObjectFile::GetModuleSpecifications(file_spec, 0, 0,
- module_specs) &&
- module_specs.FindMatchingModuleSpec(module_spec,
- matched_module_spec))
-
- {
+ if (ModuleSpecList module_specs =
+ ObjectFile::GetModuleSpecifications(file_spec, 0, 0);
+ module_specs.FindMatchingModuleSpec(
+ module_spec, matched_module_spec)) {
++items_found;
return_module_spec.GetFileSpec() = file_spec;
LLDB_LOGF(log,
diff --git a/lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp b/lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp
index 919f26ba70123..096510215cfa2 100644
--- a/lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp
@@ -80,11 +80,12 @@ std::optional<ModuleSpec> SymbolLocatorDefault::LocateExecutableObjectFile(
exec_fspec ? exec_fspec.GetFilename().AsCString("<NULL>") : "<NULL>",
arch ? arch->GetArchitectureName() : "<NULL>", (const void *)uuid);
- ModuleSpecList module_specs;
ModuleSpec matched_module_spec;
- if (exec_fspec &&
- ObjectFile::GetModuleSpecifications(exec_fspec, 0, 0, module_specs) &&
- module_specs.FindMatchingModuleSpec(module_spec, matched_module_spec)) {
+ if (!exec_fspec)
+ return {};
+ ModuleSpecList module_specs =
+ ObjectFile::GetModuleSpecifications(exec_fspec, 0, 0);
+ if (module_specs.FindMatchingModuleSpec(module_spec, matched_module_spec)) {
ModuleSpec result;
result.GetFileSpec() = exec_fspec;
return result;
@@ -215,12 +216,11 @@ std::optional<FileSpec> SymbolLocatorDefault::LocateExecutableSymbolFile(
continue;
if (FileSystem::Instance().Exists(file_spec)) {
- lldb_private::ModuleSpecList specs;
- const size_t num_specs =
- ObjectFile::GetModuleSpecifications(file_spec, 0, 0, specs);
+ lldb_private::ModuleSpecList specs =
+ ObjectFile::GetModuleSpecifications(file_spec, 0, 0);
ModuleSpec mspec;
bool valid_mspec = false;
- if (num_specs == 2) {
+ if (specs.GetSize() == 2) {
// Special case to handle both i386 and i686 from ObjectFilePECOFF
ModuleSpec mspec2;
if (specs.GetModuleSpecAtIndex(0, mspec) &&
@@ -231,9 +231,9 @@ std::optional<FileSpec> SymbolLocatorDefault::LocateExecutableSymbolFile(
}
}
if (!valid_mspec) {
- assert(num_specs <= 1 &&
+ assert(specs.GetSize() <= 1 &&
"Symbol Vendor supports only a single architecture");
- if (num_specs == 1) {
+ if (specs.GetSize() == 1) {
if (specs.GetModuleSpecAtIndex(0, mspec)) {
valid_mspec = true;
}
diff --git a/lldb/source/Symbol/ObjectFile.cpp b/lldb/source/Symbol/ObjectFile.cpp
index 0bb107fa5f85a..27143fd0ed3fb 100644
--- a/lldb/source/Symbol/ObjectFile.cpp
+++ b/lldb/source/Symbol/ObjectFile.cpp
@@ -194,11 +194,9 @@ bool ObjectFile::IsObjectFile(lldb_private::FileSpec file_spec) {
extractor_sp, data_offset));
}
-size_t ObjectFile::GetModuleSpecifications(const FileSpec &file,
- lldb::offset_t file_offset,
- lldb::offset_t file_size,
- ModuleSpecList &specs,
- DataExtractorSP extractor_sp) {
+ModuleSpecList ObjectFile::GetModuleSpecifications(
+ const FileSpec &file, lldb::offset_t file_offset, lldb::offset_t file_size,
+ DataExtractorSP extractor_sp) {
if (!extractor_sp)
extractor_sp = std::make_shared<DataExtractor>();
if (!extractor_sp->HasData()) {
@@ -213,40 +211,33 @@ size_t ObjectFile::GetModuleSpecifications(const FileSpec &file,
if (actual_file_size > file_offset)
file_size = actual_file_size - file_offset;
}
- return ObjectFile::GetModuleSpecifications(file, // file spec
- extractor_sp, // data bytes
- 0, // data offset
- file_offset, // file offset
- file_size, // file length
- specs);
+ return ObjectFile::GetModuleSpecifications(file, extractor_sp,
+ /*data_offset=*/0, file_offset,
+ file_size);
}
- return 0;
+ return {};
}
-size_t ObjectFile::GetModuleSpecifications(
+ModuleSpecList ObjectFile::GetModuleSpecifications(
const lldb_private::FileSpec &file, lldb::DataExtractorSP &extractor_sp,
lldb::offset_t data_offset, lldb::offset_t file_offset,
- lldb::offset_t file_size, lldb_private::ModuleSpecList &specs) {
+ lldb::offset_t file_size) {
// Try the ObjectFile plug-ins
for (auto &cbs : PluginManager::GetObjectFileCallbacks()) {
- ModuleSpecList cb_specs = cbs.get_module_specifications(
+ ModuleSpecList specs = cbs.get_module_specifications(
file, extractor_sp, data_offset, file_offset, file_size);
- if (cb_specs.GetSize() > 0) {
- specs.Append(cb_specs);
- return cb_specs.GetSize();
- }
+ if (specs.GetSize() > 0)
+ return specs;
}
// Try the ObjectContainer plug-ins
for (auto &cbs : PluginManager::GetObjectContainerCallbacks()) {
- ModuleSpecList cb_specs = cbs.get_module_specifications(
+ ModuleSpecList specs = cbs.get_module_specifications(
file, extractor_sp, data_offset, file_offset, file_size);
- if (cb_specs.GetSize() > 0) {
- specs.Append(cb_specs);
- return cb_specs.GetSize();
- }
+ if (specs.GetSize() > 0)
+ return specs;
}
- return 0;
+ return {};
}
ObjectFile::ObjectFile(const lldb::ModuleSP &module_sp,
diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index 51ef8d7259d9a..c4bcdfab268c9 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -275,10 +275,8 @@ Status Platform::GetSharedModule(
bool Platform::GetModuleSpec(const FileSpec &module_file_spec,
const ArchSpec &arch, ModuleSpec &module_spec) {
- ModuleSpecList module_specs;
- if (ObjectFile::GetModuleSpecifications(module_file_spec, 0, 0,
- module_specs) == 0)
- return false;
+ ModuleSpecList module_specs =
+ ObjectFile::GetModuleSpecifications(module_file_spec, 0, 0);
ModuleSpec matched_module_spec;
return module_specs.FindMatchingModuleSpec(ModuleSpec(module_file_spec, arch),
diff --git a/lldb/source/Target/TargetList.cpp b/lldb/source/Target/TargetList.cpp
index 623f8c4cf8324..d7c5134d2cbbb 100644
--- a/lldb/source/Target/TargetList.cpp
+++ b/lldb/source/Target/TargetList.cpp
@@ -135,14 +135,13 @@ Status TargetList::CreateTargetInternal(
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);
+ ModuleSpecList module_specs = ObjectFile::GetModuleSpecifications(
+ module_spec.GetFileSpec(), file_offset, file_size);
- if (num_specs > 0) {
+ if (module_specs.GetSize() > 0) {
ModuleSpec matching_module_spec;
- if (num_specs == 1) {
+ if (module_specs.GetSize() == 1) {
if (module_specs.GetModuleSpecAtIndex(0, matching_module_spec)) {
if (platform_arch.IsValid()) {
if (platform_arch.IsCompatibleMatch(
diff --git a/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp b/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
index 411387832f733..2f36974e74fc3 100644
--- a/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ b/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -147,8 +147,9 @@ TEST_F(ObjectFileELFTest, SectionsResolveConsistently) {
*/
TEST_F(ObjectFileELFTest, GetModuleSpecifications_EarlySectionHeaders) {
std::string SO = GetInputFilePath("early-section-headers.so");
- ModuleSpecList Specs;
- ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, Specs));
+ ModuleSpecList Specs =
+ ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0);
+ ASSERT_EQ(Specs.GetSize(), 1u);
ModuleSpec Spec;
ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
UUID Uuid;
@@ -158,8 +159,9 @@ TEST_F(ObjectFileELFTest, GetModuleSpecifications_EarlySectionHeaders) {
TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithNormalFile) {
std::string SO = GetInputFilePath("liboffset-test.so");
- ModuleSpecList Specs;
- ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, Specs));
+ ModuleSpecList Specs =
+ ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0);
+ ASSERT_EQ(Specs.GetSize(), 1u);
ModuleSpec Spec;
ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
UUID Uuid;
@@ -176,9 +178,9 @@ TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithOffsetFile) {
// - 1024-4623: liboffset-test.so (...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/188509
More information about the lldb-commits
mailing list