Skip to content

Commit c9dab7d

Browse files
committed
[lldb][NFC] Change ObjectFile's DataExtractor to a unique ptr
ObjectFile has an m_data DataExtractor ivar which may be default constructed initially, or initialized with a DataBuffer passed in to a ctor. Subclasses will provide the DataExtrator with a Buffer source if not. When a DataBuffer is passed in to the base class ctor, the DataExtractor only has its buffer initalized; we don't yet know the address size and endianness to fully initialize the DataExtractor. This patch changes ObjectFile to instead have a DataExtractorSP ivar which is always initialized with at least a default-constructed DataExtractor object in the base class ctor. The next patch I will be writing is to change the ObjectFile ctor which accepts a DataBuffer to instead accept a DataExtractorSP, so the caller can intialize it with a DataExtractor subclass -- the VirtualizeDataExtractor being added in #168802 The change is otherwise mechanical; all `m_data.` changed to `m_data_up->` and all the places where `m_data` was passed in for a by-ref call were changed to `*m_data_up.get()`. The unique pointer is always initialized to contain an object. I can't remember off hand if I'm making a mistake using a unique_ptr here, given that the ctor may take a DataExtractor as an argument. The caller will have to do std::move(extractor_up) when it calls the ObjectFile ctor for correct behavior. Even though a unique_ptr makes sense internal to ObjectFile, given that it can be passed as an argument, should I use the more straightforward shared_ptr? An ObjectFile only has one of them, so the extra storage for the refcount isn't important. I built & ran the testsuite on macOS and on aarch64-Ubuntu (thanks for getting the Linux testsuite to run on SME-only systems David). All of the ObjectFile subclasses I modifed compile cleanly, but I haven't tested them beyond any unit tests they may have (prob breakpad). rdar://148939795
1 parent 4dc29b8 commit c9dab7d

File tree

11 files changed

+223
-200
lines changed

11 files changed

+223
-200
lines changed

lldb/include/lldb/Symbol/ObjectFile.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
418418
/// Attempts to parse the object header.
419419
///
420420
/// This function is used as a test to see if a given plug-in instance can
421-
/// parse the header data already contained in ObjectFile::m_data. If an
421+
/// parse the header data already contained in ObjectFile::m_data_up. If an
422422
/// object file parser does not recognize that magic bytes in a header,
423423
/// false should be returned and the next plug-in can attempt to parse an
424424
/// object file.
@@ -786,8 +786,11 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
786786
lldb::addr_t m_length; ///< The length of this object file if it is known (can
787787
///be zero if length is unknown or can't be
788788
///determined).
789-
DataExtractor
790-
m_data; ///< The data for this object file so things can be parsed lazily.
789+
lldb::DataExtractorUP
790+
m_data_up; ///< The data for this object file so things
791+
///< can be parsed lazily. This unique pointer
792+
///< will always have a DataExtractor object,
793+
///< although it may only be default-constructed.
791794
lldb::ProcessWP m_process_wp;
792795
/// Set if the object file only exists in memory.
793796
const lldb::addr_t m_memory_addr;

lldb/include/lldb/lldb-forward.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,7 @@ typedef std::shared_ptr<lldb_private::CompileUnit> CompUnitSP;
342342
typedef std::shared_ptr<lldb_private::DataBuffer> DataBufferSP;
343343
typedef std::shared_ptr<lldb_private::WritableDataBuffer> WritableDataBufferSP;
344344
typedef std::shared_ptr<lldb_private::DataExtractor> DataExtractorSP;
345+
typedef std::unique_ptr<lldb_private::DataExtractor> DataExtractorUP;
345346
typedef std::shared_ptr<lldb_private::Debugger> DebuggerSP;
346347
typedef std::weak_ptr<lldb_private::Debugger> DebuggerWP;
347348
typedef std::shared_ptr<lldb_private::Disassembler> DisassemblerSP;

