[PATCH] D89379: [llvm-readobj/libObject] - Allow dumping objects that has a broken SHT_SYMTAB_SHNDX section.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/include/llvm/Object/ELFObjectFile.h:697
                                       const Elf_Shdr *SymTab) const {
+  assert(false);
+  ArrayRef<Elf_Word> ShndxTable;
----------------
Debugging code needs removing?


================
Comment at: llvm/test/tools/llvm-readobj/ELF/ARM/dwarf-cfi.s:1-2
-# RUN: llvm-mc -triple arm-linux -filetype obj -o - %s | llvm-readobj -u - | FileCheck %s
+# RUN: llvm-mc -triple arm-linux -filetype obj %s -o %t
+# RUN: llvm-readobj -u %t | FileCheck %s
 
----------------
Looks like you don't need to do this as part of this change, since this file is otherwise unaffected?


================
Comment at: llvm/test/tools/llvm-readobj/ELF/symbol-shndx.test:258
+
+## Check we are trying to dump symbols even when the number of entries in the
+## SHT_SYMTAB_SHNDX section doesn't match the number of symbols in the symbol table.
----------------



================
Comment at: llvm/test/tools/llvm-readobj/ELF/symbol-shndx.test:265
+
+#       SHNDX-ERR-GNU: warning: '[[FILE]]': SHT_SYMTAB_SHNDX has 3 entries, but the symbol table associated has 2
+# SHNDX-ERR-GNU-EMPTY:
----------------
Presumably not part of this patch, since I can't see the error message being reported anywhere in the changes, but this should be "the associated symbol table".


================
Comment at: llvm/unittests/Object/ELFObjectFileTest.cpp:314
+// SHT_SYMTAB_SHNDX section can't be read properly.
+TEST(ELFObjectFileTest, RelocationTestForVE) {
+  SmallString<0> Storage;
----------------
The test name here doesn't correspond with the behaviour being tested.


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

https://reviews.llvm.org/D89379