[llvm-readelf/obj] - Handle out-of-order PT_LOADs better.

This is https://bugs.llvm.org/show_bug.cgi?id=45698. Specification says that "Loadable segment entries in the program header table appear in ascending order, sorted on the p_vaddr member." Our `toMappedAddr()` relies on this condition. This patch adds a warning when the sorting order of loadable segments is wrong. In this case we force segments sorting and that allows `toMappedAddr()` to work as expected. Differential revision: https://reviews.llvm.org/D92641
llvm · Dec 16, 2020 · 78aea98 · 78aea98
1 parent 0da240c
commit 78aea98
Show file tree

Hide file tree

Showing 5 changed files with 144 additions and 3 deletions.
diff --git a/llvm/include/llvm/Object/ELF.h b/llvm/include/llvm/Object/ELF.h
@@ -183,7 +183,9 @@ class ELFFile {
 
   Expected<Elf_Dyn_Range> dynamicEntries() const;
 
-  Expected<const uint8_t *> toMappedAddr(uint64_t VAddr) const;
+  Expected<const uint8_t *>
+  toMappedAddr(uint64_t VAddr,
+               WarningHandler WarnHandler = &defaultWarningHandler) const;
 
   Expected<Elf_Sym_Range> symbols(const Elf_Shdr *Sec) const {
     if (!Sec)

diff --git a/llvm/lib/Object/ELF.cpp b/llvm/lib/Object/ELF.cpp
@@ -566,7 +566,8 @@ Expected<typename ELFT::DynRange> ELFFile<ELFT>::dynamicEntries() const {
 }
 
 template <class ELFT>
-Expected<const uint8_t *> ELFFile<ELFT>::toMappedAddr(uint64_t VAddr) const {
+Expected<const uint8_t *>
+ELFFile<ELFT>::toMappedAddr(uint64_t VAddr, WarningHandler WarnHandler) const {
   auto ProgramHeadersOrError = program_headers();
   if (!ProgramHeadersOrError)
     return ProgramHeadersOrError.takeError();
@@ -577,6 +578,17 @@ Expected<const uint8_t *> ELFFile<ELFT>::toMappedAddr(uint64_t VAddr) const {
     if (Phdr.p_type == ELF::PT_LOAD)
       LoadSegments.push_back(const_cast<Elf_Phdr *>(&Phdr));
 
+  auto SortPred = [](const Elf_Phdr_Impl<ELFT> *A,
+                     const Elf_Phdr_Impl<ELFT> *B) {
+    return A->p_vaddr < B->p_vaddr;
+  };
+  if (!llvm::is_sorted(LoadSegments, SortPred)) {
+    if (Error E =
+            WarnHandler("loadable segments are unsorted by virtual address"))
+      return std::move(E);
+    llvm::stable_sort(LoadSegments, SortPred);
+  }
+
   const Elf_Phdr *const *I =
       std::upper_bound(LoadSegments.begin(), LoadSegments.end(), VAddr,
                        [](uint64_t VAddr, const Elf_Phdr_Impl<ELFT> *Phdr) {

diff --git a/llvm/test/tools/llvm-readobj/ELF/dynamic-malformed.test b/llvm/test/tools/llvm-readobj/ELF/dynamic-malformed.test
@@ -414,3 +414,58 @@ ProgramHeaders:
 # PAST-THE-EOF-NEXT: {{[(]?}}RPATH{{[)]?}}     Library rpath: [<?>]
 # PAST-THE-EOF-NEXT: {{[(]?}}RUNPATH{{[)]?}}   Library runpath: [<?>]
 # PAST-THE-EOF-NEXT: {{[(]?}}NULL{{[)]?}}      0x0
+
+## Check that we report a warning when we try to map an address, but loadable
+## segments appear unsorted by the p_vaddr member. In this case we are still
+## able to map an address.
+
+# RUN: yaml2obj %s --docnum=7 -o %t10
+# RUN: llvm-readobj --dynamic-table %t10 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t10 --implicit-check-not=warning: --check-prefixes=OUT-OF-ORDER,OUT-OF-ORDER-LLVM
+# RUN: llvm-readelf --dynamic-table %t10 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t10 --implicit-check-not=warning: --check-prefixes=OUT-OF-ORDER,OUT-OF-ORDER-GNU
+
+# OUT-OF-ORDER:           warning: '[[FILE]]': loadable segments are unsorted by virtual address
+
+# OUT-OF-ORDER-LLVM:      DynamicSection [ (2 entries)
+# OUT-OF-ORDER-LLVM-NEXT:   Tag                Type   Name/Value
+# OUT-OF-ORDER-LLVM-NEXT:   0x0000000000000005 STRTAB 0x1000
+# OUT-OF-ORDER-LLVM-NEXT:   0x0000000000000000 NULL   0x0
+# OUT-OF-ORDER-LLVM-NEXT: ]
+
+# OUT-OF-ORDER-GNU:       Dynamic section at offset 0xe9 contains 2 entries:
+# OUT-OF-ORDER-GNU-NEXT:   Tag                Type     Name/Value
+# OUT-OF-ORDER-GNU-NEXT:   0x0000000000000005 (STRTAB) 0x1000
+# OUT-OF-ORDER-GNU-NEXT:   0x0000000000000000 (NULL)   0x0
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+Sections:
+  - Name:    .dynstr
+    Type:    SHT_STRTAB
+    Address: 0x1000
+  - Name:    .dynamic
+    Type:    SHT_DYNAMIC
+    Address: 0x1010
+    Entries:
+      - Tag:   DT_STRTAB
+        Value: 0x1000
+      - Tag:   DT_NULL
+        Value: 0
+Symbols: []
+ProgramHeaders:
+  - Type:     PT_LOAD
+    VAddr:    0x1010
+    FirstSec: .dynamic
+    LastSec:  .dynamic
+  - Type:     PT_LOAD
+    VAddr:    0x1000
+    FirstSec: .dynstr
+    LastSec:  .dynstr
+  - Type:     PT_DYNAMIC
+    VAddr:    0x1010
+    FirstSec: .dynamic
+    LastSec:  .dynamic
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -2087,7 +2087,10 @@ ELFDumper<ELFT>::ELFDumper(const object::ELFObjectFile<ELFT> &O,
 
 template <typename ELFT> void ELFDumper<ELFT>::parseDynamicTable() {
   auto toMappedAddr = [&](uint64_t Tag, uint64_t VAddr) -> const uint8_t * {
-    auto MappedAddrOrError = Obj.toMappedAddr(VAddr);
+    auto MappedAddrOrError = Obj.toMappedAddr(VAddr, [&](const Twine &Msg) {
+      this->reportUniqueWarning(Msg);
+      return Error::success();
+    });
     if (!MappedAddrOrError) {
       this->reportUniqueWarning("unable to parse DT_" +
                                 Obj.getDynamicTagAsString(Tag) + ": " +

diff --git a/llvm/unittests/Object/ELFObjectFileTest.cpp b/llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -328,3 +328,72 @@ TEST(ELFObjectFileTest, InvalidSymtabShndxTest) {
 
   ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
 }
+
+// Test that we are able to create an ELFObjectFile even when loadable segments
+// are unsorted by virtual address.
+// Test that ELFFile<ELFT>::toMappedAddr works properly in this case.
+
+TEST(ELFObjectFileTest, InvalidLoadSegmentsOrderTest) {
+  SmallString<0> Storage;
+  Expected<ELFObjectFile<ELF64LE>> ExpectedFile = toBinary<ELF64LE>(Storage, R"(
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+Sections:
+  - Name:         .foo
+    Type:         SHT_PROGBITS
+    Address:      0x1000
+    Offset:       0x3000
+    ContentArray: [ 0x11 ]
+  - Name:         .bar
+    Type:         SHT_PROGBITS
+    Address:      0x2000
+    Offset:       0x4000
+    ContentArray: [ 0x99 ]
+ProgramHeaders:
+  - Type:     PT_LOAD
+    VAddr:    0x2000
+    FirstSec: .bar
+    LastSec:  .bar
+  - Type:     PT_LOAD
+    VAddr:    0x1000
+    FirstSec: .foo
+    LastSec:  .foo
+)");
+
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+
+  std::string WarnString;
+  auto ToMappedAddr = [&](uint64_t Addr) -> const uint8_t * {
+    Expected<const uint8_t *> DataOrErr =
+        ExpectedFile->getELFFile().toMappedAddr(Addr, [&](const Twine &Msg) {
+          EXPECT_TRUE(WarnString.empty());
+          WarnString = Msg.str();
+          return Error::success();
+        });
+
+    if (!DataOrErr) {
+      ADD_FAILURE() << toString(DataOrErr.takeError());
+      return nullptr;
+    }
+
+    EXPECT_TRUE(WarnString ==
+                "loadable segments are unsorted by virtual address");
+    WarnString = "";
+    return *DataOrErr;
+  };
+
+  const uint8_t *Data = ToMappedAddr(0x1000);
+  ASSERT_TRUE(Data);
+  MemoryBufferRef Buf = ExpectedFile->getMemoryBufferRef();
+  EXPECT_EQ((const char *)Data - Buf.getBufferStart(), 0x3000);
+  EXPECT_EQ(Data[0], 0x11);
+
+  Data = ToMappedAddr(0x2000);
+  ASSERT_TRUE(Data);
+  Buf = ExpectedFile->getMemoryBufferRef();
+  EXPECT_EQ((const char *)Data - Buf.getBufferStart(), 0x4000);
+  EXPECT_EQ(Data[0], 0x99);
+}