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

[Joseph Huber via Phabricator via llvm-commits]

jhuber6 marked 16 inline comments as done.
jhuber6 added inline comments.


================
Comment at: llvm/include/llvm/Object/ELF.h:505-516
+  if (Sec.sh_type != ELF::SHT_HASH)
+    return createError("invalid sh_type for hash table, expected SHT_HASH");
+  Expected<Elf_Shdr_Range> SectionsOrError = sections();
+  if (!SectionsOrError)
+    return SectionsOrError.takeError();
+
+  auto SymTabOrErr = object::getSection<ELFT>(*SectionsOrError, Sec.sh_link);
----------------
jhenderson wrote:
> All these checks still need test cases.
Checking these errors explicitly is excessive. This isn't any new logic so it should already be covered by existing tests. I added one for calling this with an incorrect section but the other error paths are already covered as far as I'm aware.


================
Comment at: llvm/unittests/Object/ELFObjectFileTest.cpp:867
+
+  auto StrTabOrErr = Elf.getSection(/* SHT_STRTAB */ 3);
+  ASSERT_THAT_EXPECTED(StrTabOrErr, Succeeded());
----------------
jhenderson wrote:
> There's a lot of unhelpful `auto` in this test, which I don't think really conforms to LLVM's style guide.
ELF uses a lot of type-aliases depending on the input. It's much easier to just use `auto` rather than importing the names and using `Expected<typename ELF64LE::xyz>` for everything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131309