[PATCH] D86142: [LLD] Search archives for non-tentative defintions.

Sean Fertile via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 24 12:43:05 PDT 2020


sfertile added a comment.

I've spent some time reading through the linked thread. Thanks.  There is a lot to reply to so I hope I don't miss anything.

> Before we jump directly into a fix, can you create a FORTRAN example where this special common symbol handling is reasonably used?

The Engineering and Scientific Subroutine Library (ESSL) <https://www.ibm.com/support/knowledgecenter/SSFHY8_6.1/navigation/welcome.html> uses common block and block data. One case that shows this problem is when using `DRCFTD` function, which calls an internal routine that uses a table of trig values. If we link the library into a shared object we get the expected behavior: the file with the block data overrides all the common blocks and the global symbol is properly initialized in the shared object. If we package the library into an archive and link against that then we end up missing the strong definition since an earlier object provides the tentative definition and the global symbol is zero initialized, producing broken runtime behavior.

> Note that regular overriding common has an analog: global overriding weak.

I agree that the special handling for commons is ugly, and I really don't like the asymmetry it produces in relation to weak symbols any more then you do. If we approached the problem strictly from an ELF semantics perspective, then the consistency of golds behavior is arguably desirable. From a usability perspective though, our archive member handling breaks a common idiom in an old but still used dialect of a rather important language.  The interaction of common symbols and archive member inclusion  unfortunately breaks  the FORTRAN language semantics. My understanding is there are lots of modern(ish) programs (not necessarily written in fortran) that link against old FORTRAN libraries and so IMO the trade off in consistency is worth it for the usability.

I don't think using `--whole-archives` to pull in the entire archive is a reasonable fix. With the libraries I was using, linking in whole archives for all the FORTRAN libraries failed due to multiply defined symbols. In the cases where a library is structured in a way that we can use the whole archive option it will lead to binary bloat in any program that needs to link against an old Fortran library.

In D86142#2225679 <https://reviews.llvm.org/D86142#2225679>, @psmith wrote:

> Given that BFD supports this behaviour but Gold and LLD do not I suspect that the risk of breaking existing programs is small, especially as LLD tends to be used with more recent programs where -fno-common is the default. However I think we should be cautious and perhaps enable under a command line option, especially as it looks like this is a convention rather than part of a specification. If anyone has better references, then I'm happy to retract.

I agree the new behavior needs an option to enable it and disable it. Initially I considered having the Fortran driver pass the option to LLD, but after discussing it offline with @rzurob I think it might be best to enable the behavior by default. The consumer of these libraries aren't necessarily Fortran programs, and as you mentioned its likely to be lower risk because we are adopting a behavior already implemented in ld.bfd.

In D86142#2225447 <https://reviews.llvm.org/D86142#2225447>, @MaskRay wrote:

> If we end up patching LLD, we should avoid making Symbol::resolveLazy complex.
> We need to think hard doing symbol resolution in batch, in
> ArchiveFile/LazyObjFile::parse. For the tests, all the code is insignificant.
> My example at https://sourceware.org/pipermail/binutils/2020-August/112878.html
> is probably sufficient. As you can see, bitcode LazyObjFile requires special handling.
> I need to think about the interaction and I may be able to send a patch.

That is why I posted a WIP patch early. So we could figure out what the best direction is. I'm open to any suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86142



More information about the llvm-commits mailing list