Created attachment 34617 [details]
Simple testcase. Use "make" to reproduce.
While trying to construct a testcase for a 4.8 to 4.9 change of LTO linking behaviour, I stumbled over this (just re-tested with r220248):
g++-5 -flto -fuse-linker-plugin -O2 -Wall -Wextra -c file1.cpp
g++-5 -flto -fuse-linker-plugin -O2 -Wall -Wextra -c file2.cpp
gcc-5 -flto -fuse-linker-plugin -Wl,-r -nostdlib -o lib1.lib file1.o
gcc-5 -flto -fuse-linker-plugin -Wl,-r -nostdlib -o lib2.lib file2.o
g++-5 -flto -fuse-linker-plugin -O2 -o testexe lib1.lib lib2.lib
lib2.lib:(.rodata+0x0): multiple definition of `typeinfo name for CDialogBase'
lib1.lib:(.rodata+0x0): first defined here
collect2: error: ld returned 1 exit status
# g++-5 -v
Using built-in specs.
Configured with: ../configure --prefix=/usr --infodir=/usr/share/info --mandir=/usr/share/man --libdir=/usr/lib64 --libexecdir=/usr/lib64 --enable-languages=c,c++,fortran --with-gxx-include-dir=/usr/include/c++/5 --enable-ssp --disable-libssp --disable-libvtv --disable-plugin --with-bugurl=http://bugs.opensuse.org/ --with-pkgversion='SUSE Linux' --disable-libgcj --with-slibdir=/lib64 --with-system-zlib --enable-__cxa_atexit --enable-libstdcxx-allocator=new --disable-libstdcxx-pch --with-default-libstdcxx-abi=c++98 --enable-version-specific-runtime-libs --enable-linker-build-id --enable-linux-futex --program-suffix=-5 --without-system-libunwind --enable-multilib --with-arch-32=i586 --with-tune=generic --build=x86_64-suse-linux --host=x86_64-suse-linux
Thread model: posix
gcc version 5.0.0 20150129 (experimental) (SUSE Linux)
gcc-4.8.3 and 4.9.2 compile and link the same code just fine.
Hmm, interesting, I did not even think we support this ;)
I am not sure how much it makes sense to do incremntal linking like this with LTO because this limits LTO to incremental link only (it would perhaps make sense to produce LTO file after merigng in this case).
What changed since GCC 4.9 is that visibility pass is now more aggressive on bringing comdat locals. In resolution file we have:
$ more *.res
212 f6705eca5fa850e3 PREVAILING_DEF _ZN11CDialogBase11GetDocumentEv
217 f6705eca5fa850e3 PREVAILING_DEF dialog2
204 f6705eca5fa850e3 PREVAILING_DEF _ZTV11CDialogBase
232 f6705eca5fa850e3 PREVAILING_DEF _ZTS11CDialogBase
248 f6705eca5fa850e3 PREVAILING_DEF _ZTI11CDialogBase
259 f6705eca5fa850e3 UNDEF _ZTVN10__cxxabiv117__class_type_infoE
which makes us to believe that all the symbols are previaling ones for current DSO. The trick it that incrementally this is not quite true. I suppose only way to make this work reliably is to modify:
resolution_to_local_definition_p (enum ld_plugin_symbol_resolution resolution)
return (resolution == LDPR_PREVAILING_DEF
|| resolution == LDPR_PREVAILING_DEF_IRONLY_EXP
|| resolution == LDPR_PREVAILING_DEF_IRONLY);
to exclude LDPR_PREVAILING_DEF and similarly in ipa.c. This way we lose some optimization in non-incremental linking where we know that we are the only LTO unit in proram.
So perhaps doing this conditionally on -r? We will probably makes it funnier to produce testcases. Or we don't really care as it normally affects only cases where non-LTO and LTO objects are mixed.
Yes, -r should be honored by the appropriate resolution info (not _IR_ONLY). Thus you have to treat that conservatively.
Otherwise you can create a testcase that fails with the final link and the
other strong definition in a shared library.
Well, the problem is that -r is the only case where we get LDPR_PREVAILING_DEF or IRONLY and the resulting symbol may be removed from the unit later.
We would need a new LDPR_PREVAILING_DEF_FOR_NOW or something like that to represent the incremental nature of linking here, or we need a global flag in GCC saying if incremental linking is supposed tohappen or not.
(just to explain bit more - the main difference between static and dynamic linking here is that dynamic linking never remove any definition and thus you can dissolve comdat groups)
I suppose proper fix is to make flag_incremental_linking and turn -r from Driver only. Then we could revisit individual optimizations that do rely on the fact that static linking will not be re-done.
Created attachment 35100 [details]
this is a patch that adds necessary checks into resolution code. Basically if static linking is anticipated, we can not derive much of useful info from PREVAILING_DEF/PREVAILING_DEF_IRONLY_EXP and must anticipate that static linking may change these and drop comdat groups.
THe catch is that someone needs to pass -r to lto1 that is not currently happening (I verified that if I hack opts.c to set incremental_linking to true the testcase works).
With -r option, I can imagine that lto-wrapper can do it, because we get:
COLLECT_GCC_OPTIONS='-c' '-fmath-errno' '-fsigned-zeros' '-ftrapping-math' '-fno-trapv' '-fno-openmp' '-fno-openacc' '-mtune=generic' '-march=x86-64' '-O2' '-B' './' '-O2' '-r' '-v' '-mtune=generic' '-march=x86-64' '-fltrans-output-list=/tmp/cc3o3UIr.ltrans.out' '-fwpa' '-fresolution=/tmp/cchYXuvF.res
I donot know hwere -r gets dropped though. But the testcase uses -Wl,-r that may need LTO plugin extension?
Lowering to P2 on Honza's request.
I filled in binutils PR for extension of plugin API https://sourceware.org/bugzilla/show_bug.cgi?id=18178
GCC 5.1 has been released.
GCC 5.2 is being released, adjusting target milestone to 5.3.
This is another bug related to incremental linking and LTO. I will redirect them all to PR67548. The testcase now works on mainline GCC in a sense that the duplicate is gone and we get missing typeinfo:
lib1.lib:<artificial>:typeinfo for CDialogBase: error: undefined reference to 'vtable for __cxxabiv1::__class_type_info'
/usr/bin/ld: the vtable symbol may be undefined because the class is missing its key function
collect2: error: ld returned 1 exit status
that seems intended.
*** This bug has been marked as a duplicate of bug 67548 ***