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

[Haowei Wu via Phabricator via llvm-commits]

haowei added inline comments.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:229
     size_t DynStrIndex = DynTab.Content.addAddr(DT_STRTAB, 0);
+    DynTab.Content.addValue(DT_STRSZ, DynSym.Size);
     for (const std::string &Lib : Stub.NeededLibs)
----------------
jhenderson wrote:
> This feels like it should be its own independent fix, with corresponding testing etc.
I splitted it into https://reviews.llvm.org/D117058. Please take a look.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:529
+  }
+  // Fallback to use offsets from .dynamic as .dynsym was not found
+  // from the section headers.
----------------
jhenderson wrote:
> mcgrathr wrote:
> > haowei wrote:
> > > 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).
> > No SHT_DYNSYM section could ever be empty, because index 0 is reserved and must contain a stub entry.
> > 
> > I've never seen a linker that produces a .dynamic at all without having a .dynsym.  `clang -nostdlib -shared -o /tmp/foo.so -xassembler /dev/null` with a toolchain that uses current LLD by default produces a .dynsym that has nothing but the null entry.  With some current or past GNU linkers, that degenerate case would still produce either some local STT_SECTION symbols in .dynsym (which really never had a reason to be there), or always export some symbols like `_end` or `_edata` in .dynsym so it was never empty of symbols even when it probably should have been.
> > 
> > So I think basically, if there are section headers at all and this is a real-world ET_DYN file of any plausible normal sort of provenance that development tools were ever really expected to handle, then finding no SHT_DYNSYM at all is more likely to indicate a corrupted file or tool bug than a degenerate ELF file that's actually harmless.  Likewise, if you're looking at PT_DYNAMIC data via phdrs instead, then once you've found the dynamic section then if it doesn't include all of DT_{SYMTAB,SYMENT,STRTAB,STRSZ} then it's more likely to be a corrupt image than a valid degenerate ELF file with no dynamic symbol table.  Even a static PIE that has no dynamic relocs that require referring to a symbol table at all (only simple fixups) will in practice still have a symbol table with the null entry at index 0 and an empty string table but with an address and headers as if it had a length other than zero.
> FWIW, we have a proprietary linker that sticks dynamic data like the dynsym and dynstr inside a combined section, pointed to by dynamic tags contained in a (normal) dynamic section. As such, in our case, we would have a .dynamic, but no .dynstr or .dynsym (in the section header table). We don't use this tool however, and I know our case is unusual, so don't feel a need to handle it necessarily - just highlighting that it is a possible case (just not with, to my knowledge, mainstream linkers).
I am slightly lean a bit more with the fallback logic in diff3 (if .dynsym is not found, fallback to use program header and pointers from .dynamic) instead of  exit early when .dynsym is not found.  I bet there will be other elf tools in the wild that may generate ELF files with not common sections arrangements and it may help to avoid hassles in the future. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116769