|
Description
Zeev Tarantov
2011-03-19 01:13:42 UTC
This is not enough to reproduce the issue, even making the source compile ends up failing with /usr/bin/ld: t.o: version node not found for symbol pci_fill_info@LIBPCI_3.0 /usr/bin/ld: failed to set dynamic section sizes: Bad value collect2: ld returned 1 exit status please provide a complete compilable testcase. Thanks. Created attachment 23740 [details]
sources, object files and two shared objects: one good, one bad (resulted from linking with LTO)
I am sorry but I don't know which parts are relevant. The source is GPL2, I just ran "make SHARED=yes CFLAGS="-fPIC -O2 -flto" lib/libpci.so.3.1.7" and then linked the SO a second time with -Wl,-flto (as if it was in the LDFLAGS) and attached a tarball with the result. You can compare the two (renamed) shared objects, built from the same object files and version script. I'll try to construct a minimal (single file, single symbol) test case. I've constructed a trivial example, the bug did not reproduce in it. I've tried -save-temps and -Wl,--verbose but those did not reveal anything. I do not have a nicer testcase than comment #2. Confirmed. I think what happens is that the symver global asms get partitioned away from the function definitions. With -flto-partition=none it works for me. > grep pci_fill_info libpci.so.3.1.7.ltrans?.s libpci.so.3.1.7.ltrans0.s: .symver pci_fill_info_v30,pci_fill_info@LIBPCI_3.0 libpci.so.3.1.7.ltrans0.s: .symver pci_fill_info_v31,pci_fill_info@@LIBPCI_3.1 libpci.so.3.1.7.ltrans1.s: call pci_fill_info_v31@PLT libpci.so.3.1.7.ltrans1.s: .globl pci_fill_info_v31 libpci.so.3.1.7.ltrans1.s: .type pci_fill_info_v31, @function libpci.so.3.1.7.ltrans1.s:pci_fill_info_v31: libpci.so.3.1.7.ltrans1.s: .size pci_fill_info_v31, .-pci_fill_info_v31 libpci.so.3.1.7.ltrans1.s: .globl pci_fill_info_v30 libpci.so.3.1.7.ltrans1.s: .set pci_fill_info_v30,pci_fill_info_v31 libpci.so.3.1.7.ltrans2.s: call pci_fill_info_v31@PLT so indeed that is what happens (-save-temps appended to the link command produces those intermediate files). We can't really know better (we do not parse asm strings), -flto-partition=none is a workaround. A fix would be to not use toplevel asms, but I'm not sure a different way for the symvers exists (maybe it's possible to do entirely in the linker script ...). Btw, the minimal partition size is quite small (1000), 10 times smaller than our large-unit-insns inline limit and 2.7 times smaller than our large function inline limit. Upping that limit by a factor of 10 magically solves this problem by only using a single partition automagically (it looks to me that it should be similar to large-unit-insns anyway). Well, upping it only workarounds the problem and you loose parallelizm linking small files. I did bit of tunning on this compiling one of spec2k benchmarks (twolf?) to get smallest time with the cpus I had (4 if I remember) Honza (In reply to comment #7) > Well, upping it only workarounds the problem and you loose parallelizm linking > small files. I did bit of tunning on this compiling one of spec2k benchmarks > (twolf?) to get smallest time with the cpus I had (4 if I remember) Well ... I suppose this also heavily depends on your I/O speed. Nevetheless the number _looks_ awfully low compared to those others. > Well ... I suppose this also heavily depends on your I/O speed. Nevetheless
> the number _looks_ awfully low compared to those others.
Well, totally minimizing compilation of small unit is not very worthwhile. It was from time I was
trying to stress whopr to get issues like this one soon.
It does not matter if we build application in 1/100th of second or 1/200th of
second and too much parallelizm could slow down build of many small binaries.
I guess we could shoot for resonable user response. UI guys thinks it is about
1/3rd of second, so I would not mind tuning number up until compilation on not
terribly high end system approximately gets to this time.
I don't think this is however terribly related to large unit sizes and inliner stuff.
Honza
I can confirm the same bug with pciutils 3.2.1 and GCC 4.9.0, although now it's pci_fill_info_v32(). There's a similar issue when compiling libgcc for powerpc64. libgcc/config/rs6000/ibm-ldouble.c has these aliases:
__asm__ (".symver __gcc_qadd,_xlqadd@GCC_3.4\n\t"
".symver __gcc_qsub,_xlqsub@GCC_3.4\n\t"
".symver __gcc_qmul,_xlqmul@GCC_3.4\n\t"
".symver __gcc_qdiv,_xlqdiv@GCC_3.4\n\t"
".symver .__gcc_qadd,._xlqadd@GCC_3.4\n\t"
".symver .__gcc_qsub,._xlqsub@GCC_3.4\n\t"
".symver .__gcc_qmul,._xlqmul@GCC_3.4\n\t"
".symver .__gcc_qdiv,._xlqdiv@GCC_3.4");
Which seem to get dropped if you use -flto.
This issue makes the shared libraries (libstdc++.so etc.) unusable if GCC is bootstrapped with C{XX,}FLAGS_FOR_TARGET="-flto -O3".
Concerning comment #11, if you have toplevel asms you need to disable LTO for that unit, as there is no way to tell for gcc what the asm statement is doing. Perhaps attribute would be better way to do this? What is status of this bug in general? I must admit I thought we have issues long solved here :( (In reply to Jan Hubicka from comment #13) > What is status of this bug in general? I must admit I thought we have issues > long solved here :( It still exists in 8.1. Libstdc++ uses top level asm statement for symbol versioning. Then symbol std::istream::ignore(long)@GLIBCXX_3_4 just disappears, and libstdc++.so can't be used (this symbol becomes *UND*). (In reply to Jan Hubicka from comment #13) > Concerning comment #11, if you have toplevel asms you need to disable LTO > for that unit, as there is no way to tell for gcc what the asm statement is > doing. Perhaps attribute would be better way to do this? Well then this is PR 57703. Should we mark duplicate? :( (In reply to Xi Ruoyao from comment #15) > (In reply to Jan Hubicka from comment #13) > > Concerning comment #11, if you have toplevel asms you need to disable LTO > > for that unit, as there is no way to tell for gcc what the asm statement is > > doing. Perhaps attribute would be better way to do this? > > Well then this is PR 57703. Should we mark duplicate? :( I think the bug is sufficiently different and should have different workarounds. We might want to add a function attribute to allow to specify symver, so like int pci_fill_info_v31(struct pci_dev *d, int flags) __attribute__((alias("pci_fill_info"), symver("@LIBPCI_3.0")); or similar (combine alias and symver as symver_alias?). Or simply recognize @ in alias. (In reply to Richard Biener from comment #16) > We might want to add a function attribute to allow to specify symver, so like > > int pci_fill_info_v31(struct pci_dev *d, int flags) > __attribute__((alias("pci_fill_info"), symver("@LIBPCI_3.0")); > > or similar (combine alias and symver as symver_alias?). Or simply > recognize @ in alias. But a normal alias is int pci_fill_info(struct pci_dev *, int) __attribute__((alias("pci_fill_info_v31"))); The exported name is the function declare name, and the actual name is in alias attribute. I prefer int pci_fill_info(struct pci_dev *, int) __attribute__((symver_alias("@LIBPCI_3.0", "pci_fill_info_v31"))); (In reply to Xi Ruoyao from comment #17) > I prefer > > int pci_fill_info(struct pci_dev *, int) > __attribute__((symver_alias("@LIBPCI_3.0", "pci_fill_info_v31"))); But then what should we do for int pci_fill_info(struct pci_dev *, int) __attribute__((symver_alias("@LIBPCI_3.0", "pci_fill_info_v31"))); int pci_fill_info(struct pci_dev *, int) __attribute__((symver_alias("@LIBPCI_3.1", "pci_fill_info_v32"))); I think the best result would be like FMV, for e.g.
int foo(void) __attribute__((symver("@1.1")))
{
return 0;
}
int foo(void) __attribute__((symver("@@1.2")))
{
return 1;
}
Would produce two symbols "foo.symver.1.1" and "foo.symver.1.2", and
.symver foo.symver.1.1 foo@1.1
.symver foo.symver.1.2 foo@@1.2
And we can also use
int foo(void) __attribute__((symver("@1.0"), alias("foo_old")));
But this seems difficult in C FE, it tends to complain the "redefine" of foo - note that FMV is still only for C++ until now.
(In reply to Xi Ruoyao from comment #19) > I think the best result would be like FMV, for e.g. > > int foo(void) __attribute__((symver("@1.1"))) > { > return 0; > } > > int foo(void) __attribute__((symver("@@1.2"))) > { > return 1; > } > > Would produce two symbols "foo.symver.1.1" and "foo.symver.1.2", and > > .symver foo.symver.1.1 foo@1.1 > .symver foo.symver.1.2 foo@@1.2 > > And we can also use > > int foo(void) __attribute__((symver("@1.0"), alias("foo_old"))); > > But this seems difficult in C FE, it tends to complain the "redefine" of foo > - note that FMV is still only for C++ until now. Sounds reasonable to me. I have some experience with MVC so I will work on that in this stage1. I made up a (highly immature) patch in the days.
This "thing" works with simple source code files but I believe there are many bugs in the patch.
And I suggest to make "weakref" attribute to output ".symver" directive for arguments with "@" in it. For e.g.
static void *old_memcpy (void *, void *, size_t) __attribute__((weakref("memcpy@GLIBC_2.2.5")));
should produce
.symver old_memcpy memcpy@GLIBC_2.2.5
So we can "import" old functions.
Created attachment 44126 [details]
Patch to add symver attribute, highly experimental, C++ only
(In reply to Xi Ruoyao from comment #22) > Created attachment 44126 [details] > Patch to add symver attribute, highly experimental, C++ only That's great you did the prototype. I'll take a look. (In reply to Richard Biener from comment #16) > (In reply to Xi Ruoyao from comment #15) > > (In reply to Jan Hubicka from comment #13) > > > Concerning comment #11, if you have toplevel asms you need to disable LTO > > > for that unit, as there is no way to tell for gcc what the asm statement is > > > doing. Perhaps attribute would be better way to do this? > > > > Well then this is PR 57703. Should we mark duplicate? :( > > I think the bug is sufficiently different and should have different > workarounds. > Still worth putting under "See Also" at least... This attribute should also apply to objects. /subscribe I've tripped over this problem myself in the context of the new password-hashing library, libxcrypt (see https://github.com/besser82/libxcrypt/issues/24 ) Our symbol-versioning techniques are lifted from glibc (see https://github.com/besser82/libxcrypt/blob/aae4c1baea534d2e4c9dfe2faf42ee0c5f7a6f22/crypt-port.h#L122 ). Function attributes seem like a good replacement to me, although I'd ask that people think about how a library author might write macros that will seamlessly fall back to the older technique. Assigning to Honza at will require usage of transparent aliases, so some extra work will be needed. It seems that using symbol aliases (via .symver) in conjunction with LTO and a version script which has a local: * clause causes the LTO plugin to assume that the aliased function definitions are not (externally) referenced, so GCC will elide them. The workaround for now appears to be to put __attribute__ ((externally_visible)) on the aliased functions, so that GCC assumes that there is a reference to them that it cannot see. -flto-partition=none may also be needed. I'm sure -flto-parition=none is needed for workaround, now. It keeps .symver directive and the function definition in the same assembly file. Unfortunately for large codebase -flto-partition leads to OOM. Pick up this again...
Now __asm__(".symver a, b@@ver") is used for two different purposes:
1. Mark a function/object *definition* as the specified version.
2. "Import" a symbol (function/object) with the specified version.
If we make up two attributes for those two purposes, life will be easier. For 1 we can emit ".symver" directive when the definition is emited. For 2 we can borrow the logic from .weakref and do minimal change.
*** Bug 91186 has been marked as a duplicate of this bug. *** >I'm sure -flto-parition=none is needed for workaround, now.
This does not always work. I think if the symver was done in an archive.
Any progress on that issue? Just hit that issue trying to build NetworkManager https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/278 > Any progress on that issue?
> Just hit that issue trying to build NetworkManager
>
> https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/278
I am working on a patch for symver attribute, hope to have it done this
week.
Honza
Thanks for update. Please let me know when you will have working version of your patch. I have ready to use gcc build in which after about two hours (my gcc compile time) I would be able to to try to help you test that patch :) Just point to exact patch in git or attach the patch here. *** Bug 66825 has been marked as a duplicate of this bug. *** Created attachment 47268 [details] symverp Hi, I am attaching prototype patch. It implements "symver" attribute which can be attached to a definition such as: hubicka@lomikamen-jh:/aux/hubicka/trunk5/build4/gcc$ cat t.c __attribute__ ((symver("bar@2.2"))) int t() { } hubicka@lomikamen-jh:/aux/hubicka/trunk5/build4/gcc$ ./xgcc -B ./ -O2 t.c -S hubicka@lomikamen-jh:/aux/hubicka/trunk5/build4/gcc$ cat t.s .file "t.c" .text .p2align 4 .globl t .type t, @function t: .LFB0: .cfi_startproc ret .cfi_endproc .LFE0: .size t, .-t .symver t, bar@2.2 .ident "GCC: (GNU) 10.0.0 20191114 (experimental)" .section .note.GNU-stack,"",@progbits Internally it defines a new kind of alias (symver alias) with assembler name "bar@2.2" so LTO symbol table match ELF symbol table for the unit: hubicka@lomikamen-jh:/aux/hubicka/trunk5/build4/gcc$ nm t.o 0000000000000000 T bar@2.2 0000000000000000 T t hubicka@lomikamen-jh:/aux/hubicka/trunk5/build4/gcc$ ./xgcc -B ./ -O2 t.c -c -flto hubicka@lomikamen-jh:/aux/hubicka/trunk5/build4/gcc$ gcc-nm t.o 00000000 T bar@2.2 00000000 T t Internally in GCC the symver is handled just as normal alias only output differntly. I think GCC, just like static linker, does not really need to understand the versions because there are no direct ways to bind to those symbols. I wonder if someone sees limitations to this design (in particular attaching the symver directly to the definition). In PR66825 Carlos proposed "version_def" attribute which can be implemented too (I suppose all I need is to concatenate the strings into the actual name). I wonder what would be preferred and easier for users? Also Carlos proposes "version_ref" that can be used for external definitions. I am not quite sure how to do that with existing GAS and LTO since different translation units can reffer to different versions. But my understnading is that for LTO it is critical to implement the def part. Hopefully I can turn this into a patch proposal tomorrow to meet the stage1 deadline, so comments are welcome especially if they come really soon :) Honza (In reply to Jan Hubicka from comment #38) > Internally in GCC the symver is handled just as normal alias only output > differntly. I think GCC, just like static linker, does not really need > to understand the versions because there are no direct ways to bind to > those symbols. > > I wonder if someone sees limitations to this design (in particular > attaching the symver directly to the definition). I think this design is correct. "symver" attribute is introduced to "bind" a symver to a definition. If they can be seperated LTO will do stupid things. I prefer "symver" than "version_def". I posted initial patch here https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01334.html (In reply to Jan Hubicka from comment #40) > I posted initial patch here > https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01334.html I applied it into gcc-9.2.0 and it works. But, unfortunately, the problem with LTO and symver is not fixed. A simple testcase fails: $ cat foo.c __attribute__ ((__symver__ ("foo@VERS_1"))) int foo_v1 (void) { return 1; } __attribute__ ((__symver__ ("foo@@VERS_2"))) int foo_v2 (void) { return 2; } $ cat version.map VERS_1 { global: foo; local: *; }; VERS_2 { } VERS_1; $ gcc foo.c -flto -Wl,--version-script -Wl,version.map -shared -Wl,--as-needed --save-temp $ cat foo.res 1 foo.o 4 211 93dd820662070d19 PREVAILING_DEF_IRONLY foo_v1 213 93dd820662070d19 PREVAILING_DEF_IRONLY foo@VERS_1 222 93dd820662070d19 PREVAILING_DEF_IRONLY foo_v2 224 93dd820662070d19 PREVAILING_DEF_IRONLY foo@@VERS_2 $ grep symver cc*.s || echo "no symver" no symver I just sent a patch making symver attribute "really" useful for LTO. https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01162.html It looks there is now some support for a symver function attribute. But it only accepts the single and double @ forms. This makes things a little awkward when using a symbol foo itself for foo@@DEFAULT_VERSION. It causes the (non-versioned) foo symbol to still appear in the object, which doesn't work very nicely when combined with linker version scripts. For elfutils I had to workaround that by adding foo also to a non-exported version section, so that the assembler and linker didn't both try to create a default version for foo: https://sourceware.org/pipermail/elfutils-devel/2020q2/002609.html It works, but feels like a hack. Please also support the @@@ symver variant. Thanks, I am happy we now have real-world use of symver attribute. I have WIP patch for better control over the symbol visibility, but I have run into problems with gas limitations which was fixed by HJ about two weeks ago. I will try to update the patch and aim for backporting to gcc 10.2. Note that the hack in comment 43 doesn't really work with elfutils since the .symver attribute doesn't work exactly like the assembly construct with @@@. The @@@ symver variant see https://sourceware.org/binutils/docs/as/Symver.html The third usage of the .symver directive is: .symver name, name2@@@nodename When name is not defined within the file being assembled, it is treated as name2@nodename. When name is defined within the file being assembled, the symbol name, name, will be changed to name2@@nodename. Which means it is renamed, so that it doesn't clash when used in a version map file. LTO was fixed at r10-5568. I think we can close this as fixed for GCC 10. (In reply to Mark Wielaard from comment #45) > Note that the hack in comment 43 doesn't really work with elfutils since the > .symver attribute doesn't work exactly like the assembly construct with @@@. > The @@@ symver variant see > https://sourceware.org/binutils/docs/as/Symver.html > > The third usage of the .symver directive is: > > .symver name, name2@@@nodename > > When name is not defined within the file being assembled, it is treated as > name2@nodename. When name is defined within the file being assembled, the > symbol name, name, will be changed to name2@@nodename. > > Which means it is renamed, so that it doesn't clash when used in a version > map file. Reopen for this part of the discussion since I misunderstood it at first. |