Skip to content

Commit ae68377

Browse files
authored
[lldb][NFC] Change ObjectFile's DataExtractor to a shared ptr (#170066)
ObjectFile has an m_data DataExtractor ivar which may be default constructed initially, or initialized with a DataBuffer passed in to its ctor. If the DataExtractor does not get a DataBuffer source passed in, the subclass will initialize it with access to the object file's data. When a DataBuffer is passed in to the base class ctor, the DataExtractor only has its buffer initialized; ObjectFile doesn'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 to take an optional DataExtractorSP, so the caller can pass a DataExtractor subclass -- the VirtualizeDataExtractor being added via #168802 instead of a DataBuffer which is trivially saved into the DataExtractor. The change is otherwise mechanical; all `m_data.` changed to `m_data_sp->` and all the places where `m_data` was passed in for a by-ref call were changed to `*m_data_sp.get()`. The shared pointer is always initialized to contain an object. 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 28d2208 commit ae68377

File tree

11 files changed

+230
-201
lines changed

11 files changed

+230
-201
lines changed

lldb/include/lldb/Symbol/ObjectFile.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "lldb/Utility/Endian.h"
1919
#include "lldb/Utility/FileSpec.h"
2020
#include "lldb/Utility/FileSpecList.h"
21+
#include "lldb/Utility/NonNullSharedPtr.h"
2122
#include "lldb/Utility/StructuredData.h"
2223
#include "lldb/Utility/UUID.h"
2324
#include "lldb/lldb-private.h"
@@ -418,7 +419,7 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
418419
/// Attempts to parse the object header.
419420
///
420421
/// 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
422+
/// parse the header data already contained in ObjectFile::m_data_nsp. If an
422423
/// object file parser does not recognize that magic bytes in a header,
423424
/// false should be returned and the next plug-in can attempt to parse an
424425
/// object file.
@@ -777,6 +778,8 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
777778
std::string GetObjectName() const;
778779

779780
protected:
781+
typedef NonNullSharedPtr<lldb_private::DataExtractor> DataExtractorNSP;
782+
780783
// Member variables.
781784
FileSpec m_file;
782785
Type m_type;
@@ -786,8 +789,10 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
786789
lldb::addr_t m_length; ///< The length of this object file if it is known (can
787790
///be zero if length is unknown or can't be
788791
///determined).
789-
DataExtractor
790-
m_data; ///< The data for this object file so things can be parsed lazily.
792+
DataExtractorNSP m_data_nsp; ///< The data for this object file so things
793+
///< can be parsed lazily. This shared pointer
794+
///< will always have a DataExtractor object,
795+
///< although it may only be default-constructed.
791796
lldb::ProcessWP m_process_wp;
792797
/// Set if the object file only exists in memory.
793798
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_nsp->SetByteOrder(delegate_sp->GetByteOrder());
77+
m_data_nsp->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_nsp->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_nsp->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_nsp->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_nsp->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_nsp->GetDataStart();
166166
}
167167
// Finally, add the last section.
168-
maybe_add_section(m_data.GetDataEnd());
168+
maybe_add_section(m_data_nsp->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_nsp->SetByteOrder(eByteOrderLittle);
304+
m_data_nsp->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_nsp->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_nsp.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_nsp.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_nsp.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_nsp.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_nsp.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_nsp.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_nsp 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_nsp.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)