ELF GetSectionHeaderInfo: process SHT_NOTE even without .shstrtab

[jankratochvil]

Summary

This is written for tha YAML testcase of D42914. Its unstripped.yaml contains:

  Start of section headers:          64 (bytes into file)
  Number of program headers:         0
  Section header string table index: 5
Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  [ 1] .note.gnu.build-id NOTE            0000000000400274 0001c0 000024 00   A  0   0  4
  [ 5] .shstrtab         STRTAB          0000000000000000 000226 000034 00      0   0  1

while there is:

ObjectFile::GetModuleSpecifications:
  DataBufferSP data_sp = DataBufferLLVM::CreateSliceFromPath(file.GetPath(), 512, file_offset);

and so ObjectFileELF::GetSectionHeaderInfo has only 0x200 bytes available to find UUID (=build-id). It could find it if there were either program headers present (obj2yaml does not preserve them) or if .shstrtab was present in the first 512 bytes (which it is not).
But then we do not need .shstrtab as we can identify the note section by its SHT_NOTE, we do not need its name. That's this patch.
It does not have a testcase but D42914 does FAIL for me without this patch on Fedora 27 x86_64.

[jankratochvil]

I see now Pavel has solved it by:

[Lldb-commits] [lldb] r324254 - Fix parsing of object files with "early" section headers
https://lists.llvm.org/pipermail/lldb-commits/Week-of-Mon-20180205/038992.html

I am not sure if that is not a more general solution but this patch should be faster/cheaper.
build-id's goal is to be at the very beginning of the file to fit into the Linux kernel CORE_DUMP_DEFAULT_ELF_HEADERS dumping of the first page (4096 bytes). I see now some system files have the build-id at offset 0x274 for example so one would have to also change the 512 bytes chunk of ObjectFile::GetModuleSpecifications: to 4096 to make this patch really valid.

[labath]

