Skip to content

[lldb][NFC] Change ObjectFile's DataExtractor to a shared ptr #170066

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 1, 2025
Merged

[lldb][NFC] Change ObjectFile's DataExtractor to a shared ptr #170066

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c9dab7d
[lldb][NFC] Change ObjectFile's DataExtractor to a unique ptr
jasonmolenda Dec 1, 2025
04ef7eb
Change the ObjectFile ivar over to a shared pointer. I realized
jasonmolenda Dec 1, 2025
a7e6265
Make m_data_sp a NonNullSharedPtr to enforce
jasonmolenda Dec 1, 2025
1165609
Rename ivar to m_data_nsp to reflect it is always non-null
jasonmolenda Dec 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions lldb/include/lldb/Symbol/ObjectFile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
/// Attempts to parse the object header.
///
/// This function is used as a test to see if a given plug-in instance can
/// parse the header data already contained in ObjectFile::m_data. If an
/// parse the header data already contained in ObjectFile::m_data_sp. If an
/// object file parser does not recognize that magic bytes in a header,
/// false should be returned and the next plug-in can attempt to parse an
/// object file.
Expand Down Expand Up @@ -786,8 +786,11 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
lldb::addr_t m_length; ///< The length of this object file if it is known (can
///be zero if length is unknown or can't be
///determined).
DataExtractor
m_data; ///< The data for this object file so things can be parsed lazily.
lldb::DataExtractorSP
m_data_sp; ///< The data for this object file so things
///< can be parsed lazily. This shared pointer
///< will always have a DataExtractor object,
///< although it may only be default-constructed.
Copy link
Member

@bulbazord bulbazord Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is meant to always be non-null, maybe you could use Jonas's new NonNullSharedPtr? You could catch invariant violations at creation time instead of dereference time.

Copy link
Collaborator Author

@jasonmolenda jasonmolenda Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, perfect. Yeah I wasn't thrilled about how this wasn't enforced. Updated.

