[PATCH] D55629: [elfabi] Add support for reading DT_SONAME from binaries

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Basically looks fine to me, with one outstanding issue, namely that I feel like the address to offset code is a reinvention of something either in llvm-objcopy or llvm-objdump somewhere, but I don't remember where unfortunately. I'd suggest checking how llvm-objdump (or possibly llvm-readobj) looks up information relating to dynamic symbols etc.



================
Comment at: llvm/test/tools/llvm-elfabi/binary-read-replace-soname.test:46-58
+# ORIGINAL:      --- !tapi-tbe
+# ORIGINAL-NEXT: TbeVersion: {{[1-9]\d*\.(0|([1-9]\d*))}}
+# ORIGINAL-NEXT: SoName: somelib.so
+# ORIGINAL-NEXT: Arch: x86_64
+# ORIGINAL-NEXT: Symbols: {}
+# ORIGINAL-NEXT: ...
+
----------------
You could probably simplify both of these to just check the SoName field. Same goes for other tests.

Also add the {{$}} here and above.


================
Comment at: llvm/tools/llvm-elfabi/ELFObjHandler.cpp:156
 Expected<std::unique_ptr<ELFStub>>
-buildStub(const ELFObjectFile<ELFT> &ElfObj) {
+static buildStub(const ELFObjectFile<ELFT> &ElfObj) {
+  using Elf_Dyn_Range = typename ELFT::DynRange;
----------------
Nit: Shouldn't this static be before the return value?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55629