As I mentioned in the patch above, I don't think it's worth it trying to tiptoe here, as for 99% of files (basically, anything that is not yaml2obj's output, I think), we will end up reading the whole file anyway. It just increases the number of things that can go wrong.

It might be interesting to make GetModuleSpecifications access only the smallest number of bytes as possible, but for that we should make a larger patch that changes the entire parsing logic.

[jankratochvil]

I think this patch still makes sense but whatever.

source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

size_t ObjectFileELF::GetSectionHeaderInfo(SectionHeaderColl &section_headers,
 
   const unsigned strtab_idx = header.e_shstrndx;
   if (strtab_idx && strtab_idx < section_headers.size()) {
     const ELFSectionHeaderInfo &sheader = section_headers[strtab_idx];
     const size_t byte_size = sheader.sh_size;
     const Elf64_Off offset = sheader.sh_offset;
     lldb_private::DataExtractor shstr_data;
 
-    if (shstr_data.SetData(object_data, offset, byte_size) == byte_size) {
-      for (SectionHeaderCollIter I = section_headers.begin();
-           I != section_headers.end(); ++I) {
-        static ConstString g_sect_name_gnu_debuglink(".gnu_debuglink");
-        const ELFSectionHeaderInfo &sheader = *I;
-        const uint64_t section_size =
-            sheader.sh_type == SHT_NOBITS ? 0 : sheader.sh_size;
-        ConstString name(shstr_data.PeekCStr(I->sh_name));
-
+    const bool shstr_valid =
+        shstr_data.SetData(object_data, offset, byte_size) == byte_size;
+      ConstString name;
+      if (shstr_valid) {
+        name = ConstString(shstr_data.PeekCStr(I->sh_name));
         I->section_name = name;
+      }
 
-        if (arch_spec.IsMIPS()) {
-          uint32_t arch_flags = arch_spec.GetFlags();
-          DataExtractor data;
-          if (sheader.sh_type == SHT_MIPS_ABIFLAGS) {
 
-            if (section_size && (data.SetData(object_data, sheader.sh_offset,
-                                              section_size) == section_size)) {
-              // MIPS ASE Mask is at offset 12 in MIPS.abiflags section
-              lldb::offset_t offset = 12; // MIPS ABI Flags Version: 0
-              arch_flags |= data.GetU32(&offset);
 
-              // The floating point ABI is at offset 7
-              offset = 7;
-              switch (data.GetU8(&offset)) {
-              case llvm::Mips::Val_GNU_MIPS_ABI_FP_ANY:
-                arch_flags |= lldb_private::ArchSpec::eMIPS_ABI_FP_ANY;
-                break;
-              case llvm::Mips::Val_GNU_MIPS_ABI_FP_DOUBLE:
-                arch_flags |= lldb_private::ArchSpec::eMIPS_ABI_FP_DOUBLE;
-                break;
-              case llvm::Mips::Val_GNU_MIPS_ABI_FP_SINGLE:
-                arch_flags |= lldb_private::ArchSpec::eMIPS_ABI_FP_SINGLE;
-                break;
-              case llvm::Mips::Val_GNU_MIPS_ABI_FP_SOFT:
-                arch_flags |= lldb_private::ArchSpec::eMIPS_ABI_FP_SOFT;
-                break;
-              case llvm::Mips::Val_GNU_MIPS_ABI_FP_OLD_64:
-                arch_flags |= lldb_private::ArchSpec::eMIPS_ABI_FP_OLD_64;
-                break;
-              case llvm::Mips::Val_GNU_MIPS_ABI_FP_XX:
-                arch_flags |= lldb_private::ArchSpec::eMIPS_ABI_FP_XX;
-                break;
-              case llvm::Mips::Val_GNU_MIPS_ABI_FP_64:
-                arch_flags |= lldb_private::ArchSpec::eMIPS_ABI_FP_64;
-                break;
-              case llvm::Mips::Val_GNU_MIPS_ABI_FP_64A:
-                arch_flags |= lldb_private::ArchSpec::eMIPS_ABI_FP_64A;
-                break;
-              }
-            }
-          }
-          // Settings appropriate ArchSpec ABI Flags
-          switch (header.e_flags & llvm::ELF::EF_MIPS_ABI) {
-          case llvm::ELF::EF_MIPS_ABI_O32:
-            arch_flags |= lldb_private::ArchSpec::eMIPSABI_O32;
-            break;
-          case EF_MIPS_ABI_O64:
-            arch_flags |= lldb_private::ArchSpec::eMIPSABI_O64;
-            break;
-          case EF_MIPS_ABI_EABI32:
-            arch_flags |= lldb_private::ArchSpec::eMIPSABI_EABI32;
-            break;
-          case EF_MIPS_ABI_EABI64:
-            arch_flags |= lldb_private::ArchSpec::eMIPSABI_EABI64;
-            break;
-          default:
-            // ABI Mask doesn't cover N32 and N64 ABI.
-            if (header.e_ident[EI_CLASS] == llvm::ELF::ELFCLASS64)
-              arch_flags |= lldb_private::ArchSpec::eMIPSABI_N64;
-            else if (header.e_flags & llvm::ELF::EF_MIPS_ABI2)
-              arch_flags |= lldb_private::ArchSpec::eMIPSABI_N32;
-            break;
-          }
-          arch_spec.SetFlags(arch_flags);
-        }
 
-        if (arch_spec.GetMachine() == llvm::Triple::arm ||
-            arch_spec.GetMachine() == llvm::Triple::thumb) {
-          DataExtractor data;
 
-          if (sheader.sh_type == SHT_ARM_ATTRIBUTES && section_size != 0 &&
-              data.SetData(object_data, sheader.sh_offset, section_size) == section_size)
-            ParseARMAttributes(data, section_size, arch_spec);
-        }
 
-        if (name == g_sect_name_gnu_debuglink) {
-          DataExtractor data;
-          if (section_size && (data.SetData(object_data, sheader.sh_offset,
-                                            section_size) == section_size)) {
-            lldb::offset_t gnu_debuglink_offset = 0;
-            gnu_debuglink_file = data.GetCStr(&gnu_debuglink_offset);
-            gnu_debuglink_offset = llvm::alignTo(gnu_debuglink_offset, 4);
-            data.GetU32(&gnu_debuglink_offset, &gnu_debuglink_crc, 1);
-          }
-        }
 
-        // Process ELF note section entries.
-        bool is_note_header = (sheader.sh_type == SHT_NOTE);
 
-        // The section header ".note.android.ident" is stored as a
-        // PROGBITS type header but it is actually a note header.
-        static ConstString g_sect_name_android_ident(".note.android.ident");
-        if (!is_note_header && name == g_sect_name_android_ident)
-          is_note_header = true;
 
-        if (is_note_header) {
-          // Allow notes to refine module info.
-          DataExtractor data;
-          if (section_size && (data.SetData(object_data, sheader.sh_offset,
-                                            section_size) == section_size)) {
-            Status error = RefineModuleDetailsFromNote(data, arch_spec, uuid);
-            if (error.Fail()) {
-              if (log)
-                log->Printf("ObjectFileELF::%s ELF note processing failed: %s",
-                            __FUNCTION__, error.AsCString());
-            }
-          }
-        }
-      }
 
-      // Make any unknown triple components to be unspecified unknowns.
-      if (arch_spec.GetTriple().getVendor() == llvm::Triple::UnknownVendor)
-        arch_spec.GetTriple().setVendorName(llvm::StringRef());
-      if (arch_spec.GetTriple().getOS() == llvm::Triple::UnknownOS)
-        arch_spec.GetTriple().setOSName(llvm::StringRef());
 
+    if (shstr_valid)
       return section_headers.size();
-    }
-  }
 
   section_headers.clear();
   return 0;
 }
 
 size_t ObjectFileELF::GetProgramHeaderCount() { return ParseProgramHeaders(); }
 
 const elf::ELFProgramHeader *