[llvm-branch-commits] [llvm] 29d395a - [llvm-objdump] Change printSymbolVersionDependency to use ELFFile API

[Tobias Hieta via llvm-branch-commits]

Author: Fangrui Song
Date: 2022-09-15T16:04:55+02:00
New Revision: 29d395a1b7a8176abb1d6278f7df98301fbe7744

URL: https://github.com/llvm/llvm-project/commit/29d395a1b7a8176abb1d6278f7df98301fbe7744
DIFF: https://github.com/llvm/llvm-project/commit/29d395a1b7a8176abb1d6278f7df98301fbe7744.diff

LOG: [llvm-objdump] Change printSymbolVersionDependency to use ELFFile API

When .gnu.version_r is empty (allowed by readelf but warned by objdump),
llvm-objdump -p may decode the next section as .gnu.version_r and may crash due
to out-of-bounds C string reference. ELFFile<ELFT>::getVersionDependencies
handles 0-entry .gnu.version_r gracefully. Just use it.

Fix https://github.com/llvm/llvm-project/issues/57707

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

(cherry picked from commit 25394c9d10e73b666f4fa1dff2426824894cce58)

Added: 
    

Modified: 
    llvm/include/llvm/Object/ELF.h
    llvm/test/tools/llvm-objdump/ELF/verneed-invalid.test
    llvm/test/tools/llvm-objdump/ELF/verneed.test
    llvm/tools/llvm-objdump/ELFDump.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Object/ELF.h b/llvm/include/llvm/Object/ELF.h
index 794d29fd99133..5eb43777a9512 100644
--- a/llvm/include/llvm/Object/ELF.h
+++ b/llvm/include/llvm/Object/ELF.h
@@ -1028,7 +1028,7 @@ ELFFile<ELFT>::getVersionDependencies(const Elf_Shdr &Sec,
     VN.Offset = VerneedBuf - Start;
 
     if (Verneed->vn_file < StrTab.size())
-      VN.File = std::string(StrTab.drop_front(Verneed->vn_file));
+      VN.File = std::string(StrTab.data() + Verneed->vn_file);
     else
       VN.File = ("<corrupt vn_file: " + Twine(Verneed->vn_file) + ">").str();
 

diff  --git a/llvm/test/tools/llvm-objdump/ELF/verneed-invalid.test b/llvm/test/tools/llvm-objdump/ELF/verneed-invalid.test
index 30011f613037a..1ed3bf9f17082 100644
--- a/llvm/test/tools/llvm-objdump/ELF/verneed-invalid.test
+++ b/llvm/test/tools/llvm-objdump/ELF/verneed-invalid.test
@@ -2,8 +2,8 @@
 # RUN: llvm-objdump -p %t 2>&1 | FileCheck %s --check-prefix=BROKEN-AUX -DFILE=%t
 
 # BROKEN-AUX:      Version References:
-# BROKEN-AUX-NEXT:   required from :
-# BROKEN-AUX-NEXT:     0x00000000 0x00 00
+# BROKEN-AUX-NEXT: warning: '[[FILE]]': invalid SHT_GNU_verneed section with index 2: found a misaligned auxiliary entry at offset 0x11
+# BROKEN-AUX-NOT:  {{.}}
 
 --- !ELF
 FileHeader:

diff  --git a/llvm/test/tools/llvm-objdump/ELF/verneed.test b/llvm/test/tools/llvm-objdump/ELF/verneed.test
index 57e856e542adb..7b38ef95fd412 100644
--- a/llvm/test/tools/llvm-objdump/ELF/verneed.test
+++ b/llvm/test/tools/llvm-objdump/ELF/verneed.test
@@ -46,3 +46,23 @@ Sections:
 DynamicSymbols:
   - Name:    f1
     Binding: STB_GLOBAL
+
+# RUN: yaml2obj --docnum=2 %s -o %t.empty
+# RUN: llvm-objdump -p %t.empty 2>&1 | FileCheck %s --check-prefix=EMPTY --implicit-check-not=warning:
+
+# EMPTY:     Version References:
+# EMPTY-NOT: {{.}}
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name:            .gnu.version_r
+    Type:            SHT_GNU_verneed
+    Flags:           [ SHF_ALLOC ]
+DynamicSymbols:
+  - Name:    f1
+    Binding: STB_GLOBAL

diff  --git a/llvm/tools/llvm-objdump/ELFDump.cpp b/llvm/tools/llvm-objdump/ELFDump.cpp
index ca73dafe2b8ed..61676b4323d2f 100644
--- a/llvm/tools/llvm-objdump/ELFDump.cpp
+++ b/llvm/tools/llvm-objdump/ELFDump.cpp
@@ -282,27 +282,28 @@ static void printProgramHeaders(const ELFFile<ELFT> &Obj, StringRef FileName) {
 }
 
 template <class ELFT>
-static void printSymbolVersionDependency(ArrayRef<uint8_t> Contents,
-                                         StringRef StrTab) {
+static void printSymbolVersionDependency(StringRef FileName,
+                                         const ELFFile<ELFT> &Obj,
+                                         const typename ELFT::Shdr &Sec) {
   outs() << "\nVersion References:\n";
 
-  const uint8_t *Buf = Contents.data();
-  while (Buf) {
-    auto *Verneed = reinterpret_cast<const typename ELFT::Verneed *>(Buf);
-    outs() << "  required from "
-           << StringRef(StrTab.drop_front(Verneed->vn_file).data()) << ":\n";
+  auto WarningHandler = [&](const Twine &Msg) {
+    reportWarning(Msg, FileName);
+    return Error::success();
+  };
+  Expected<std::vector<VerNeed>> V =
+      Obj.getVersionDependencies(Sec, WarningHandler);
+  if (!V) {
+    reportWarning(toString(V.takeError()), FileName);
+    return;
+  }
 
-    const uint8_t *BufAux = Buf + Verneed->vn_aux;
-    while (BufAux) {
-      auto *Vernaux = reinterpret_cast<const typename ELFT::Vernaux *>(BufAux);
-      outs() << "    "
-             << format("0x%08" PRIx32 " ", (uint32_t)Vernaux->vna_hash)
-             << format("0x%02" PRIx16 " ", (uint16_t)Vernaux->vna_flags)
-             << format("%02" PRIu16 " ", (uint16_t)Vernaux->vna_other)
-             << StringRef(StrTab.drop_front(Vernaux->vna_name).data()) << '\n';
-      BufAux = Vernaux->vna_next ? BufAux + Vernaux->vna_next : nullptr;
-    }
-    Buf = Verneed->vn_next ? Buf + Verneed->vn_next : nullptr;
+  raw_fd_ostream &OS = outs();
+  for (const VerNeed &VN : *V) {
+    OS << "  required from " << VN.File << ":\n";
+    for (const VernAux &Aux : VN.AuxV)
+      OS << format("    0x%08x 0x%02x %02u %s\n", Aux.Hash, Aux.Flags,
+                   Aux.Other, Aux.Name.c_str());
   }
 }
 
@@ -355,7 +356,7 @@ static void printSymbolVersionInfo(const ELFFile<ELFT> &Elf,
     StringRef StrTab = unwrapOrError(Elf.getStringTable(*StrTabSec), FileName);
 
     if (Shdr.sh_type == ELF::SHT_GNU_verneed)
-      printSymbolVersionDependency<ELFT>(Contents, StrTab);
+      printSymbolVersionDependency<ELFT>(FileName, Elf, Shdr);
     else
       printSymbolVersionDefinition<ELFT>(Shdr, Contents, StrTab);
   }