[Lldb-commits] [lldb] [lldb] A few small code modernizations and cleanups [NFC] (PR #182656)
via lldb-commits
lldb-commits at lists.llvm.org
Fri Feb 20 21:55:22 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Jason Molenda (jasonmolenda)
<details>
<summary>Changes</summary>
I was reading through ObjectContainerBSDArchive and came across some dead method decls, a less-than-completely-clear `shared_ptr` typedef in `ObjectContainerBSDArchive::Archive` for a shared_ptr<Archive> which was a little unclear when reading a decl like `shared_ptr archive_sp;` for a local variable.
---
Full diff: https://github.com/llvm/llvm-project/pull/182656.diff
6 Files Affected:
- (modified) lldb/source/Core/Module.cpp (+1-3)
- (modified) lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp (+19-19)
- (modified) lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h (+12-15)
- (modified) lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.cpp (+2-2)
- (modified) lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp (+3-2)
- (modified) lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp (+1-1)
``````````diff
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 659190833c20d..9272cc828195f 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1203,9 +1203,7 @@ ObjectFile *Module::GetObjectFile() {
m_did_load_objfile = true;
// FindPlugin will modify its extractor_sp argument. Do not let it
// modify our m_extractor_sp member.
- DataExtractorSP extractor_sp;
- if (m_extractor_sp)
- extractor_sp = m_extractor_sp;
+ DataExtractorSP extractor_sp = m_extractor_sp;
m_objfile_sp = ObjectFile::FindPlugin(
shared_from_this(), &m_file, m_object_offset,
file_size - m_object_offset, extractor_sp, data_offset);
diff --git a/lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp b/lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
index bee84281ebd52..32604a60841bb 100644
--- a/lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
+++ b/lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
@@ -65,7 +65,7 @@ void ObjectContainerBSDArchive::Object::Dump() const {
file_size);
}
-ObjectContainerBSDArchive::Archive::Archive(const lldb_private::ArchSpec &arch,
+ObjectContainerBSDArchive::Archive::Archive(const ArchSpec &arch,
const llvm::sys::TimePoint<> &time,
lldb::offset_t file_offset,
lldb::DataExtractorSP extractor_sp,
@@ -175,12 +175,12 @@ ObjectContainerBSDArchive::Archive::FindObject(
return nullptr;
}
-ObjectContainerBSDArchive::Archive::shared_ptr
+ObjectContainerBSDArchive::ArchiveSP
ObjectContainerBSDArchive::Archive::FindCachedArchive(
const FileSpec &file, const ArchSpec &arch,
const llvm::sys::TimePoint<> &time, lldb::offset_t file_offset) {
std::lock_guard<std::recursive_mutex> guard(Archive::GetArchiveCacheMutex());
- shared_ptr archive_sp;
+ ArchiveSP archive_sp;
Archive::Map &archive_map = Archive::GetArchiveCache();
Archive::Map::iterator pos = archive_map.find(file);
// Don't cache a value for "archive_map.end()" below since we might delete an
@@ -215,13 +215,13 @@ ObjectContainerBSDArchive::Archive::FindCachedArchive(
return archive_sp;
}
-ObjectContainerBSDArchive::Archive::shared_ptr
+ObjectContainerBSDArchive::ArchiveSP
ObjectContainerBSDArchive::Archive::ParseAndCacheArchiveForFile(
const FileSpec &file, const ArchSpec &arch,
const llvm::sys::TimePoint<> &time, lldb::offset_t file_offset,
DataExtractorSP extractor_sp, ArchiveType archive_type) {
- shared_ptr archive_sp(
- new Archive(arch, time, file_offset, extractor_sp, archive_type));
+ ArchiveSP archive_sp = std::make_shared<Archive>(arch, time, file_offset,
+ extractor_sp, archive_type);
if (archive_sp) {
const size_t num_objects = archive_sp->ParseObjects();
if (num_objects > 0) {
@@ -290,7 +290,7 @@ ObjectContainer *ObjectContainerBSDArchive::CreateInstance(
lldb::offset_t archive_data_offset = 0;
- Archive::shared_ptr archive_sp(Archive::FindCachedArchive(
+ ArchiveSP archive_sp(Archive::FindCachedArchive(
*file, module_sp->GetArchitecture(), module_sp->GetModificationTime(),
file_offset));
std::unique_ptr<ObjectContainerBSDArchive> container_up(
@@ -309,7 +309,7 @@ ObjectContainer *ObjectContainerBSDArchive::CreateInstance(
}
} else {
// No data, just check for a cached archive
- Archive::shared_ptr archive_sp(Archive::FindCachedArchive(
+ ArchiveSP archive_sp(Archive::FindCachedArchive(
*file, module_sp->GetArchitecture(), module_sp->GetModificationTime(),
file_offset));
if (archive_sp) {
@@ -351,14 +351,14 @@ ObjectContainerBSDArchive::MagicBytesMatch(const DataExtractor &data) {
ObjectContainerBSDArchive::ObjectContainerBSDArchive(
const lldb::ModuleSP &module_sp, DataBufferSP &data_sp,
- lldb::offset_t data_offset, const lldb_private::FileSpec *file,
+ lldb::offset_t data_offset, const FileSpec *file,
lldb::offset_t file_offset, lldb::offset_t size, ArchiveType archive_type)
: ObjectContainer(module_sp, file, file_offset, size, data_sp, data_offset),
m_archive_sp() {
m_archive_type = archive_type;
}
-void ObjectContainerBSDArchive::SetArchive(Archive::shared_ptr &archive_sp) {
+void ObjectContainerBSDArchive::SetArchive(ArchiveSP &archive_sp) {
m_archive_sp = archive_sp;
}
@@ -375,7 +375,8 @@ bool ObjectContainerBSDArchive::ParseHeader() {
m_archive_type);
}
// Clear the m_extractor_sp that contains the entire archive data and let
- // our m_archive_sp hold onto the data.
+ // our m_archive_sp hold onto the data. Need to have an empty
+ // DataExtractor for code that assumes it is non-null.
m_extractor_sp = std::make_shared<DataExtractor>();
}
}
@@ -408,9 +409,8 @@ ObjectFileSP ObjectContainerBSDArchive::GetObjectFile(const FileSpec *file) {
object->ar_name.GetStringRef(), m_file);
lldb::offset_t file_offset = 0;
lldb::offset_t file_size = object->size;
- std::shared_ptr<DataBuffer> child_data_sp =
- FileSystem::Instance().CreateDataBuffer(child, file_size,
- file_offset);
+ DataBufferSP child_data_sp = FileSystem::Instance().CreateDataBuffer(
+ child, file_size, file_offset);
if (!child_data_sp ||
child_data_sp->GetByteSize() != object->file_size)
return ObjectFileSP();
@@ -422,6 +422,7 @@ ObjectFileSP ObjectContainerBSDArchive::GetObjectFile(const FileSpec *file) {
object->file_size, extractor_sp, data_offset);
}
lldb::offset_t data_offset = object->file_offset;
+ // Create a new DataExtractor object, its DataBuffer will be shared.
DataExtractorSP extractor_sp =
std::make_shared<DataExtractor>(m_archive_sp->GetData());
return lldb_private::ObjectFile::FindPlugin(
@@ -434,9 +435,9 @@ ObjectFileSP ObjectContainerBSDArchive::GetObjectFile(const FileSpec *file) {
}
size_t ObjectContainerBSDArchive::GetModuleSpecifications(
- const lldb_private::FileSpec &file, lldb::DataExtractorSP &extractor_sp,
+ const 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, ModuleSpecList &specs) {
if (!file || !extractor_sp)
return 0;
@@ -446,15 +447,14 @@ size_t ObjectContainerBSDArchive::GetModuleSpecifications(
// We have data, which means this is the first 512 bytes of the file Check to
// see if the magic bytes match and if they do, read the entire table of
// contents for the archive and cache it
- ArchiveType archive_type =
- ObjectContainerBSDArchive::MagicBytesMatch(*data_extractor_sp.get());
+ ArchiveType archive_type = MagicBytesMatch(*data_extractor_sp);
if (archive_type == ArchiveType::Invalid)
return 0;
const size_t initial_count = specs.GetSize();
llvm::sys::TimePoint<> file_mod_time =
FileSystem::Instance().GetModificationTime(file);
- Archive::shared_ptr archive_sp(
+ ArchiveSP archive_sp(
Archive::FindCachedArchive(file, ArchSpec(), file_mod_time, file_offset));
bool set_archive_arch = false;
if (!archive_sp) {
diff --git a/lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h b/lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
index be6217b9c7b92..82a23a458d58c 100644
--- a/lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
+++ b/lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
@@ -84,12 +84,6 @@ class ObjectContainerBSDArchive : public lldb_private::ObjectContainer {
void Clear();
- lldb::offset_t ExtractFromThin(const lldb_private::DataExtractor &extractor,
- lldb::offset_t offset,
- llvm::StringRef stringTable);
-
- lldb::offset_t Extract(const lldb_private::DataExtractor &extractor,
- lldb::offset_t offset);
/// Object name in the archive.
lldb_private::ConstString ar_name;
@@ -108,10 +102,12 @@ class ObjectContainerBSDArchive : public lldb_private::ObjectContainer {
void Dump() const;
};
+ class Archive;
+ typedef std::shared_ptr<Archive> ArchiveSP;
+
class Archive {
public:
- typedef std::shared_ptr<Archive> shared_ptr;
- typedef std::multimap<lldb_private::FileSpec, shared_ptr> Map;
+ typedef std::multimap<lldb_private::FileSpec, ArchiveSP> Map;
Archive(const lldb_private::ArchSpec &arch,
const llvm::sys::TimePoint<> &mod_time, lldb::offset_t file_offset,
@@ -123,11 +119,12 @@ class ObjectContainerBSDArchive : public lldb_private::ObjectContainer {
static std::recursive_mutex &GetArchiveCacheMutex();
- static Archive::shared_ptr FindCachedArchive(
- const lldb_private::FileSpec &file, const lldb_private::ArchSpec &arch,
- const llvm::sys::TimePoint<> &mod_time, lldb::offset_t file_offset);
+ static ArchiveSP FindCachedArchive(const lldb_private::FileSpec &file,
+ const lldb_private::ArchSpec &arch,
+ const llvm::sys::TimePoint<> &mod_time,
+ lldb::offset_t file_offset);
- static Archive::shared_ptr ParseAndCacheArchiveForFile(
+ static ArchiveSP ParseAndCacheArchiveForFile(
const lldb_private::FileSpec &file, const lldb_private::ArchSpec &arch,
const llvm::sys::TimePoint<> &mod_time, lldb::offset_t file_offset,
lldb::DataExtractorSP extractor_sp, ArchiveType archive_type);
@@ -157,7 +154,7 @@ class ObjectContainerBSDArchive : public lldb_private::ObjectContainer {
bool HasNoExternalReferences() const;
- lldb_private::DataExtractor &GetData() { return *m_extractor_sp.get(); }
+ lldb_private::DataExtractor &GetData() { return *m_extractor_sp; }
lldb::DataExtractorSP &GetDataSP() { return m_extractor_sp; }
ArchiveType GetArchiveType() { return m_archive_type; }
@@ -176,9 +173,9 @@ class ObjectContainerBSDArchive : public lldb_private::ObjectContainer {
ArchiveType m_archive_type;
};
- void SetArchive(Archive::shared_ptr &archive_sp);
+ void SetArchive(ArchiveSP &archive_sp);
- Archive::shared_ptr m_archive_sp;
+ ArchiveSP m_archive_sp;
ArchiveType m_archive_type;
};
diff --git a/lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.cpp b/lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.cpp
index 0b99e3faafabd..4f1405abf8b56 100644
--- a/lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.cpp
+++ b/lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.cpp
@@ -199,7 +199,7 @@ bool ObjectContainerMachOFileset::ParseHeader() {
std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
- std::optional<mach_header> header = ParseMachOHeader(*m_extractor_sp.get());
+ std::optional<mach_header> header = ParseMachOHeader(*m_extractor_sp);
if (!header)
return false;
@@ -216,7 +216,7 @@ bool ObjectContainerMachOFileset::ParseHeader() {
m_extractor_sp->SetData(data_sp);
}
- return ParseFileset(*m_extractor_sp.get(), *header, m_entries, m_memory_addr);
+ return ParseFileset(*m_extractor_sp, *header, m_entries, m_memory_addr);
}
size_t ObjectContainerMachOFileset::GetModuleSpecifications(
diff --git a/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp b/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
index 366a8852b9b5e..0ff0a05e24455 100644
--- a/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
+++ b/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
@@ -74,9 +74,10 @@ ObjectContainerUniversalMachO::ObjectContainerUniversalMachO(
ObjectContainerUniversalMachO::~ObjectContainerUniversalMachO() = default;
bool ObjectContainerUniversalMachO::ParseHeader() {
- bool success = ParseHeader(*m_extractor_sp.get(), m_header, m_fat_archs);
+ bool success = ParseHeader(*m_extractor_sp, m_header, m_fat_archs);
// We no longer need any data, we parsed all we needed to parse and cached it
- // in m_header and m_fat_archs
+ // in m_header and m_fat_archs. Need to have an empty DataExtractor for code
+ // that assumes it is non-null.
m_extractor_sp = std::make_shared<DataExtractor>();
return success;
}
diff --git a/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp b/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
index 41acc0207fb80..d3ec0ff1f81a1 100644
--- a/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
+++ b/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
@@ -70,7 +70,7 @@ ObjectFile *ObjectFileBreakpad::CreateInstance(const ModuleSP &module_sp,
extractor_sp = std::make_shared<DataExtractor>(data_sp);
data_offset = 0;
}
- // If this is opearting on a VirtualDataExtractor, it can have
+ // If this is operating on a VirtualDataExtractor, it can have
// gaps between valid bytes in the DataBuffer. We extract an
// ArrayRef of the raw bytes, and can segfault.
DataExtractorSP contiguous_extractor_sp =
``````````
</details>
https://github.com/llvm/llvm-project/pull/182656
More information about the lldb-commits
mailing list