lldb::ProcessWP m_process_wp;
/// Set if the object file only exists in memory.
const lldb::addr_t m_memory_addr;
Expand Down
1 change: 1 addition & 0 deletions lldb/include/lldb/lldb-forward.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ typedef std::shared_ptr<lldb_private::CompileUnit> CompUnitSP;
typedef std::shared_ptr<lldb_private::DataBuffer> DataBufferSP;
typedef std::shared_ptr<lldb_private::WritableDataBuffer> WritableDataBufferSP;
typedef std::shared_ptr<lldb_private::DataExtractor> DataExtractorSP;
typedef std::unique_ptr<lldb_private::DataExtractor> DataExtractorUP;
typedef std::shared_ptr<lldb_private::Debugger> DebuggerSP;
typedef std::weak_ptr<lldb_private::Debugger> DebuggerWP;
typedef std::shared_ptr<lldb_private::Disassembler> DisassemblerSP;
Expand Down
10 changes: 6 additions & 4 deletions lldb/source/Expression/ObjectFileJIT.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ ObjectFileJIT::ObjectFileJIT(const lldb::ModuleSP &module_sp,
: ObjectFile(module_sp, nullptr, 0, 0, DataBufferSP(), 0), m_delegate_wp() {
if (delegate_sp) {
m_delegate_wp = delegate_sp;
m_data.SetByteOrder(delegate_sp->GetByteOrder());
m_data.SetAddressByteSize(delegate_sp->GetAddressByteSize());
m_data_sp->SetByteOrder(delegate_sp->GetByteOrder());
m_data_sp->SetAddressByteSize(delegate_sp->GetAddressByteSize());
}
}

Expand All @@ -85,12 +85,14 @@ bool ObjectFileJIT::ParseHeader() {
return false;
}

ByteOrder ObjectFileJIT::GetByteOrder() const { return m_data.GetByteOrder(); }
ByteOrder ObjectFileJIT::GetByteOrder() const {
return m_data_sp->GetByteOrder();
}

bool ObjectFileJIT::IsExecutable() const { return false; }

uint32_t ObjectFileJIT::GetAddressByteSize() const {
return m_data.GetAddressByteSize();
return m_data_sp->GetAddressByteSize();
}

void ObjectFileJIT::ParseSymtab(Symtab &symtab) {
Expand Down
8 changes: 4 additions & 4 deletions lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,13 @@ void ObjectFileBreakpad::CreateSections(SectionList &unified_section_list) {

std::optional<Record::Kind> current_section;
offset_t section_start;
llvm::StringRef text = toStringRef(m_data.GetData());
llvm::StringRef text = toStringRef(m_data_sp->GetData());
uint32_t next_section_id = 1;
auto maybe_add_section = [&](const uint8_t *end_ptr) {
if (!current_section)
return; // We have been called before parsing the first line.

offset_t end_offset = end_ptr - m_data.GetDataStart();
offset_t end_offset = end_ptr - m_data_sp->GetDataStart();
auto section_sp = std::make_shared<Section>(
GetModule(), this, next_section_id++,
ConstString(toString(*current_section)), eSectionTypeOther,
Expand All @@ -162,8 +162,8 @@ void ObjectFileBreakpad::CreateSections(SectionList &unified_section_list) {
maybe_add_section(line.bytes_begin());
// And start a new one.
current_section = next_section;
section_start = line.bytes_begin() - m_data.GetDataStart();
section_start = line.bytes_begin() - m_data_sp->GetDataStart();
}
// Finally, add the last section.
maybe_add_section(m_data.GetDataEnd());
maybe_add_section(m_data_sp->GetDataEnd());
}
4 changes: 2 additions & 2 deletions lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,8 @@ bool ObjectFileCOFF::ParseHeader() {

std::lock_guard<std::recursive_mutex> guard(module->GetMutex());

m_data.SetByteOrder(eByteOrderLittle);
m_data.SetAddressByteSize(GetAddressByteSize());
m_data_sp->SetByteOrder(eByteOrderLittle);
m_data_sp->SetAddressByteSize(GetAddressByteSize());

return true;
}
26 changes: 14 additions & 12 deletions lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ ByteOrder ObjectFileELF::GetByteOrder() const {
}

uint32_t ObjectFileELF::GetAddressByteSize() const {
return m_data.GetAddressByteSize();
return m_data_sp->GetAddressByteSize();
}

AddressClass ObjectFileELF::GetAddressClass(addr_t file_addr) {
Expand Down Expand Up @@ -845,7 +845,7 @@ size_t ObjectFileELF::SectionIndex(const SectionHeaderCollConstIter &I) const {

bool ObjectFileELF::ParseHeader() {
lldb::offset_t offset = 0;
return m_header.Parse(m_data, &offset);
return m_header.Parse(*m_data_sp.get(), &offset);
}

UUID ObjectFileELF::GetUUID() {
Expand Down Expand Up @@ -881,7 +881,7 @@ UUID ObjectFileELF::GetUUID() {
return UUID();

core_notes_crc =
CalculateELFNotesSegmentsCRC32(m_program_headers, m_data);
CalculateELFNotesSegmentsCRC32(m_program_headers, *m_data_sp.get());

if (core_notes_crc) {
// Use 8 bytes - first 4 bytes for *magic* prefix, mainly to make it
Expand All @@ -892,7 +892,7 @@ UUID ObjectFileELF::GetUUID() {
}
} else {
if (!m_gnu_debuglink_crc)
m_gnu_debuglink_crc = calc_crc32(0, m_data);
m_gnu_debuglink_crc = calc_crc32(0, *m_data_sp.get());
if (m_gnu_debuglink_crc) {
// Use 4 bytes of crc from the .gnu_debuglink section.
u32le data(m_gnu_debuglink_crc);
Expand Down Expand Up @@ -1078,7 +1078,8 @@ size_t ObjectFileELF::GetProgramHeaderInfo(ProgramHeaderColl &program_headers,

// ParseProgramHeaders
bool ObjectFileELF::ParseProgramHeaders() {
return GetProgramHeaderInfo(m_program_headers, m_data, m_header) != 0;
return GetProgramHeaderInfo(m_program_headers, *m_data_sp.get(), m_header) !=
0;
}

lldb_private::Status
Expand Down Expand Up @@ -1668,8 +1669,8 @@ ObjectFileELF::StripLinkerSymbolAnnotations(llvm::StringRef symbol_name) const {

// ParseSectionHeaders
size_t ObjectFileELF::ParseSectionHeaders() {
return GetSectionHeaderInfo(m_section_headers, m_data, m_header, m_uuid,
m_gnu_debuglink_file, m_gnu_debuglink_crc,
return GetSectionHeaderInfo(m_section_headers, *m_data_sp.get(), m_header,
m_uuid, m_gnu_debuglink_file, m_gnu_debuglink_crc,
m_arch_spec);
}

Expand Down Expand Up @@ -3678,7 +3679,8 @@ ArchSpec ObjectFileELF::GetArchitecture() {
if (H.p_type != PT_NOTE || H.p_offset == 0 || H.p_filesz == 0)
continue;
DataExtractor data;
if (data.SetData(m_data, H.p_offset, H.p_filesz) == H.p_filesz) {
if (data.SetData(*m_data_sp.get(), H.p_offset, H.p_filesz) ==
H.p_filesz) {
UUID uuid;
RefineModuleDetailsFromNote(data, m_arch_spec, uuid);
}
Expand Down Expand Up @@ -3833,10 +3835,10 @@ llvm::ArrayRef<ELFProgramHeader> ObjectFileELF::ProgramHeaders() {
}

DataExtractor ObjectFileELF::GetSegmentData(const ELFProgramHeader &H) {
// Try and read the program header from our cached m_data which can come from
// the file on disk being mmap'ed or from the initial part of the ELF file we
// read from memory and cached.
DataExtractor data = DataExtractor(m_data, H.p_offset, H.p_filesz);
// Try and read the program header from our cached m_data_sp which can come
// from the file on disk being mmap'ed or from the initial part of the ELF
// file we read from memory and cached.
DataExtractor data = DataExtractor(*m_data_sp.get(), H.p_offset, H.p_filesz);
if (data.GetByteSize() == H.p_filesz)
return data;
if (IsInMemory()) {
Expand Down
Loading
Loading