ELF: Don't create sections for section header index 0

[labath]

Summary

The first section header does not define a real section. Instead it is
used for various elf extensions. This patch skips creation of a section
for index 0.

This has one furtunate side-effect, as it allows us to use the section
header index as the Section ID (where 0 is also invalid), and allows us
to get rid of a lot of spurious +1s in the ObjectFileELF code.

[clayborg]

Looks good. See inline comment and fix if you agree.

source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

1993 ↗

(On Diff #178431)

Should we rename "section_idx" to "shndx" to make sure people know it is not a section index, but the ELF version of it?

lldb/trunk/lit/Modules/ELF/many-sections.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux %s -o %t
 # RUN: lldb-test object-file %t | FileCheck %s
 
 ## Check that we are able to parse ELF files with more than SHN_LORESERVE
 ## sections. This generates a file that contains 64k sections from
 ## aaaaaaaa..dddddddd, plus a couple of standard ones (.strtab, etc.)
 ## Check the number is correct plus the names of a couple of chosen sections.
 
-# CHECK: Showing 65541 sections
+# CHECK: Showing 65540 sections
 # CHECK: Name: aaaaaaaa
 # CHECK: Name: bbbbbbbb
 # CHECK: Name: cccccccc
 # CHECK: Name: abcdabcd
 # CHECK: Name: dddddddd
 
 .macro gen_sections4 x
   .section a\x

lldb/trunk/lit/Modules/MachO/subsections.yaml

 # RUN: yaml2obj %s > %t
 # RUN: lldb-test object-file %t | FileCheck %s
 
 #CHECK:     Showing 2 sections
 #CHECK-NEXT:  Index: 0
+#CHECK-NEXT:  ID: 0x100
 #CHECK-NEXT:  Name: __PAGEZERO
 #CHECK-NEXT:  Type: container
 #CHECK-NEXT:  Permissions: ---
 #CHECK-NEXT:  Thread specific: no
 #CHECK-NEXT:  VM address: 0x0
 #CHECK-NEXT:  VM size: 4294967296
 #CHECK-NEXT:  File size: 0
 #CHECK-NEXT:  There are no subsections
 #CHECK-EMPTY:
 #CHECK-NEXT:  Index: 1
+#CHECK-NEXT:  ID: 0x200
 #CHECK-NEXT:  Name: __TEXT
 #CHECK-NEXT:  Type: container
 #CHECK-NEXT:  Permissions: r-x
 #CHECK-NEXT:  Thread specific: no
 #CHECK-NEXT:  VM address: 0x100000000
 #CHECK-NEXT:  VM size: 4096
 #CHECK-NEXT:  File size: 4096
 #CHECK-NEXT:  Showing 3 subsections
 #CHECK-NEXT:    Index: 0
+#CHECK-NEXT:    ID: 0x1
 #CHECK-NEXT:    Name: __text
 #CHECK-NEXT:    Type: code
 #CHECK-NEXT:    Permissions: r-x
 #CHECK-NEXT:    Thread specific: no
 #CHECK-NEXT:    VM address: 0x100000f30
 #CHECK-NEXT:    VM size: 22
 #CHECK-NEXT:    File size: 22
 #CHECK-EMPTY:
 #CHECK-NEXT:    Index: 1
+#CHECK-NEXT:    ID: 0x2
 #CHECK-NEXT:    Name: __unwind_info
 #CHECK-NEXT:    Type: compact-unwind
 #CHECK-NEXT:    Permissions: r-x
 #CHECK-NEXT:    Thread specific: no
 #CHECK-NEXT:    VM address: 0x100000f48
 #CHECK-NEXT:    VM size: 76
 #CHECK-NEXT:    File size: 76
 #CHECK-EMPTY:
 #CHECK-NEXT:    Index: 2
+#CHECK-NEXT:    ID: 0x3
 #CHECK-NEXT:    Name: __eh_frame
 #CHECK-NEXT:    Type: eh-frame
 #CHECK-NEXT:    Permissions: r-x
 #CHECK-NEXT:    Thread specific: no
 #CHECK-NEXT:    VM address: 0x100000f98
 #CHECK-NEXT:    VM size: 104
 #CHECK-NEXT:    File size: 104
 

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

private:
   lldb_private::Address m_entry_point_address;
 
   /// The architecture detected from parsing elf file contents.
   lldb_private::ArchSpec m_arch_spec;
 
   /// The address class for each symbol in the elf file
   FileAddressToAddressClassMap m_address_class_map;
 
-  /// Returns a 1 based index of the given section header.
+  /// Returns the index of the given section header.
   size_t SectionIndex(const SectionHeaderCollIter &I);
 
-  /// Returns a 1 based index of the given section header.
+  /// Returns the index of the given section header.
   size_t SectionIndex(const SectionHeaderCollConstIter &I) const;
 
   // Parses the ELF program headers.
   static size_t GetProgramHeaderInfo(ProgramHeaderColl &program_headers,
                                      lldb_private::DataExtractor &object_data,
                                      const elf::ELFHeader &header);
 
   // Finds PT_NOTE segments and calculates their crc sum.

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

AddressClass ObjectFileELF::GetAddressClass(addr_t file_addr) {
 
   // Move iterator to the address class entry preceding address
   --ub;
 
   return ub->second;
 }
 
 size_t ObjectFileELF::SectionIndex(const SectionHeaderCollIter &I) {
-  return std::distance(m_section_headers.begin(), I) + 1u;
+  return std::distance(m_section_headers.begin(), I);
 }
 
 size_t ObjectFileELF::SectionIndex(const SectionHeaderCollConstIter &I) const {
-  return std::distance(m_section_headers.begin(), I) + 1u;
+  return std::distance(m_section_headers.begin(), I);
 }
 
 bool ObjectFileELF::ParseHeader() {
   lldb::offset_t offset = 0;
   return m_header.Parse(m_data, &offset);
 }
 
 bool ObjectFileELF::GetUUID(lldb_private::UUID *uuid) {
if (!dynsym)
     return 0;
   assert(dynsym->GetObjectFile() == this);
 
   const ELFSectionHeaderInfo *header = GetSectionHeaderByIndex(dynsym->GetID());
   if (!header)
     return 0;
   // sh_link: section header index of string table used by entries in the
   // section.
-  Section *dynstr = section_list->FindSectionByID(header->sh_link + 1).get();
+  Section *dynstr = section_list->FindSectionByID(header->sh_link).get();
   if (!dynstr)
     return 0;
 
   DataExtractor dynsym_data;
   DataExtractor dynstr_data;
   if (ReadSectionData(dynsym, dynsym_data) &&
       ReadSectionData(dynstr, dynstr_data)) {
     ELFDynamic symbol;

 size_t ObjectFileELF::ParseSectionHeaders() {
   return GetSectionHeaderInfo(m_section_headers, m_data, m_header, m_uuid,
                               m_gnu_debuglink_file, m_gnu_debuglink_crc,
                               m_arch_spec);
 }
 
 const ObjectFileELF::ELFSectionHeaderInfo *
 ObjectFileELF::GetSectionHeaderByIndex(lldb::user_id_t id) {
-  if (!id || !ParseSectionHeaders())
+  if (!ParseSectionHeaders())
     return NULL;
 
-  if (--id < m_section_headers.size())
+  if (id < m_section_headers.size())
     return &m_section_headers[id];
 
   return NULL;
 }
 
 lldb::user_id_t ObjectFileELF::GetSectionIndexByName(const char *name) {
   if (!name || !name[0] || !ParseSectionHeaders())
     return 0;

 };
 }
 
 void ObjectFileELF::CreateSections(SectionList &unified_section_list) {
   if (!m_sections_ap.get() && ParseSectionHeaders()) {
     m_sections_ap.reset(new SectionList());
 
     VMAddressProvider address_provider(CalculateType());
-    for (SectionHeaderCollIter I = m_section_headers.begin();
+    for (SectionHeaderCollIter I = std::next(m_section_headers.begin());
          I != m_section_headers.end(); ++I) {
       const ELFSectionHeaderInfo &header = *I;
 
       ConstString &name = I->section_name;
       const uint64_t file_size =
           header.sh_type == SHT_NOBITS ? 0 : header.sh_size;
 
       addr_t vm_addr, vm_size;
for (i = 0; i < num_symbols; ++i) {
     // Skipping oatdata and oatexec sections if it is requested. See details
     // above the definition of skip_oatdata_oatexec for the reasons.
     if (skip_oatdata_oatexec && (::strcmp(symbol_name, "oatdata") == 0 ||
                                  ::strcmp(symbol_name, "oatexec") == 0))
       continue;
 
     SectionSP symbol_section_sp;
     SymbolType symbol_type = eSymbolTypeInvalid;
-    Elf64_Half section_idx = symbol.st_shndx;
+    Elf64_Half shndx = symbol.st_shndx;
 
-    switch (section_idx) {
+    switch (shndx) {
     case SHN_ABS:
       symbol_type = eSymbolTypeAbsolute;
       break;
     case SHN_UNDEF:
       symbol_type = eSymbolTypeUndefined;
       break;
     default:
-      symbol_section_sp = section_list->GetSectionAtIndex(section_idx);
+      symbol_section_sp = section_list->FindSectionByID(shndx);
       break;
     }
 
     // If a symbol is undefined do not process it further even if it has a STT
     // type
     if (symbol_type != eSymbolTypeUndefined) {
       switch (symbol.getType()) {
       default:
if (arch.IsValid()) {
         }
       }
     }
 
     // symbol_value_offset may contain 0 for ARM symbols or -1 for THUMB
     // symbols. See above for more details.
     uint64_t symbol_value = symbol.st_value + symbol_value_offset;
 
-    if (symbol_section_sp == nullptr && section_idx == SHN_ABS &&
+    if (symbol_section_sp == nullptr && shndx == SHN_ABS &&
         symbol.st_size != 0) {
       // We don't have a section for a symbol with non-zero size. Create a new
       // section for it so the address range covered by the symbol is also
       // covered by the module (represented through the section list). It is
       // needed so module lookup for the addresses covered by this symbol will
       // be successfull. This case happens for absolute symbols.
       ConstString fake_section_name(std::string(".absolute.") + symbol_name);
       symbol_section_sp =
unsigned ObjectFileELF::ParseSymbolTable(Symtab *symbol_table,
   if (!section_list)
     return 0;
 
   user_id_t symtab_id = symtab->GetID();
   const ELFSectionHeaderInfo *symtab_hdr = GetSectionHeaderByIndex(symtab_id);
   assert(symtab_hdr->sh_type == SHT_SYMTAB ||
          symtab_hdr->sh_type == SHT_DYNSYM);
 
-  // sh_link: section header index of associated string table. Section ID's are
-  // ones based.
-  user_id_t strtab_id = symtab_hdr->sh_link + 1;
+  // sh_link: section header index of associated string table.
+  user_id_t strtab_id = symtab_hdr->sh_link;
   Section *strtab = section_list->FindSectionByID(strtab_id).get();
 
   if (symtab && strtab) {
     assert(symtab->GetObjectFile() == this);
     assert(strtab->GetObjectFile() == this);
 
     DataExtractor symtab_data;
     DataExtractor strtab_data;
ObjectFileELF::ParseTrampolineSymbols(Symtab *symbol_table, user_id_t start_id,
 
   // Get PLT section.  We cannot use rel_hdr->sh_info, since current linkers
   // point that to the .got.plt or .got section instead of .plt.
   user_id_t plt_id = GetSectionIndexByName(".plt");
 
   if (!symtab_id || !plt_id)
     return 0;
 
-  // Section ID's are ones based;
-  symtab_id++;
-  plt_id++;
-
   const ELFSectionHeaderInfo *plt_hdr = GetSectionHeaderByIndex(plt_id);
   if (!plt_hdr)
     return 0;
 
   const ELFSectionHeaderInfo *sym_hdr = GetSectionHeaderByIndex(symtab_id);
   if (!sym_hdr)
     return 0;
 
ObjectFileELF::ParseTrampolineSymbols(Symtab *symbol_table, user_id_t start_id,
   if (!plt_section_sp)
     return 0;
 
   Section *symtab = section_list->FindSectionByID(symtab_id).get();
   if (!symtab)
     return 0;
 
   // sh_link points to associated string table.
-  Section *strtab = section_list->FindSectionByID(sym_hdr->sh_link + 1).get();
+  Section *strtab = section_list->FindSectionByID(sym_hdr->sh_link).get();
   if (!strtab)
     return 0;
 
   DataExtractor rel_data;
   if (!ReadSectionData(rel_section, rel_data))
     return 0;
 
   DataExtractor symtab_data;
unsigned ObjectFileELF::RelocateDebugSections(const ELFSectionHeader *rel_hdr,
                                               lldb_private::Symtab *thetab) {
   assert(rel_hdr->sh_type == SHT_RELA || rel_hdr->sh_type == SHT_REL);
 
   // Parse in the section list if needed.
   SectionList *section_list = GetSectionList();
   if (!section_list)
     return 0;
 
-  // Section ID's are ones based.
-  user_id_t symtab_id = rel_hdr->sh_link + 1;
-  user_id_t debug_id = rel_hdr->sh_info + 1;
+  user_id_t symtab_id = rel_hdr->sh_link;
+  user_id_t debug_id = rel_hdr->sh_info;
 
   const ELFSectionHeader *symtab_hdr = GetSectionHeaderByIndex(symtab_id);
   if (!symtab_hdr)
     return 0;
 
   const ELFSectionHeader *debug_hdr = GetSectionHeaderByIndex(debug_id);
   if (!debug_hdr)
     return 0;

lldb/trunk/tools/lldb-test/lldb-test.cpp

static void dumpSectionList(LinePrinter &Printer, const SectionList &List, bool is_subsection) {
   }
   Printer.formatLine("Showing {0} {1}sections", Count,
                      is_subsection ? "sub" : "");
   for (size_t I = 0; I < Count; ++I) {
     auto S = List.GetSectionAtIndex(I);
     assert(S);
     AutoIndent Indent(Printer, 2);
     Printer.formatLine("Index: {0}", I);
+    Printer.formatLine("ID: {0:x}", S->GetID());
     Printer.formatLine("Name: {0}", S->GetName().GetStringRef());
     Printer.formatLine("Type: {0}", S->GetTypeAsCString());
     Printer.formatLine("Permissions: {0}", GetPermissionsAsCString(S->GetPermissions()));
     Printer.formatLine("Thread specific: {0:y}", S->IsThreadSpecific());
     Printer.formatLine("VM address: {0:x}", S->GetFileAddress());
     Printer.formatLine("VM size: {0}", S->GetByteSize());
     Printer.formatLine("File size: {0}", S->GetFileSize());