[Lldb-commits] [lldb] r349268 - ELF: more section creation cleanup

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Sat Dec 15 05:45:38 PST 2018


Author: labath
Date: Sat Dec 15 05:45:38 2018
New Revision: 349268

URL: http://llvm.org/viewvc/llvm-project?rev=349268&view=rev
Log:
ELF: more section creation cleanup

Summary:
This patch attempts to move as much code as possible out of the
CreateSections function to make room for future improvements there. Some
of this may be slightly over-engineered (VMAddressProvider), but I
wanted to keep the logic of this function very simple, because once I
start taking segment headers into acount (as discussed in D55356), the
function is going to grow significantly.

While in there, I also added tests for various bits of functionality.

This should be NFC, except that I changed the order of hac^H^Heuristicks
for determining section type slightly. Previously, name-based deduction
(.symtab -> symtab) would take precedence over type-based (SHT_SYMTAB ->
symtab) one. In fact we would assert if we ran into a .text section with
type SHT_SYMTAB. Though unlikely to matter in practice, this order
seemed wrong to me, so I have inverted it.

Reviewers: clayborg, krytarowski, espindola

Subscribers: emaste, arichardson, lldb-commits

Differential Revision: https://reviews.llvm.org/D55706

Added:
    lldb/trunk/lit/Modules/ELF/section-addresses.yaml
    lldb/trunk/lit/Modules/ELF/section-permissions.yaml
    lldb/trunk/lit/Modules/ELF/section-types-edgecases.yaml
Modified:
    lldb/trunk/lit/Modules/ELF/compressed-sections.yaml
    lldb/trunk/lit/Modules/ELF/section-types.yaml
    lldb/trunk/lit/Modules/MachO/subsections.yaml
    lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
    lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
    lldb/trunk/tools/lldb-test/lldb-test.cpp

Modified: lldb/trunk/lit/Modules/ELF/compressed-sections.yaml
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/ELF/compressed-sections.yaml?rev=349268&r1=349267&r2=349268&view=diff
==============================================================================
--- lldb/trunk/lit/Modules/ELF/compressed-sections.yaml (original)
+++ lldb/trunk/lit/Modules/ELF/compressed-sections.yaml Sat Dec 15 05:45:38 2018
@@ -19,15 +19,13 @@ Sections:
 
 # CHECK: Name: .hello_elf
 # CHECK-NEXT: Type: regular
-# CHECK-NEXT: Thread specific: no
-# CHECK-NEXT: VM size: 0
+# CHECK: VM size: 0
 # CHECK-NEXT: File size: 28
 # CHECK-NEXT: Data:
 # CHECK-NEXT: 20304050 60708090
 
 # CHECK: Name: .bogus
 # CHECK-NEXT: Type: regular
-# CHECK-NEXT: Thread specific: no
-# CHECK-NEXT: VM size: 0
+# CHECK: VM size: 0
 # CHECK-NEXT: File size: 8
 # CHECK-NEXT: Data: ()

Added: lldb/trunk/lit/Modules/ELF/section-addresses.yaml
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/ELF/section-addresses.yaml?rev=349268&view=auto
==============================================================================
--- lldb/trunk/lit/Modules/ELF/section-addresses.yaml (added)
+++ lldb/trunk/lit/Modules/ELF/section-addresses.yaml Sat Dec 15 05:45:38 2018
@@ -0,0 +1,58 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# CHECK-LABEL: Name: .one
+# CHECK: VM address: 0x0
+
+# CHECK-LABEL: Name: .nonalloc
+# CHECK: VM address: 0x0
+
+# CHECK-LABEL: Name: .two
+# CHECK: VM address: 0x8
+
+# CHECK-LABEL: Name: .three
+# CHECK: VM address: 0xc
+
+# CHECK-LABEL: Name: .four
+# CHECK: VM address: 0xc
+
+# CHECK-LABEL: Name: .five
+# CHECK: VM address: 0x1000
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+  Entry:           0x00000000000007A0
+Sections:
+  - Name:            .one
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    AddressAlign:    0x0000000000000004
+    Content:         DEADBEEFBAADF00D
+  - Name:            .nonalloc
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x0000000000000004
+    Content:         DEADBEEFBAADF00D
+  - Name:            .two
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    AddressAlign:    0x0000000000000004
+    Content:         DE
+  - Name:            .three
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    AddressAlign:    0x0000000000000004
+  - Name:            .four
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    AddressAlign:    0x0000000000000004
+    Content:         DEADBEEFBAADF00D
+  - Name:            .five
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    AddressAlign:    0x0000000000001000
+    Content:         DEADBEEFBAADF00D
+...

