[PATCH] D93209: [libObject, llvm-readobj] - Reimplement `ELFFile<ELFT>::getEntry`.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/reloc-symbol-with-versioning.test:2-3
 # RUN: yaml2obj %s -o %t.o
-# RUN: llvm-readobj --demangle -r %t.o | FileCheck %s --check-prefix LLVM
-# RUN: llvm-readelf --demangle -r %t.o | FileCheck %s --check-prefix GNU
+# RUN: llvm-readobj --demangle -r %t.o 2>&1 | FileCheck %s --check-prefix LLVM
+# RUN: llvm-readelf --demangle -r %t.o 2>&1 | FileCheck %s --check-prefix GNU
 
----------------
jhenderson wrote:
> Why did this change?
Left from debugging. I've added `--implicit-check-not=warning:` just in case,
as this test was updated to stop triggering the fixed issue.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/relocations.test:535-538
+      - Type:   R_X86_64_NONE
+        Symbol: 0x0
+      - Type:   R_X86_64_NONE
+        Symbol: 0x1
----------------
jhenderson wrote:
> I'm not sure you really need these other relocations - just the one with the invalid symbol index would be enough.
I think we need at least 2 to test the new "unable to read an entry with index X" part properly.


================
Comment at: llvm/unittests/Object/ELFObjectFileTest.cpp:353-354
+  Expected<const typename ELF64LE::Shdr *> SecOrErr = Elf.getSection(1);
+  if (!SecOrErr)
+    GTEST_FAIL() << toString(SecOrErr.takeError());
+
----------------
jhenderson wrote:
> This could be `ASSERT_THAT_EXPECTED(SecOrErr, Succeeded());` right?
I think so. But this place is gone.


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

https://reviews.llvm.org/D93209