jankratochvil created this revision.
jankratochvil added a reviewer: labath.
Herald added subscribers: emaste, srhines.

This is written for tha YAML testcase of https://reviews.llvm.org/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 https://reviews.llvm.org/D42914 does FAIL for 
me without this patch on Fedora 27 x86_64.


https://reviews.llvm.org/D42931

Files:
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===================================================================
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1617,135 +1617,138 @@
     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;
+    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;
+      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 (arch_spec.IsMIPS()) {
+        uint32_t arch_flags = arch_spec.GetFlags();
+        DataExtractor data;
+        if (sheader.sh_type == SHT_MIPS_ABIFLAGS) {
 
-        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);
+            // 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);
+      }
 
-        // Process ELF note section entries.
-        bool is_note_header = (sheader.sh_type == SHT_NOTE);
+      if (arch_spec.GetMachine() == llvm::Triple::arm ||
+          arch_spec.GetMachine() == llvm::Triple::thumb) {
+        DataExtractor data;
 
-        // 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 (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 (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());
-            }
+      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());
+    // 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();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to