Added: lldb/trunk/lit/Modules/ELF/section-permissions.yaml
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/ELF/section-permissions.yaml?rev=349268&view=auto
==============================================================================
--- lldb/trunk/lit/Modules/ELF/section-permissions.yaml (added)
+++ lldb/trunk/lit/Modules/ELF/section-permissions.yaml Sat Dec 15 05:45:38 2018
@@ -0,0 +1,34 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# CHECK-LABEL: Name: .r-x
+# CHECK: Permissions: r-x
+#
+# CHECK-LABEL: Name: .rw-
+# CHECK: Permissions: rw-
+
+# CHECK-LABEL: Name: .---
+# CHECK: Permissions: ---
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_DYN
+  Machine:         EM_X86_64
+  Entry:           0x00000000000007A0
+Sections:
+  - Name:            .r-x
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Content:         DEADBEEFBAADF00D
+  - Name:            .rw-
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    AddressAlign:    0x0000000000000004
+    Content:         DEADBEEFBAADF00D
+  - Name:            .---
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x0000000000000001
+    Content:         DEADBEEFBAADF00D
+...

Added: lldb/trunk/lit/Modules/ELF/section-types-edgecases.yaml
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/ELF/section-types-edgecases.yaml?rev=349268&view=auto
==============================================================================
--- lldb/trunk/lit/Modules/ELF/section-types-edgecases.yaml (added)
+++ lldb/trunk/lit/Modules/ELF/section-types-edgecases.yaml Sat Dec 15 05:45:38 2018
@@ -0,0 +1,35 @@
+# This test doesn't attempt to mandate a specific section classification. It is
+# here to document the existing behavior and to make sure we don't do something
+# completely crazy (like crashing).
+
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# The section is called .data, but it has the SHF_EXECINSTR flag set. Have
+# the flag take precedence over the name.
+# CHECK-LABEL: Name: .data
+# CHECK-NEXT: Type: code
+
+# Section type (SHT_SYMTAB) takes precedence over name-based deduction.
+# CHECK-LABEL: Name: .text
+# CHECK-NEXT: Type: elf-symbol-table
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_DYN
+  Machine:         EM_X86_64
+  Entry:           0x00000000000007A0
+Sections:
+  - Name:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_EXECINSTR, SHF_ALLOC ]
+    AddressAlign:    0x0000000000000004
+    Content:         DEADBEEFBAADF00D
+  - Name:            .text
+    Type:            SHT_SYMTAB
+    Flags:           [ ]
+    AddressAlign:    0x0000000000000004
+    Content:         DEADBEEFBAADF00D
+...

Modified: lldb/trunk/lit/Modules/ELF/section-types.yaml
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/ELF/section-types.yaml?rev=349268&r1=349267&r2=349268&view=diff
==============================================================================
--- lldb/trunk/lit/Modules/ELF/section-types.yaml (original)
+++ lldb/trunk/lit/Modules/ELF/section-types.yaml Sat Dec 15 05:45:38 2018
@@ -25,11 +25,11 @@
 
 # CHECK-LABEL: Name: .tdata
 # CHECK-NEXT: Type: data
-# CHECK-NEXT: Thread specific: yes
+# CHECK: Thread specific: yes
 
 # CHECK-LABEL: Name: .tbss
 # CHECK-NEXT: Type: zero-fill
-# CHECK-NEXT: Thread specific: yes
+# CHECK: Thread specific: yes
 
 --- !ELF
 FileHeader:

Modified: lldb/trunk/lit/Modules/MachO/subsections.yaml
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/MachO/subsections.yaml?rev=349268&r1=349267&r2=349268&view=diff
==============================================================================
--- lldb/trunk/lit/Modules/MachO/subsections.yaml (original)
+++ lldb/trunk/lit/Modules/MachO/subsections.yaml Sat Dec 15 05:45:38 2018
@@ -5,7 +5,9 @@
 #CHECK-NEXT:  Index: 0
 #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
@@ -13,28 +15,36 @@
 #CHECK:       Index: 1
 #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:    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:         Index: 1
 #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:         Index: 2
 #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
 

Modified: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp?rev=349268&r1=349267&r2=349268&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp Sat Dec 15 05:45:38 2018
@@ -1735,7 +1735,7 @@ lldb::user_id_t ObjectFileELF::GetSectio
   return 0;
 }
 
