Author: Jason Molenda Date: 2026-02-23T22:03:40-08:00 New Revision: 3f024d08355d40e935d89c19d799ccdd298e95a7
URL: https://github.com/llvm/llvm-project/commit/3f024d08355d40e935d89c19d799ccdd298e95a7 DIFF: https://github.com/llvm/llvm-project/commit/3f024d08355d40e935d89c19d799ccdd298e95a7.diff LOG: [lldb] A few small code modernizations and cleanups [NFC] (#182656) 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. Added: Modified: lldb/source/Core/Module.cpp lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.cpp lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp Removed: ################################################################################ diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp index 5a31db672658c..863b43e2385f2 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 = _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
