ELF: Simplify program header iteration

Instead of GetProgramHeaderCount+GetProgramHeaderByIndex, expose an
ArrayRef of all program headers, to enable range-based iteration.
Instead of GetSegmentDataByIndex, expose GetSegmentData, taking a
program header (reference).

This makes the code simpler by enabling range-based loops and also
allowed to remove some null checks, as it became locally obvious that
some pointers can never be null.

llvm-svn: 348928

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

@@ -539,14 +539,13 @@ static uint32_t calc_gnu_debuglink_crc32(const void *buf, size_t size) {
 
 uint32_t ObjectFileELF::CalculateELFNotesSegmentsCRC32(
     const ProgramHeaderColl &program_headers, DataExtractor &object_data) {
-  typedef ProgramHeaderCollConstIter Iter;
 
   uint32_t core_notes_crc = 0;
 
-  for (Iter I = program_headers.begin(); I != program_headers.end(); ++I) {
-    if (I->p_type == llvm::ELF::PT_NOTE) {
-      const elf_off ph_offset = I->p_offset;
-      const size_t ph_size = I->p_filesz;
+  for (const ELFProgramHeader &H : program_headers) {
+    if (H.p_type == llvm::ELF::PT_NOTE) {
+      const elf_off ph_offset = H.p_offset;
+      const size_t ph_size = H.p_filesz;
 
       DataExtractor segment_data;
       if (segment_data.SetData(object_data, ph_offset, ph_size) != ph_size) {
@@ -806,15 +805,11 @@ bool ObjectFileELF::SetLoadAddress(Target &target, lldb::addr_t value,
     if (section_list) {
       if (!value_is_offset) {
         bool found_offset = false;
-        for (size_t i = 1, count = GetProgramHeaderCount(); i <= count; ++i) {
-          const elf::ELFProgramHeader *header = GetProgramHeaderByIndex(i);
-          if (header == nullptr)
+        for (const ELFProgramHeader &H : ProgramHeaders()) {
+          if (H.p_type != PT_LOAD || H.p_offset != 0)
             continue;
 
-          if (header->p_type != PT_LOAD || header->p_offset != 0)
-            continue;
-
-          value = value - header->p_vaddr;
+          value = value - H.p_vaddr;
           found_offset = true;
           break;
         }
@@ -1158,8 +1153,8 @@ size_t ObjectFileELF::GetProgramHeaderInfo(ProgramHeaderColl &program_headers,
 //----------------------------------------------------------------------
 // ParseProgramHeaders
 //----------------------------------------------------------------------
-size_t ObjectFileELF::ParseProgramHeaders() {
-  return GetProgramHeaderInfo(m_program_headers, m_data, m_header);
+bool ObjectFileELF::ParseProgramHeaders() {
+  return GetProgramHeaderInfo(m_program_headers, m_data, m_header) != 0;
 }
 
 lldb_private::Status
@@ -1705,27 +1700,6 @@ size_t ObjectFileELF::GetSectionHeaderInfo(SectionHeaderColl &section_headers,
   return 0;
 }
 
-size_t ObjectFileELF::GetProgramHeaderCount() { return ParseProgramHeaders(); }
-
-const elf::ELFProgramHeader *
-ObjectFileELF::GetProgramHeaderByIndex(lldb::user_id_t id) {
-  if (!id || !ParseProgramHeaders())
-    return NULL;
-
-  if (--id < m_program_headers.size())
-    return &m_program_headers[id];
-
-  return NULL;
-}
-
-DataExtractor ObjectFileELF::GetSegmentDataByIndex(lldb::user_id_t id) {
-  const elf::ELFProgramHeader *segment_header = GetProgramHeaderByIndex(id);
-  if (segment_header == NULL)
-    return DataExtractor();
-  return DataExtractor(m_data, segment_header->p_offset,
-                       segment_header->p_filesz);
-}
-
 llvm::StringRef
 ObjectFileELF::StripLinkerSymbolAnnotations(llvm::StringRef symbol_name) const {
   size_t pos = symbol_name.find('@');
@@ -3181,11 +3155,9 @@ void ObjectFileELF::DumpELFProgramHeaders(Stream *s) {
   s->PutCString("==== --------------- -------- -------- -------- "
                 "-------- -------- ------------------------- --------\n");
 
-  uint32_t idx = 0;
-  for (ProgramHeaderCollConstIter I = m_program_headers.begin();
-       I != m_program_headers.end(); ++I, ++idx) {
-    s->Printf("[%2u] ", idx);
-    ObjectFileELF::DumpELFProgramHeader(s, *I);
+  for (const auto &H : llvm::enumerate(m_program_headers)) {
+    s->Format("[{0,2}] ", H.index());
+    ObjectFileELF::DumpELFProgramHeader(s, H.value());
     s->EOL();
   }
 }
@@ -3305,18 +3277,13 @@ bool ObjectFileELF::GetArchitecture(ArchSpec &arch) {
       m_arch_spec.TripleOSIsUnspecifiedUnknown()) {
     // Core files don't have section headers yet they have PT_NOTE program
     // headers that might shed more light on the architecture
-    if (ParseProgramHeaders()) {
-      for (size_t i = 1, count = GetProgramHeaderCount(); i <= count; ++i) {
-        const elf::ELFProgramHeader *header = GetProgramHeaderByIndex(i);
-        if (header && header->p_type == PT_NOTE && header->p_offset != 0 &&
-            header->p_filesz > 0) {
-          DataExtractor data;
-          if (data.SetData(m_data, header->p_offset, header->p_filesz) ==
-              header->p_filesz) {
-            lldb_private::UUID uuid;
-            RefineModuleDetailsFromNote(data, m_arch_spec, uuid);
-          }
-        }
+    for (const elf::ELFProgramHeader &H : ProgramHeaders()) {
+      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) {
+        UUID uuid;
+        RefineModuleDetailsFromNote(data, m_arch_spec, uuid);
       }
     }
   }
@@ -3448,11 +3415,18 @@ size_t ObjectFileELF::ReadSectionData(Section *section,
   return buffer_sp->GetByteSize();
 }
 
+llvm::ArrayRef<ELFProgramHeader> ObjectFileELF::ProgramHeaders() {
+  ParseProgramHeaders();
+  return m_program_headers;
+}
+
+DataExtractor ObjectFileELF::GetSegmentData(const ELFProgramHeader &H) {
+  return DataExtractor(m_data, H.p_offset, H.p_filesz);
+}
+
 bool ObjectFileELF::AnySegmentHasPhysicalAddress() {
-  size_t header_count = ParseProgramHeaders();
-  for (size_t i = 1; i <= header_count; ++i) {
-    auto header = GetProgramHeaderByIndex(i);
-    if (header->p_paddr != 0)
+  for (const ELFProgramHeader &H : ProgramHeaders()) {
+    if (H.p_paddr != 0)
       return true;
   }
   return false;
@@ -3463,19 +3437,17 @@ ObjectFileELF::GetLoadableData(Target &target) {
   // Create a list of loadable data from loadable segments, using physical
   // addresses if they aren't all null
   std::vector<LoadableData> loadables;
-  size_t header_count = ParseProgramHeaders();
   bool should_use_paddr = AnySegmentHasPhysicalAddress();
-  for (size_t i = 1; i <= header_count; ++i) {
+  for (const ELFProgramHeader &H : ProgramHeaders()) {
     LoadableData loadable;
-    auto header = GetProgramHeaderByIndex(i);
-    if (header->p_type != llvm::ELF::PT_LOAD)
+    if (H.p_type != llvm::ELF::PT_LOAD)
       continue;
-    loadable.Dest = should_use_paddr ? header->p_paddr : header->p_vaddr;
+    loadable.Dest = should_use_paddr ? H.p_paddr : H.p_vaddr;
     if (loadable.Dest == LLDB_INVALID_ADDRESS)
       continue;
-    if (header->p_filesz == 0)
+    if (H.p_filesz == 0)
       continue;
-    auto segment_data = GetSegmentDataByIndex(i);
+    auto segment_data = GetSegmentData(H);
     loadable.Contents = llvm::ArrayRef<uint8_t>(segment_data.GetDataStart(),
                                                 segment_data.GetByteSize());
     loadables.push_back(loadable);

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

@@ -145,14 +145,8 @@ public:
   size_t ReadSectionData(lldb_private::Section *section,
                          lldb_private::DataExtractor &section_data) override;
 
-  // Returns number of program headers found in the ELF file.
-  size_t GetProgramHeaderCount();
-
-  // Returns the program header with the given index.
-  const elf::ELFProgramHeader *GetProgramHeaderByIndex(lldb::user_id_t id);
-
-  // Returns segment data for the given index.
-  lldb_private::DataExtractor GetSegmentDataByIndex(lldb::user_id_t id);
+  llvm::ArrayRef<elf::ELFProgramHeader> ProgramHeaders();
+  lldb_private::DataExtractor GetSegmentData(const elf::ELFProgramHeader &H);
 
   llvm::StringRef
   StripLinkerSymbolAnnotations(llvm::StringRef symbol_name) const override;
@@ -174,8 +168,6 @@ private:
                 const lldb::ProcessSP &process_sp, lldb::addr_t header_addr);
 
   typedef std::vector<elf::ELFProgramHeader> ProgramHeaderColl;
-  typedef ProgramHeaderColl::iterator ProgramHeaderCollIter;
-  typedef ProgramHeaderColl::const_iterator ProgramHeaderCollConstIter;
 
   struct ELFSectionHeaderInfo : public elf::ELFSectionHeader {
     lldb_private::ConstString section_name;
@@ -246,8 +238,8 @@ private:
 
   /// Parses all section headers present in this object file and populates
   /// m_program_headers.  This method will compute the header list only once.
-  /// Returns the number of headers parsed.
-  size_t ParseProgramHeaders();
+  /// Returns true iff the headers have been successfully parsed.
+  bool ParseProgramHeaders();
 
   /// Parses all section headers present in this object file and populates
   /// m_section_headers.  This method will compute the header list only once.

lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp

@@ -118,10 +118,10 @@ ConstString ProcessElfCore::GetPluginName() { return GetPluginNameStatic(); }
 uint32_t ProcessElfCore::GetPluginVersion() { return 1; }
 
 lldb::addr_t ProcessElfCore::AddAddressRangeFromLoadSegment(
-    const elf::ELFProgramHeader *header) {
-  const lldb::addr_t addr = header->p_vaddr;
-  FileRange file_range(header->p_offset, header->p_filesz);
-  VMRangeToFileOffset::Entry range_entry(addr, header->p_memsz, file_range);
+    const elf::ELFProgramHeader &header) {
+  const lldb::addr_t addr = header.p_vaddr;
+  FileRange file_range(header.p_offset, header.p_filesz);
+  VMRangeToFileOffset::Entry range_entry(addr, header.p_memsz, file_range);
 
   VMRangeToFileOffset::Entry *last_entry = m_core_aranges.Back();
   if (last_entry && last_entry->GetRangeEnd() == range_entry.GetRangeBase() &&
@@ -136,12 +136,12 @@ lldb::addr_t ProcessElfCore::AddAddressRangeFromLoadSegment(
   // Keep a separate map of permissions that that isn't coalesced so all ranges
   // are maintained.
   const uint32_t permissions =
-      ((header->p_flags & llvm::ELF::PF_R) ? lldb::ePermissionsReadable : 0u) |
-      ((header->p_flags & llvm::ELF::PF_W) ? lldb::ePermissionsWritable : 0u) |
-      ((header->p_flags & llvm::ELF::PF_X) ? lldb::ePermissionsExecutable : 0u);
+      ((header.p_flags & llvm::ELF::PF_R) ? lldb::ePermissionsReadable : 0u) |
+      ((header.p_flags & llvm::ELF::PF_W) ? lldb::ePermissionsWritable : 0u) |
+      ((header.p_flags & llvm::ELF::PF_X) ? lldb::ePermissionsExecutable : 0u);
 
   m_core_range_infos.Append(
-      VMRangeToPermissions::Entry(addr, header->p_memsz, permissions));
+      VMRangeToPermissions::Entry(addr, header.p_memsz, permissions));
 
   return addr;
 }
@@ -162,8 +162,8 @@ Status ProcessElfCore::DoLoadCore() {
     return error;
   }
 
-  const uint32_t num_segments = core->GetProgramHeaderCount();
-  if (num_segments == 0) {
+  llvm::ArrayRef<elf::ELFProgramHeader> segments = core->ProgramHeaders();
+  if (segments.size() == 0) {
     error.SetErrorString("core file has no segments");
     return error;
   }
@@ -177,20 +177,17 @@ Status ProcessElfCore::DoLoadCore() {
   /// Walk through segments and Thread and Address Map information.
   /// PT_NOTE - Contains Thread and Register information
   /// PT_LOAD - Contains a contiguous range of Process Address Space
-  for (uint32_t i = 1; i <= num_segments; i++) {
-    const elf::ELFProgramHeader *header = core->GetProgramHeaderByIndex(i);
-    assert(header != NULL);
-
-    DataExtractor data = core->GetSegmentDataByIndex(i);
+  for (const elf::ELFProgramHeader &H : segments) {
+    DataExtractor data = core->GetSegmentData(H);
 
     // Parse thread contexts and auxv structure
-    if (header->p_type == llvm::ELF::PT_NOTE) {
-      if (llvm::Error error = ParseThreadContextsFromNoteSegment(header, data))
+    if (H.p_type == llvm::ELF::PT_NOTE) {
+      if (llvm::Error error = ParseThreadContextsFromNoteSegment(H, data))
         return Status(std::move(error));
     }
     // PT_LOAD segments contains address map
-    if (header->p_type == llvm::ELF::PT_LOAD) {
-      lldb::addr_t last_addr = AddAddressRangeFromLoadSegment(header);
+    if (H.p_type == llvm::ELF::PT_LOAD) {
+      lldb::addr_t last_addr = AddAddressRangeFromLoadSegment(H);
       if (vm_addr > last_addr)
         ranges_are_sorted = false;
       vm_addr = last_addr;
@@ -710,8 +707,8 @@ llvm::Error ProcessElfCore::parseLinuxNotes(llvm::ArrayRef<CoreNote> notes) {
 /// A note segment consists of one or more NOTE entries, but their types and
 /// meaning differ depending on the OS.
 llvm::Error ProcessElfCore::ParseThreadContextsFromNoteSegment(
-    const elf::ELFProgramHeader *segment_header, DataExtractor segment_data) {
-  assert(segment_header && segment_header->p_type == llvm::ELF::PT_NOTE);
+    const elf::ELFProgramHeader &segment_header, DataExtractor segment_data) {
+  assert(segment_header.p_type == llvm::ELF::PT_NOTE);
 
   auto notes_or_error = parseSegment(segment_data);
   if(!notes_or_error)

lldb/source/Plugins/Process/elf-core/ProcessElfCore.h

@@ -166,7 +166,7 @@ private:
 
   // Parse thread(s) data structures(prstatus, prpsinfo) from given NOTE segment
   llvm::Error ParseThreadContextsFromNoteSegment(
-      const elf::ELFProgramHeader *segment_header,
+      const elf::ELFProgramHeader &segment_header,
       lldb_private::DataExtractor segment_data);
 
   // Returns number of thread contexts stored in the core file
@@ -174,7 +174,7 @@ private:
 
   // Parse a contiguous address range of the process from LOAD segment
   lldb::addr_t
-  AddAddressRangeFromLoadSegment(const elf::ELFProgramHeader *header);
+  AddAddressRangeFromLoadSegment(const elf::ELFProgramHeader &header);
 
   llvm::Expected<std::vector<lldb_private::CoreNote>>
   parseSegment(const lldb_private::DataExtractor &segment);