-static SectionType getSectionType(llvm::StringRef Name) {
+static SectionType GetSectionTypeFromName(llvm::StringRef Name) {
   return llvm::StringSwitch<SectionType>(Name)
       .Case(".ARM.exidx", eSectionTypeARMexidx)
       .Case(".ARM.extab", eSectionTypeARMextab)
@@ -1774,16 +1774,85 @@ static SectionType getSectionType(llvm::
       .Default(eSectionTypeOther);
 }
 
+SectionType ObjectFileELF::GetSectionType(const ELFSectionHeaderInfo &H) const {
+  switch (H.sh_type) {
+  case SHT_PROGBITS:
+    if (H.sh_flags & SHF_EXECINSTR)
+      return eSectionTypeCode;
+    break;
+  case SHT_SYMTAB:
+    return eSectionTypeELFSymbolTable;
+  case SHT_DYNSYM:
+    return eSectionTypeELFDynamicSymbols;
+  case SHT_RELA:
+  case SHT_REL:
+    return eSectionTypeELFRelocationEntries;
+  case SHT_DYNAMIC:
+    return eSectionTypeELFDynamicLinkInfo;
+  }
+  SectionType Type = GetSectionTypeFromName(H.section_name.GetStringRef());
+  if (Type == eSectionTypeOther) {
+    // the kalimba toolchain assumes that ELF section names are free-form.
+    // It does support linkscripts which (can) give rise to various
+    // arbitrarily named sections being "Code" or "Data".
+    Type = kalimbaSectionType(m_header, H);
+  }
+  return Type;
+}
+
+static uint32_t GetTargetByteSize(SectionType Type, const ArchSpec &arch) {
+  switch (Type) {
+  case eSectionTypeData:
+  case eSectionTypeZeroFill:
+    return arch.GetDataByteSize();
+  case eSectionTypeCode:
+    return arch.GetCodeByteSize();
+  default:
+    return 1;
+  }
+}
+
+static Permissions GetPermissions(const ELFSectionHeader &H) {
+  Permissions Perm = Permissions(0);
+  if (H.sh_flags & SHF_ALLOC)
+    Perm |= ePermissionsReadable;
+  if (H.sh_flags & SHF_WRITE)
+    Perm |= ePermissionsWritable;
+  if (H.sh_flags & SHF_EXECINSTR)
+    Perm |= ePermissionsExecutable;
+  return Perm;
+}
+
+namespace {
+// (Unlinked) ELF object files usually have 0 for every section address, meaning
+// we need to compute synthetic addresses in order for "file addresses" from
+// different sections to not overlap. This class handles that logic.
+class VMAddressProvider {
+  bool m_synthesizing;
+  addr_t m_next;
+
+public:
+  VMAddressProvider(ObjectFile::Type Type)
+      : m_synthesizing(Type == ObjectFile::Type::eTypeObjectFile), m_next(0) {}
+
+  std::pair<addr_t, addr_t> GetAddressAndSize(const ELFSectionHeader &H) {
+    addr_t address = H.sh_addr;
+    addr_t size = H.sh_flags & SHF_ALLOC ? H.sh_size : 0;
+    if (m_synthesizing && (H.sh_flags & SHF_ALLOC)) {
+      m_next = llvm::alignTo(m_next, std::max<addr_t>(H.sh_addralign, 1));
+      address = m_next;
+      m_next += size;
+    }
+    return {address, size};
+  }
+};
+}
+
 void ObjectFileELF::CreateSections(SectionList &unified_section_list) {
   if (!m_sections_ap.get() && ParseSectionHeaders()) {
     m_sections_ap.reset(new SectionList());
 
-    // Object files frequently have 0 for every section address, meaning we
-    // need to compute synthetic addresses in order for "file addresses" from
-    // different sections to not overlap
-    bool synthaddrs = (CalculateType() == ObjectFile::Type::eTypeObjectFile);
-    uint64_t nextaddr = 0;
-
+    VMAddressProvider address_provider(CalculateType());
     for (SectionHeaderCollIter I = m_section_headers.begin();
          I != m_section_headers.end(); ++I) {
       const ELFSectionHeaderInfo &header = *I;
@@ -1791,69 +1860,18 @@ void ObjectFileELF::CreateSections(Secti
       ConstString &name = I->section_name;
       const uint64_t file_size =
           header.sh_type == SHT_NOBITS ? 0 : header.sh_size;
-      const uint64_t vm_size = header.sh_flags & SHF_ALLOC ? header.sh_size : 0;
 
-      SectionType sect_type = getSectionType(name.GetStringRef());
+      addr_t vm_addr, vm_size;
+      std::tie(vm_addr, vm_size) = address_provider.GetAddressAndSize(header);
 
-      bool is_thread_specific = header.sh_flags & SHF_TLS;
-      const uint32_t permissions =
-          ((header.sh_flags & SHF_ALLOC) ? ePermissionsReadable : 0u) |
-          ((header.sh_flags & SHF_WRITE) ? ePermissionsWritable : 0u) |
-          ((header.sh_flags & SHF_EXECINSTR) ? ePermissionsExecutable : 0u);
-      switch (header.sh_type) {
-      case SHT_SYMTAB:
-        assert(sect_type == eSectionTypeOther);
-        sect_type = eSectionTypeELFSymbolTable;
-        break;
-      case SHT_DYNSYM:
-        assert(sect_type == eSectionTypeOther);
-        sect_type = eSectionTypeELFDynamicSymbols;
-        break;
-      case SHT_RELA:
-      case SHT_REL:
-        assert(sect_type == eSectionTypeOther);
-        sect_type = eSectionTypeELFRelocationEntries;
-        break;
-      case SHT_DYNAMIC:
-        assert(sect_type == eSectionTypeOther);
-        sect_type = eSectionTypeELFDynamicLinkInfo;
-        break;
-      }
-
-      if (eSectionTypeOther == sect_type) {
-        // the kalimba toolchain assumes that ELF section names are free-form.
-        // It does support linkscripts which (can) give rise to various
-        // arbitrarily named sections being "Code" or "Data".
-        sect_type = kalimbaSectionType(m_header, header);
-      }
-
-      // In common case ELF code section can have arbitrary name (for example,
-      // we can specify it using section attribute for particular function) so
-      // assume that section is a code section if it has SHF_EXECINSTR flag set
-      // and has SHT_PROGBITS type.
-      if (eSectionTypeOther == sect_type &&
-          llvm::ELF::SHT_PROGBITS == header.sh_type &&
-          (header.sh_flags & SHF_EXECINSTR)) {
-        sect_type = eSectionTypeCode;
-      }
+      SectionType sect_type = GetSectionType(header);
 
       const uint32_t target_bytes_size =
-          (eSectionTypeData == sect_type || eSectionTypeZeroFill == sect_type)
-              ? m_arch_spec.GetDataByteSize()
-              : eSectionTypeCode == sect_type ? m_arch_spec.GetCodeByteSize()
-                                              : 1;
+          GetTargetByteSize(sect_type, m_arch_spec);
+
       elf::elf_xword log2align =
           (header.sh_addralign == 0) ? 0 : llvm::Log2_64(header.sh_addralign);
 
-      uint64_t addr = header.sh_addr;
-
-      if ((header.sh_flags & SHF_ALLOC) && synthaddrs) {
-          nextaddr =
-              (nextaddr + header.sh_addralign - 1) & ~(header.sh_addralign - 1);
-          addr = nextaddr;
-          nextaddr += vm_size;
-      }
-
       SectionSP section_sp(new Section(
           GetModule(), // Module to which this section belongs.
           this, // ObjectFile to which this section belongs and should read
@@ -1861,7 +1879,7 @@ void ObjectFileELF::CreateSections(Secti
           SectionIndex(I),     // Section ID.
           name,                // Section name.
           sect_type,           // Section type.
-          addr,                // VM address.
+          vm_addr,             // VM address.
           vm_size,             // VM size in bytes of this section.
           header.sh_offset,    // Offset of this section in the file.
           file_size,           // Size of the section as found in the file.
@@ -1869,9 +1887,8 @@ void ObjectFileELF::CreateSections(Secti
           header.sh_flags,     // Flags for this section.
           target_bytes_size)); // Number of host bytes per target byte
 
-      section_sp->SetPermissions(permissions);
-      if (is_thread_specific)
-        section_sp->SetIsThreadSpecific(is_thread_specific);
+      section_sp->SetPermissions(GetPermissions(header));
+      section_sp->SetIsThreadSpecific(header.sh_flags & SHF_TLS);
       m_sections_ap->AddSection(section_sp);
     }
   }

Modified: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h?rev=349268&r1=349267&r2=349268&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h (original)
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h Sat Dec 15 05:45:38 2018
@@ -246,6 +246,8 @@ private:
   /// Returns the number of headers parsed.
   size_t ParseSectionHeaders();
 
+  lldb::SectionType GetSectionType(const ELFSectionHeaderInfo &H) const;
+
   static void ParseARMAttributes(lldb_private::DataExtractor &data,
                                  uint64_t length,
                                  lldb_private::ArchSpec &arch_spec);

Modified: lldb/trunk/tools/lldb-test/lldb-test.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-test/lldb-test.cpp?rev=349268&r1=349267&r2=349268&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-test/lldb-test.cpp (original)
+++ lldb/trunk/tools/lldb-test/lldb-test.cpp Sat Dec 15 05:45:38 2018
@@ -30,6 +30,7 @@
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/State.h"
 #include "lldb/Utility/StreamString.h"
 
 #include "llvm/ADT/IntervalMap.h"
@@ -733,7 +734,9 @@ static void dumpSectionList(LinePrinter
     Printer.formatLine("Index: {0}", I);
     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());
 




More information about the lldb-commits mailing list