[llvm] llvm-objdump/ELF: fix crash when reading dyn str table (PR #87519)

Ramkumar Ramachandra via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 5 03:32:36 PDT 2024


https://github.com/artagnon updated https://github.com/llvm/llvm-project/pull/87519

>From 37e0c0d89ea5432dcdc2a3d6dfd995cda4622bd6 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ram.ramachandra at arm.com>
Date: Wed, 3 Apr 2024 16:23:41 +0100
Subject: [PATCH 1/5] llvm-objdump/ELF: fix crash when reading dyn str table

When reading the dynamic string table, llvm-objdump used to crash if
the ELF was malformed, due to an erroneous consumption of error status.
Instead, propogate the error status to the caller, fixing the crash, and
printing a warning.

Fixes #86612.
---
 llvm/test/tools/llvm-objdump/ELF/pr86612.test | 93 +++++++++++++++++++
 llvm/tools/llvm-objdump/ELFDump.cpp           |  2 +-
 2 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 llvm/test/tools/llvm-objdump/ELF/pr86612.test

diff --git a/llvm/test/tools/llvm-objdump/ELF/pr86612.test b/llvm/test/tools/llvm-objdump/ELF/pr86612.test
new file mode 100644
index 00000000000000..7250891f042cb3
--- /dev/null
+++ b/llvm/test/tools/llvm-objdump/ELF/pr86612.test
@@ -0,0 +1,93 @@
+# RUN: yaml2obj %s -o %t1
+# RUN: llvm-objdump -p %t1 | FileCheck %s
+# RUN: llvm-objdump -p %t1 2>&1 >/dev/null | \
+# RUN: FileCheck %s -DFILE=%t1 --implicit-check-not=warning --check-prefix=ERROR
+
+# ERROR: warning: '[[FILE]]': virtual address is not in any segment: 0x474
+# CHECK:      Dynamic Section:
+# CHECK-NEXT:   NEEDED       0xffffffffbe5a0b5f
+# CHECK-NEXT:   FLAGS_1      0x0000000008000000
+# CHECK-NEXT:   DEBUG        0x0000000000000000
+# CHECK-NEXT:   RELA         0x00000000000004e0
+# CHECK-NEXT:   RELASZ       0x0000000000000090
+# CHECK-NEXT:   RELAENT      0x0000000000000018
+# CHECK-NEXT:   RELACOUNT    0x0000000000000004
+# CHECK-NEXT:   JMPREL       0x0000000000000570
+# CHECK-NEXT:   PLTRELSZ     0x0000000000000078
+# CHECK-NEXT:   PLTGOT       0x0000000000003aa0
+# CHECK-NEXT:   PLTREL       0x0000000000000007
+# CHECK-NEXT:   SYMTAB       0x0000000000000308
+# CHECK-NEXT:   SYMENT       0x0000000000000018
+# CHECK-NEXT:   STRTAB       0x0000000000000474
+# CHECK-NEXT:   STRSZ        0x000000000000006b
+# CHECK-NEXT:   GNU_HASH     0x0000000000000408
+# CHECK-NEXT:   HASH         0x000000000000042c
+# CHECK-NEXT:   INIT_ARRAY   0x00000000000028d8
+# CHECK-NEXT:   INIT_ARRAYSZ 0x0000000000000008
+# CHECK-NEXT:   INIT         0x000000000000180e
+# CHECK-NEXT:   FINI         0x0000000000001820
+# CHECK-NEXT:   VERSYM       0x00000000000003c8
+# CHECK-NEXT:   VERNEED      0x00000000000003d8
+# CHECK-NEXT:   VERNEEDNUM   0x0000000000000001
+
+---
+!ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data: ELFDATA2LSB
+  Type: ET_DYN
+Sections:
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Flags: [SHF_WRITE, SHF_ALLOC]
+    Entries:
+      - Tag: DT_NEEDED
+        Value: 0xFFFFFFFFBE5A0B5F
+      - Tag: DT_FLAGS_1
+        Value: 0x8000000
+      - Tag: DT_DEBUG
+        Value: 0x0
+      - Tag: DT_RELA
+        Value: 0x4E0
+      - Tag: DT_RELASZ
+        Value: 0x90
+      - Tag: DT_RELAENT
+        Value: 0x18
+      - Tag: DT_RELACOUNT
+        Value: 0x4
+      - Tag: DT_JMPREL
+        Value: 0x570
+      - Tag: DT_PLTRELSZ
+        Value: 0x78
+      - Tag: DT_PLTGOT
+        Value: 0x3AA0
+      - Tag: DT_PLTREL
+        Value: 0x7
+      - Tag: DT_SYMTAB
+        Value: 0x308
+      - Tag: DT_SYMENT
+        Value: 0x18
+      - Tag: DT_STRTAB
+        Value: 0x474
+      - Tag: DT_STRSZ
+        Value: 0x6B
+      - Tag: DT_GNU_HASH
+        Value: 0x408
+      - Tag: DT_HASH
+        Value: 0x42C
+      - Tag: DT_INIT_ARRAY
+        Value: 0x28D8
+      - Tag: DT_INIT_ARRAYSZ
+        Value: 0x8
+      - Tag: DT_INIT
+        Value: 0x180E
+      - Tag: DT_FINI
+        Value: 0x1820
+      - Tag: DT_VERSYM
+        Value: 0x3C8
+      - Tag: DT_VERNEED
+        Value: 0x3D8
+      - Tag: DT_VERNEEDNUM
+        Value: 0x1
+      - Tag: DT_NULL
+        Value: 0x0
diff --git a/llvm/tools/llvm-objdump/ELFDump.cpp b/llvm/tools/llvm-objdump/ELFDump.cpp
index fda99bd6d33e17..c352182489107c 100644
--- a/llvm/tools/llvm-objdump/ELFDump.cpp
+++ b/llvm/tools/llvm-objdump/ELFDump.cpp
@@ -68,7 +68,7 @@ static Expected<StringRef> getDynamicStrTab(const ELFFile<ELFT> &Elf) {
     if (Dyn.d_tag == ELF::DT_STRTAB) {
       auto MappedAddrOrError = Elf.toMappedAddr(Dyn.getPtr());
       if (!MappedAddrOrError)
-        consumeError(MappedAddrOrError.takeError());
+        return MappedAddrOrError.takeError();
       return StringRef(reinterpret_cast<const char *>(*MappedAddrOrError));
     }
   }

>From fd4fc6b36750c7e7f8261a6f7569c6481eea3416 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ram.ramachandra at arm.com>
Date: Thu, 4 Apr 2024 11:32:28 +0100
Subject: [PATCH 2/5] objdump/test: address James' review; reduce test

---
 .../llvm-objdump/ELF/dynamic-malformed.test   | 29 +++++-
 llvm/test/tools/llvm-objdump/ELF/pr86612.test | 93 -------------------
 2 files changed, 27 insertions(+), 95 deletions(-)
 delete mode 100644 llvm/test/tools/llvm-objdump/ELF/pr86612.test

diff --git a/llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test b/llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test
index b10e4f5e44f181..e221b4608d6880 100644
--- a/llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test
+++ b/llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test
@@ -12,7 +12,6 @@ FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_EXEC
-  Machine: EM_X86_64
 Sections:
   - Name: .dynamic
     Type: SHT_DYNAMIC
@@ -29,10 +28,36 @@ FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_EXEC
-  Machine: EM_X86_64
 Sections:
   - Name: .dynamic
     Type: SHT_DYNAMIC
     Entries:
       - Tag:   DT_SONAME
         Value: 1
+
+# RUN: yaml2obj %s --docnum=3 -o %t.invalidaddr
+# RUN: llvm-objdump -p %t.invalidaddr | FileCheck %s --check-prefix=ADDR
+# RUN: llvm-objdump -p %t.invalidaddr 2>&1 >/dev/null | \
+# RUN: FileCheck %s -DFILE=%t.invalidaddr --implicit-check-not=warning --check-prefix=ADDR-ERROR
+
+# ADDR-ERROR: warning: '[[FILE]]': virtual address is not in any segment: 0x474
+# ADDR:       Dynamic Section:
+# ADDR-NEXT:    NEEDED       0xffffffffbe5a0b5f
+# ADDR-NEXT:    STRTAB       0x0000000000000474
+
+---
+!ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data: ELFDATA2LSB
+  Type: ET_DYN
+Sections:
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag: DT_NEEDED
+        Value: 0xFFFFFFFFBE5A0B5F
+      - Tag: DT_STRTAB
+        Value: 0x474
+      - Tag: DT_NULL
+        Value: 0x0
diff --git a/llvm/test/tools/llvm-objdump/ELF/pr86612.test b/llvm/test/tools/llvm-objdump/ELF/pr86612.test
deleted file mode 100644
index 7250891f042cb3..00000000000000
--- a/llvm/test/tools/llvm-objdump/ELF/pr86612.test
+++ /dev/null
@@ -1,93 +0,0 @@
-# RUN: yaml2obj %s -o %t1
-# RUN: llvm-objdump -p %t1 | FileCheck %s
-# RUN: llvm-objdump -p %t1 2>&1 >/dev/null | \
-# RUN: FileCheck %s -DFILE=%t1 --implicit-check-not=warning --check-prefix=ERROR
-
-# ERROR: warning: '[[FILE]]': virtual address is not in any segment: 0x474
-# CHECK:      Dynamic Section:
-# CHECK-NEXT:   NEEDED       0xffffffffbe5a0b5f
-# CHECK-NEXT:   FLAGS_1      0x0000000008000000
-# CHECK-NEXT:   DEBUG        0x0000000000000000
-# CHECK-NEXT:   RELA         0x00000000000004e0
-# CHECK-NEXT:   RELASZ       0x0000000000000090
-# CHECK-NEXT:   RELAENT      0x0000000000000018
-# CHECK-NEXT:   RELACOUNT    0x0000000000000004
-# CHECK-NEXT:   JMPREL       0x0000000000000570
-# CHECK-NEXT:   PLTRELSZ     0x0000000000000078
-# CHECK-NEXT:   PLTGOT       0x0000000000003aa0
-# CHECK-NEXT:   PLTREL       0x0000000000000007
-# CHECK-NEXT:   SYMTAB       0x0000000000000308
-# CHECK-NEXT:   SYMENT       0x0000000000000018
-# CHECK-NEXT:   STRTAB       0x0000000000000474
-# CHECK-NEXT:   STRSZ        0x000000000000006b
-# CHECK-NEXT:   GNU_HASH     0x0000000000000408
-# CHECK-NEXT:   HASH         0x000000000000042c
-# CHECK-NEXT:   INIT_ARRAY   0x00000000000028d8
-# CHECK-NEXT:   INIT_ARRAYSZ 0x0000000000000008
-# CHECK-NEXT:   INIT         0x000000000000180e
-# CHECK-NEXT:   FINI         0x0000000000001820
-# CHECK-NEXT:   VERSYM       0x00000000000003c8
-# CHECK-NEXT:   VERNEED      0x00000000000003d8
-# CHECK-NEXT:   VERNEEDNUM   0x0000000000000001
-
----
-!ELF
-FileHeader:
-  Class: ELFCLASS64
-  Data: ELFDATA2LSB
-  Type: ET_DYN
-Sections:
-  - Name: .dynamic
-    Type: SHT_DYNAMIC
-    Flags: [SHF_WRITE, SHF_ALLOC]
-    Entries:
-      - Tag: DT_NEEDED
-        Value: 0xFFFFFFFFBE5A0B5F
-      - Tag: DT_FLAGS_1
-        Value: 0x8000000
-      - Tag: DT_DEBUG
-        Value: 0x0
-      - Tag: DT_RELA
-        Value: 0x4E0
-      - Tag: DT_RELASZ
-        Value: 0x90
-      - Tag: DT_RELAENT
-        Value: 0x18
-      - Tag: DT_RELACOUNT
-        Value: 0x4
-      - Tag: DT_JMPREL
-        Value: 0x570
-      - Tag: DT_PLTRELSZ
-        Value: 0x78
-      - Tag: DT_PLTGOT
-        Value: 0x3AA0
-      - Tag: DT_PLTREL
-        Value: 0x7
-      - Tag: DT_SYMTAB
-        Value: 0x308
-      - Tag: DT_SYMENT
-        Value: 0x18
-      - Tag: DT_STRTAB
-        Value: 0x474
-      - Tag: DT_STRSZ
-        Value: 0x6B
-      - Tag: DT_GNU_HASH
-        Value: 0x408
-      - Tag: DT_HASH
-        Value: 0x42C
-      - Tag: DT_INIT_ARRAY
-        Value: 0x28D8
-      - Tag: DT_INIT_ARRAYSZ
-        Value: 0x8
-      - Tag: DT_INIT
-        Value: 0x180E
-      - Tag: DT_FINI
-        Value: 0x1820
-      - Tag: DT_VERSYM
-        Value: 0x3C8
-      - Tag: DT_VERNEED
-        Value: 0x3D8
-      - Tag: DT_VERNEEDNUM
-        Value: 0x1
-      - Tag: DT_NULL
-        Value: 0x0

>From 5509c215eecac0914a5b17183fae81d2f1d2653e Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ram.ramachandra at arm.com>
Date: Thu, 4 Apr 2024 17:06:55 +0100
Subject: [PATCH 3/5] dynamic-malformed.test: minor change from James' review

---
 llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test b/llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test
index e221b4608d6880..05c4c58c7df070 100644
--- a/llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test
+++ b/llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test
@@ -38,7 +38,7 @@ Sections:
 # RUN: yaml2obj %s --docnum=3 -o %t.invalidaddr
 # RUN: llvm-objdump -p %t.invalidaddr | FileCheck %s --check-prefix=ADDR
 # RUN: llvm-objdump -p %t.invalidaddr 2>&1 >/dev/null | \
-# RUN: FileCheck %s -DFILE=%t.invalidaddr --implicit-check-not=warning --check-prefix=ADDR-ERROR
+# RUN:   FileCheck %s -DFILE=%t.invalidaddr --implicit-check-not=warning: --check-prefix=ADDR-ERROR
 
 # ADDR-ERROR: warning: '[[FILE]]': virtual address is not in any segment: 0x474
 # ADDR:       Dynamic Section:

>From 057e2dbea195abf32c3612dff3892d604b3adcbc Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ram.ramachandra at arm.com>
Date: Thu, 4 Apr 2024 19:43:12 +0100
Subject: [PATCH 4/5] llvm-objdump: fix printing order; original bug not fixed

---
 llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test | 7 +++----
 llvm/tools/llvm-objdump/ELFDump.cpp                     | 9 +++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test b/llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test
index 05c4c58c7df070..b1e3ca17b50490 100644
--- a/llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test
+++ b/llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test
@@ -36,12 +36,11 @@ Sections:
         Value: 1
 
 # RUN: yaml2obj %s --docnum=3 -o %t.invalidaddr
-# RUN: llvm-objdump -p %t.invalidaddr | FileCheck %s --check-prefix=ADDR
-# RUN: llvm-objdump -p %t.invalidaddr 2>&1 >/dev/null | \
-# RUN:   FileCheck %s -DFILE=%t.invalidaddr --implicit-check-not=warning: --check-prefix=ADDR-ERROR
+# RUN: llvm-objdump -p %t.invalidaddr 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t.invalidaddr --implicit-check-not=warning: --check-prefix=ADDR
 
-# ADDR-ERROR: warning: '[[FILE]]': virtual address is not in any segment: 0x474
 # ADDR:       Dynamic Section:
+# ADDR-NEXT:  warning: '[[FILE]]': virtual address is not in any segment: 0x474
 # ADDR-NEXT:    NEEDED       0xffffffffbe5a0b5f
 # ADDR-NEXT:    STRTAB       0x0000000000000474
 
diff --git a/llvm/tools/llvm-objdump/ELFDump.cpp b/llvm/tools/llvm-objdump/ELFDump.cpp
index c352182489107c..4248948e946a7d 100644
--- a/llvm/tools/llvm-objdump/ELFDump.cpp
+++ b/llvm/tools/llvm-objdump/ELFDump.cpp
@@ -223,7 +223,6 @@ template <class ELFT> void ELFDumper<ELFT>::printDynamicSection() {
       continue;
 
     std::string Str = Elf.getDynamicTagAsString(Dyn.d_tag);
-    outs() << format(TagFmt.c_str(), Str.c_str());
 
     const char *Fmt =
         ELFT::Is64Bits ? "0x%016" PRIx64 "\n" : "0x%08" PRIx64 "\n";
@@ -232,14 +231,16 @@ template <class ELFT> void ELFDumper<ELFT>::printDynamicSection() {
         Dyn.d_tag == ELF::DT_AUXILIARY || Dyn.d_tag == ELF::DT_FILTER) {
       Expected<StringRef> StrTabOrErr = getDynamicStrTab(Elf);
       if (StrTabOrErr) {
-        const char *Data = StrTabOrErr.get().data();
-        outs() << (Data + Dyn.d_un.d_val) << "\n";
+        const char *Data = StrTabOrErr->data();
+        outs() << format(TagFmt.c_str(), Str.c_str());
+        outs() << Data + Dyn.getVal() << "\n";
         continue;
       }
       reportWarning(toString(StrTabOrErr.takeError()), Obj.getFileName());
       consumeError(StrTabOrErr.takeError());
     }
-    outs() << format(Fmt, (uint64_t)Dyn.d_un.d_val);
+    outs() << format(TagFmt.c_str(), Str.c_str());
+    outs() << format(Fmt, (uint64_t)Dyn.getVal());
   }
 }
 

