[PATCH] D116769: [ifs] Allow llvm-ifs to generate text stub from elf stub

[Haowei Wu via Phabricator via llvm-commits]

haowei marked an inline comment as done.
haowei added inline comments.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:529
+  }
+  // Fallback to use offsets from .dynamic as .dynsym was not found
+  // from the section headers.
----------------
haowei wrote:
> mcgrathr wrote:
> > I would actually fall back here only if there are no section headers at all.  If there are any section headers at all but no SHT_DYNSYM section, then don't look further.  i.e. return failure after the `for` loop above inside the `if`.
> > 
> If it fail when .dynsym is absent, test https://github.com/llvm/llvm-project/blob/ebd8eee62a431a6744c3f187fcda58e5dea08499/llvm/test/tools/llvm-ifs/binary-read-add-soname.test will fail (there are 7 similar tests) as there is no .dynsym at all, .dynstr (though empty) is pointed by .dynamic . I believe either fallback strategy won't cause any issues in the field.
> 
> If an ELF has no symbols (which is unlikely), should it have no .dynsym section or should it have an empty .dynsym?
I uploaded a patch using your suggested fallback logic and modified affected tests (and deleted one as it not achievable).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116769