[PATCH] D131309: [ELF] Add ability to get a symbol by name from the hash table section

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/unittests/Object/ELFObjectFileTest.cpp:1106
+
+// Test for getSymbolFromHashTable: check that it fails with an invalid sh_link.
+TEST(ELFObjectFileTest, SymbolFromHashTableTestLink) {
----------------
jhenderson wrote:
> Probably change this comment to "check that it fails when not referencing a dynamic symbol table" or something to that effect. "invalid sh_link" sounds like the value is outside the range of the section indexes, which is what the next case does.
You've updated the wrong comment in response to my suggestion here. This test case is the case where the sh_link is valid, but points at a .dynstr. The one you updated is the one that has a sh_link that is not within the section count range.


================
Comment at: llvm/unittests/Object/ELFObjectFileTest.cpp:1164-1165
+
+// Test for getSymbolFromHashTable: check that it fails given invalid section
+// header offsets.
+TEST(ELFObjectFileTest, SymbolFromHashTableTestHeaderOffset) {
----------------
jhenderson wrote:
> 
Not addressed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131309