>From 77c600e0fe50955af4984a5ecf4cbf433db4760c Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ram.ramachandra at arm.com>
Date: Fri, 5 Apr 2024 11:20:26 +0100
Subject: [PATCH 5/5] ELFDump: address review; chain output

---
 llvm/tools/llvm-objdump/ELFDump.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/llvm/tools/llvm-objdump/ELFDump.cpp b/llvm/tools/llvm-objdump/ELFDump.cpp
index 4248948e946a7d..8c184fc1fbb66a 100644
--- a/llvm/tools/llvm-objdump/ELFDump.cpp
+++ b/llvm/tools/llvm-objdump/ELFDump.cpp
@@ -232,15 +232,15 @@ template <class ELFT> void ELFDumper<ELFT>::printDynamicSection() {
       Expected<StringRef> StrTabOrErr = getDynamicStrTab(Elf);
       if (StrTabOrErr) {
         const char *Data = StrTabOrErr->data();
-        outs() << format(TagFmt.c_str(), Str.c_str());
-        outs() << Data + Dyn.getVal() << "\n";
+        outs() << format(TagFmt.c_str(), Str.c_str()) << Data + Dyn.getVal()
+               << "\n";
         continue;
       }
       reportWarning(toString(StrTabOrErr.takeError()), Obj.getFileName());
       consumeError(StrTabOrErr.takeError());
     }
-    outs() << format(TagFmt.c_str(), Str.c_str());
-    outs() << format(Fmt, (uint64_t)Dyn.getVal());
+    outs() << format(TagFmt.c_str(), Str.c_str())
+           << format(Fmt, (uint64_t)Dyn.getVal());
   }
 }
 



More information about the llvm-commits mailing list