[PATCH] D68249: [llvm-objdump] Don't throw error for empty dynamic section

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

I would argue that this is indeed an invalid case, though objdump -p does not error.

http://www.sco.com/developers/gabi/latest/ch5.dynamic.html#dynamic_section

> Except for the DT_NULL element at the end of the array, and the relative order of DT_NEEDED elements, entries may appear in any order. Tag values not appearing in the table are reserved.

I take it that there should be a DT_NULL element at the end, so this section cannot be empty. The `if (Dyn.back().d_tag != ELF::DT_NULL)` test below becomes slightly more complex: "dynamic section must end with a DT_NULL" to "non-empty dynamic section must end with a DT_NULL".

That said, -p emitting an error may be a bit inconvenient because -x otherwise does not emit any error.

Maybe we should test: !empty or SHT_NOBITS?

BTW, do you have an opinion on D67090 <https://reviews.llvm.org/D67090>?



================
Comment at: llvm/test/tools/llvm-objdump/elf-keep-debug-only.test:1
+# 'objcopy --only-keep-debug' will create objects with empty dynamic section,
+# llvm-objcopy should not crash for these files.
----------------
We can move this test into elf-dynamic-section.test or rename it to elf-dynamic-section-*.test.


================
Comment at: llvm/test/tools/llvm-objdump/elf-keep-debug-only.test:3
+# llvm-objcopy should not crash for these files.
+# RUN: yaml2obj %s > %t
+# RUN: llvm-objdump -p %t | FileCheck %s
----------------
-o


================
Comment at: llvm/test/tools/llvm-objdump/elf-keep-debug-only.test:16
+  - Name:    .dynamic
+    Type:    SHT_DYNAMIC
+    Address: 0x1010
----------------
For --only-keep-debug, this is SHT_NOBITS


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68249/new/

https://reviews.llvm.org/D68249