lldb/source/Expression/ObjectFileJIT.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ ObjectFileJIT::ObjectFileJIT(const lldb::ModuleSP &module_sp,
7373
: ObjectFile(module_sp, nullptr, 0, 0, DataBufferSP(), 0), m_delegate_wp() {
7474
if (delegate_sp) {
7575
m_delegate_wp = delegate_sp;
76-
m_data.SetByteOrder(delegate_sp->GetByteOrder());
77-
m_data.SetAddressByteSize(delegate_sp->GetAddressByteSize());
76+
m_data_up->SetByteOrder(delegate_sp->GetByteOrder());
77+
m_data_up->SetAddressByteSize(delegate_sp->GetAddressByteSize());
7878
}
7979
}
8080

@@ -85,12 +85,14 @@ bool ObjectFileJIT::ParseHeader() {
8585
return false;
8686
}
8787

88-
ByteOrder ObjectFileJIT::GetByteOrder() const { return m_data.GetByteOrder(); }
88+
ByteOrder ObjectFileJIT::GetByteOrder() const {
89+
return m_data_up->GetByteOrder();
90+
}
8991

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

9294
uint32_t ObjectFileJIT::GetAddressByteSize() const {
93-
return m_data.GetAddressByteSize();
95+
return m_data_up->GetAddressByteSize();
9496
}
9597

9698
void ObjectFileJIT::ParseSymtab(Symtab &symtab) {

lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,13 +130,13 @@ void ObjectFileBreakpad::CreateSections(SectionList &unified_section_list) {
130130

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

139-
offset_t end_offset = end_ptr - m_data.GetDataStart();
139+
offset_t end_offset = end_ptr - m_data_up->GetDataStart();
140140
auto section_sp = std::make_shared<Section>(
141141
GetModule(), this, next_section_id++,
142142
ConstString(toString(*current_section)), eSectionTypeOther,
@@ -162,8 +162,8 @@ void ObjectFileBreakpad::CreateSections(SectionList &unified_section_list) {
162162
maybe_add_section(line.bytes_begin());
163163
// And start a new one.
164164
current_section = next_section;
165-
section_start = line.bytes_begin() - m_data.GetDataStart();
165+
section_start = line.bytes_begin() - m_data_up->GetDataStart();
166166
}
167167
// Finally, add the last section.
168-
maybe_add_section(m_data.GetDataEnd());
168+
maybe_add_section(m_data_up->GetDataEnd());
169169
}

lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -300,8 +300,8 @@ bool ObjectFileCOFF::ParseHeader() {
300300

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

303-
m_data.SetByteOrder(eByteOrderLittle);
304-
m_data.SetAddressByteSize(GetAddressByteSize());
303+
m_data_up->SetByteOrder(eByteOrderLittle);
304+
m_data_up->SetAddressByteSize(GetAddressByteSize());
305305

306306
return true;
307307
}

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -804,7 +804,7 @@ ByteOrder ObjectFileELF::GetByteOrder() const {
804804
}
805805

806806
uint32_t ObjectFileELF::GetAddressByteSize() const {
807-
return m_data.GetAddressByteSize();
807+
return m_data_up->GetAddressByteSize();
808808
}
809809

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

846846
bool ObjectFileELF::ParseHeader() {
847847
lldb::offset_t offset = 0;
848-
return m_header.Parse(m_data, &offset);
848+
return m_header.Parse(*m_data_up.get(), &offset);
849849
}
850850

851851
UUID ObjectFileELF::GetUUID() {
@@ -881,7 +881,7 @@ UUID ObjectFileELF::GetUUID() {
881881
return UUID();
882882

883883
core_notes_crc =
884-
CalculateELFNotesSegmentsCRC32(m_program_headers, m_data);
884+
CalculateELFNotesSegmentsCRC32(m_program_headers, *m_data_up.get());
885885

886886
if (core_notes_crc) {
887887
// Use 8 bytes - first 4 bytes for *magic* prefix, mainly to make it
@@ -892,7 +892,7 @@ UUID ObjectFileELF::GetUUID() {
892892
}
893893
} else {
894894
if (!m_gnu_debuglink_crc)
895-
m_gnu_debuglink_crc = calc_crc32(0, m_data);
895+
m_gnu_debuglink_crc = calc_crc32(0, *m_data_up.get());
896896
if (m_gnu_debuglink_crc) {
897897
// Use 4 bytes of crc from the .gnu_debuglink section.
898898
u32le data(m_gnu_debuglink_crc);
@@ -1078,7 +1078,8 @@ size_t ObjectFileELF::GetProgramHeaderInfo(ProgramHeaderColl &program_headers,
10781078

10791079
// ParseProgramHeaders
10801080
bool ObjectFileELF::ParseProgramHeaders() {
1081-
return GetProgramHeaderInfo(m_program_headers, m_data, m_header) != 0;
1081+
return GetProgramHeaderInfo(m_program_headers, *m_data_up.get(), m_header) !=
1082+
0;
10821083
}
10831084

10841085
lldb_private::Status
@@ -1668,8 +1669,8 @@ ObjectFileELF::StripLinkerSymbolAnnotations(llvm::StringRef symbol_name) const {
16681669

16691670
// ParseSectionHeaders
16701671
size_t ObjectFileELF::ParseSectionHeaders() {
1671-
return GetSectionHeaderInfo(m_section_headers, m_data, m_header, m_uuid,
1672-
m_gnu_debuglink_file, m_gnu_debuglink_crc,
1672+
return GetSectionHeaderInfo(m_section_headers, *m_data_up.get(), m_header,
1673+
m_uuid, m_gnu_debuglink_file, m_gnu_debuglink_crc,
16731674
m_arch_spec);
16741675
}
16751676

@@ -3678,7 +3679,8 @@ ArchSpec ObjectFileELF::GetArchitecture() {
36783679
if (H.p_type != PT_NOTE || H.p_offset == 0 || H.p_filesz == 0)
36793680
continue;
36803681
DataExtractor data;
3681-
if (data.SetData(m_data, H.p_offset, H.p_filesz) == H.p_filesz) {
3682+
if (data.SetData(*m_data_up.get(), H.p_offset, H.p_filesz) ==
3683+
H.p_filesz) {
36823684
UUID uuid;
36833685
RefineModuleDetailsFromNote(data, m_arch_spec, uuid);
36843686
}
@@ -3833,10 +3835,10 @@ llvm::ArrayRef<ELFProgramHeader> ObjectFileELF::ProgramHeaders() {
38333835
}
38343836

38353837
DataExtractor ObjectFileELF::GetSegmentData(const ELFProgramHeader &H) {
3836-
// Try and read the program header from our cached m_data which can come from
3837-
// the file on disk being mmap'ed or from the initial part of the ELF file we
3838-
// read from memory and cached.
3839-
DataExtractor data = DataExtractor(m_data, H.p_offset, H.p_filesz);
3838+
// Try and read the program header from our cached m_data_up which can come
3839+
// from the file on disk being mmap'ed or from the initial part of the ELF
3840+
// file we read from memory and cached.
3841+
DataExtractor data = DataExtractor(*m_data_up.get(), H.p_offset, H.p_filesz);
38403842
if (data.GetByteSize() == H.p_filesz)
38413843
return data;
38423844
if (IsInMemory()) {

0 commit comments

Comments
 (0)