[PATCH] D92923: [llvm-readelf/obj] - Add support of multiple SHT_SYMTAB_SHNDX sections.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/symtab-shndx.test:199
+## Check we report a warning when multiple SHT_SYMTAB_SHNDX sections are linked to a symbol table.
+## In this case 2, sections are linked to the dynamic symbol table. Check it doesn't affect
+## anything, because the SHT_SYMTAB_SHNDX section specified by the DT_SYMTAB_SHNDX tag is still used.
----------------
jhenderson wrote:
> You didn't follow my fix properly here :)
Oops, sorry. Fixed.


================
Comment at: llvm/unittests/Object/ELFTest.cpp:66-69
+      if (Expected<uint8_t> ValOrErr = R[I])
+        EXPECT_EQ(*ValOrErr, I);
+      else
+        ADD_FAILURE() << toString(ValOrErr.takeError());
----------------
jhenderson wrote:
> Could this be:
> 
> ```
> ASSERT_THAT_EXPECTED(ValOrErr, Succeeded());
> EXPECT_EQ(*ValOrErr, I);
> ```
> 
> Or something to that effect? It seems simpler, and if the good index-value cases don't work, there doesn't seem to be much point in worrying about the rest of the test case running, so asserting is fine.
Yes, it is possible to write in this way here. Done.


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

https://reviews.llvm.org/D92923