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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I was going to start reviewing this, and made a couple of initial nitpick comments (there will be more if I come back to this), but if the patch series gets abandoned due to @MaskRay's opposition, there's little point in me wasting the time on the review, so I'll hold off for a while.



================
Comment at: llvm/include/llvm/Object/ELF.h:536
 
-  const Elf_Hash *HashTab =
-      reinterpret_cast<const Elf_Hash *>(base() + Sec.sh_offset);
+  if (Sec.sh_type == ELF::SHT_HASH) {
+    const Elf_Hash *HashTab =
----------------
>From a pure readability perspective, it might be worth breaking the two "modes" of this function (SHT_GNU_HASH vs SHT_HASH) into helper functions. This function is otherwise getting quite long and half of it is irrelevant to any given usage.


================
Comment at: llvm/unittests/Object/ELFObjectFileTest.cpp:945-946
       SymOrErr,
-      FailedWithMessage("section [index 2] has a sh_offset (0x270f) + sh_size (0x18) that is greater than the file size (0x218)"));
+      FailedWithMessage("section [index 2] has a sh_offset (0x270f) + sh_size "
+                        "(0x18) that is greater than the file size (0x218)"));
 }
----------------
Nit: This should be fixed in the prerequisite patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132743