[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.
This commit is contained in:
Jason Molenda 2026-02-23 22:03:40 -08:00 committed by GitHub
parent 6654737d9a
commit 3f024d0835
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 38 additions and 42 deletions

View File

@ -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);

View File

@ -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) {

View File

@ -84,12 +84,6 @@ protected:
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 @@ protected:
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 @@ protected:
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 @@ protected:
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 @@ protected:
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;
};

View File

@ -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(

View File

@ -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;
}

View File

@ -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 =