* --as-needed change wrt undefined weak symbols @ 2013-01-14 3:58 Alan Modra 2013-01-14 15:31 ` H.J. Lu 0 siblings, 1 reply; 17+ messages in thread From: Alan Modra @ 2013-01-14 3:58 UTC (permalink / raw) To: binutils; +Cc: Pierre Ossman Does anyone have objections or serious reservations on changing --as-needed to require a strong reference? I'm in favour of excluding weak references as that more closely follows the rules for ld extracting objects from archives. Now that http://sourceware.org/ml/binutils/2013-01/msg00165.html and http://sourceware.org/ml/binutils/2013-01/msg00186.html have gone in I think the following is all we need. bfd/ PR ld/12549 elflink.c (elf_link_add_object_symbols): Exclude weak refs when considering whether an --as-needed library is needed. ld/ * ld.texinfo (--as-needed): Update. ld/testsuite/ * ld-elf/pr14862.out: Expect no output. Index: bfd/elflink.c =================================================================== RCS file: /cvs/src/src/bfd/elflink.c,v retrieving revision 1.463 diff -u -p -r1.463 elflink.c --- bfd/elflink.c 13 Jan 2013 12:32:10 -0000 1.463 +++ bfd/elflink.c 14 Jan 2013 03:37:56 -0000 @@ -4480,8 +4480,8 @@ error_free_dyn: if (!add_needed && definition && ((dynsym - && h->ref_regular) - || (h->ref_dynamic + && h->ref_regular_nonweak) + || (h->ref_dynamic_nonweak && (elf_dyn_lib_class (abfd) & DYN_AS_NEEDED) != 0 && !on_needed_list (elf_dt_name (abfd), htab->needed)))) { Index: ld/ld.texinfo =================================================================== RCS file: /cvs/src/src/ld/ld.texinfo,v retrieving revision 1.290 diff -u -p -r1.290 ld.texinfo --- ld/ld.texinfo 7 Jan 2013 12:11:11 -0000 1.290 +++ ld/ld.texinfo 14 Jan 2013 03:37:57 -0000 @@ -1150,11 +1150,14 @@ on the command line after the @option{-- the linker will add a DT_NEEDED tag for each dynamic library mentioned on the command line, regardless of whether the library is actually needed or not. @option{--as-needed} causes a DT_NEEDED tag to only be -emitted for a library that satisfies an undefined symbol reference -from a regular object file or, if the library is not found in the -DT_NEEDED lists of other libraries linked up to that point, an -undefined symbol reference from another dynamic library. -@option{--no-as-needed} restores the default behaviour. +emitted for a library that @emph{at that point in the link} satisfies a +non-weak undefined symbol reference from a regular object file or, if +the library is not found in the DT_NEEDED lists of other libraries, a +non-weak undefined symbol reference from another dynamic library. +Object files or libraries appearing on the command line @emph{after} +the library in question do not affect whether the library is seen as +needed. This is similar to the rules for extraction of object files +from archives. @option{--no-as-needed} restores the default behaviour. @kindex --add-needed @kindex --no-add-needed Index: ld/testsuite/ld-elf/pr14862.out =================================================================== RCS file: /cvs/src/src/ld/testsuite/ld-elf/pr14862.out,v retrieving revision 1.1 diff -u -p -r1.1 pr14862.out --- ld/testsuite/ld-elf/pr14862.out 20 Nov 2012 22:17:27 -0000 1.1 +++ ld/testsuite/ld-elf/pr14862.out 14 Jan 2013 03:37:57 -0000 @@ -1 +0,0 @@ -OK -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: --as-needed change wrt undefined weak symbols 2013-01-14 3:58 --as-needed change wrt undefined weak symbols Alan Modra @ 2013-01-14 15:31 ` H.J. Lu 2013-01-15 2:23 ` Alan Modra 0 siblings, 1 reply; 17+ messages in thread From: H.J. Lu @ 2013-01-14 15:31 UTC (permalink / raw) To: binutils, Pierre Ossman On Sun, Jan 13, 2013 at 7:58 PM, Alan Modra <amodra@gmail.com> wrote: > Does anyone have objections or serious reservations on changing > --as-needed to require a strong reference? I'm in favour of excluding > weak references as that more closely follows the rules for ld > extracting objects from archives. > > Now that http://sourceware.org/ml/binutils/2013-01/msg00165.html and > http://sourceware.org/ml/binutils/2013-01/msg00186.html have gone in I > think the following is all we need. > > bfd/ > PR ld/12549 > elflink.c (elf_link_add_object_symbols): Exclude weak refs when > considering whether an --as-needed library is needed. > ld/ > * ld.texinfo (--as-needed): Update. > ld/testsuite/ > * ld-elf/pr14862.out: Expect no output. > Given then the behavior of pr14862 is changed, I don't think it is a good idea. -- H.J. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: --as-needed change wrt undefined weak symbols 2013-01-14 15:31 ` H.J. Lu @ 2013-01-15 2:23 ` Alan Modra 2013-01-15 3:30 ` H.J. Lu 0 siblings, 1 reply; 17+ messages in thread From: Alan Modra @ 2013-01-15 2:23 UTC (permalink / raw) To: H.J. Lu; +Cc: binutils, Pierre Ossman On Mon, Jan 14, 2013 at 07:30:54AM -0800, H.J. Lu wrote: > Given then the behavior of pr14862 is changed, > I don't think it is a good idea. You added a testcase in November last year that (possibly accidentally) depends on the current --as-needed behaviour for weak references. Now you claim that testcase as a reason to not change --as-needed. How is that a reasonable objection? -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: --as-needed change wrt undefined weak symbols 2013-01-15 2:23 ` Alan Modra @ 2013-01-15 3:30 ` H.J. Lu 2013-01-15 5:23 ` Alan Modra 0 siblings, 1 reply; 17+ messages in thread From: H.J. Lu @ 2013-01-15 3:30 UTC (permalink / raw) To: Binutils, Pierre Ossman On Mon, Jan 14, 2013 at 6:23 PM, Alan Modra <amodra@gmail.com> wrote: > On Mon, Jan 14, 2013 at 07:30:54AM -0800, H.J. Lu wrote: >> Given then the behavior of pr14862 is changed, >> I don't think it is a good idea. > > You added a testcase in November last year that (possibly > accidentally) depends on the current --as-needed behaviour for weak > references. Now you claim that testcase as a reason to not change > --as-needed. How is that a reasonable objection? > It shows that this patch will change the behavior of some programs. Adding DT_NEEDED is one thing. Change program behavior is another. I don't think we should do it by default. -- H.J. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: --as-needed change wrt undefined weak symbols 2013-01-15 3:30 ` H.J. Lu @ 2013-01-15 5:23 ` Alan Modra 2013-01-15 15:04 ` H.J. Lu 0 siblings, 1 reply; 17+ messages in thread From: Alan Modra @ 2013-01-15 5:23 UTC (permalink / raw) To: H.J. Lu; +Cc: Binutils, Pierre Ossman On Mon, Jan 14, 2013 at 07:30:24PM -0800, H.J. Lu wrote: > On Mon, Jan 14, 2013 at 6:23 PM, Alan Modra <amodra@gmail.com> wrote: > > On Mon, Jan 14, 2013 at 07:30:54AM -0800, H.J. Lu wrote: > >> Given then the behavior of pr14862 is changed, > >> I don't think it is a good idea. > > > > You added a testcase in November last year that (possibly > > accidentally) depends on the current --as-needed behaviour for weak > > references. Now you claim that testcase as a reason to not change > > --as-needed. How is that a reasonable objection? > > > > It shows that this patch will change the behavior of some > programs. Adding DT_NEEDED is one thing. Change > program behavior is another. I don't think we should do it > by default. Of course this patch potentially changes program behaviour. I'd argue that people using undefined weak symbols are prepared for and indeed expect such changes in behaviour. The PR12549 reporter and another commenter quite reasonably call ld --as-needed buggy in that we get a library added to DT_NEEDED only to satify an undefined weak symbol. If the same program were linked against archive libraries supplying the same functionality you'd find the undefined weak symbols would stay undefined. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: --as-needed change wrt undefined weak symbols 2013-01-15 5:23 ` Alan Modra @ 2013-01-15 15:04 ` H.J. Lu 2013-03-18 2:41 ` Alan Modra 0 siblings, 1 reply; 17+ messages in thread From: H.J. Lu @ 2013-01-15 15:04 UTC (permalink / raw) To: Binutils, Pierre Ossman On Mon, Jan 14, 2013 at 9:23 PM, Alan Modra <amodra@gmail.com> wrote: > On Mon, Jan 14, 2013 at 07:30:24PM -0800, H.J. Lu wrote: >> On Mon, Jan 14, 2013 at 6:23 PM, Alan Modra <amodra@gmail.com> wrote: >> > On Mon, Jan 14, 2013 at 07:30:54AM -0800, H.J. Lu wrote: >> >> Given then the behavior of pr14862 is changed, >> >> I don't think it is a good idea. >> > >> > You added a testcase in November last year that (possibly >> > accidentally) depends on the current --as-needed behaviour for weak >> > references. Now you claim that testcase as a reason to not change >> > --as-needed. How is that a reasonable objection? >> > >> >> It shows that this patch will change the behavior of some >> programs. Adding DT_NEEDED is one thing. Change >> program behavior is another. I don't think we should do it >> by default. > > Of course this patch potentially changes program behaviour. I'd > argue that people using undefined weak symbols are prepared for and > indeed expect such changes in behaviour. The PR12549 reporter and The input file works with undefined weak symbol. The issue is the expected program behavior for a given command line. If libfoo.so provides a definition for weak undefined symbol, it is used as of today. > another commenter quite reasonably call ld --as-needed buggy in that Sure, add a new option. But we shouldn't change the existing option. > we get a library added to DT_NEEDED only to satify an undefined weak > symbol. If the same program were linked against archive libraries > supplying the same functionality you'd find the undefined weak symbols > would stay undefined. > People know linker treats archive and shared object differently. -- H.J. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: --as-needed change wrt undefined weak symbols 2013-01-15 15:04 ` H.J. Lu @ 2013-03-18 2:41 ` Alan Modra 2013-03-18 7:55 ` [GOLD] " Alan Modra ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Alan Modra @ 2013-03-18 2:41 UTC (permalink / raw) To: binutils http://sourceware.org/ml/binutils/2013-01/msg00188.html bfd/ PR ld/12549 elflink.c (elf_link_add_object_symbols): Exclude weak refs when considering whether an --as-needed library is needed. ld/ * ld.texinfo (--as-needed): Update. ld/testsuite/ * ld-elf/pr14862.out: Expect no output. I'm committing the above patch. Apologies to the bug reporter that it's taken so long. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 17+ messages in thread
* [GOLD] --as-needed change wrt undefined weak symbols 2013-03-18 2:41 ` Alan Modra @ 2013-03-18 7:55 ` Alan Modra 2013-03-18 8:16 ` Alan Modra 2013-03-18 13:43 ` Will Newton 2013-04-04 17:16 ` H.J. Lu 2 siblings, 1 reply; 17+ messages in thread From: Alan Modra @ 2013-03-18 7:55 UTC (permalink / raw) To: binutils On Mon, Mar 18, 2013 at 01:11:30PM +1030, Alan Modra wrote: > http://sourceware.org/ml/binutils/2013-01/msg00188.html The corresponding gold change. Not quite so simple as just testing whether the reference is strong before setting is_needed, since we can't have symbol versions coming from a non-loaded shared lib. If you do, ld.so segfaults. OK to apply? * symtab.h (Symbol::override_version): Make public. * symtab.cc (Symbol_table::set_dynsym_indexes): Don't set object is_needed by weak references. Clear version for symbols defined in as-needed objects that are not needed. Index: gold/symtab.h =================================================================== RCS file: /cvs/src/src/gold/symtab.h,v retrieving revision 1.129 diff -u -p -r1.129 symtab.h --- gold/symtab.h 10 Mar 2013 23:08:18 -0000 1.129 +++ gold/symtab.h 18 Mar 2013 07:28:12 -0000 @@ -121,6 +121,10 @@ class Symbol version() const { return this->version_; } + // Override symbol version. + void + override_version(const char* version); + // Return whether this version is the default for this symbol name // (eg, "foo@@V2" is a default version; "foo@V1" is not). Only // meaningful for versioned symbols. @@ -872,10 +876,6 @@ class Symbol void override_base_with_special(const Symbol* from); - // Override symbol version. - void - override_version(const char* version); - // Allocate a common symbol by giving it a location in the output // file. void Index: gold/symtab.cc =================================================================== RCS file: /cvs/src/src/gold/symtab.cc,v retrieving revision 1.169 diff -u -p -r1.169 symtab.cc --- gold/symtab.cc 10 Mar 2013 23:08:18 -0000 1.169 +++ gold/symtab.cc 18 Mar 2013 07:36:40 -0000 @@ -2387,18 +2387,37 @@ Symbol_table::set_dynsym_indexes(unsigne syms->push_back(sym); dynpool->add(sym->name(), false, NULL); - // Record any version information. - if (sym->version() != NULL) - versions->record_version(this, dynpool, sym); - // If the symbol is defined in a dynamic object and is - // referenced in a regular object, then mark the dynamic - // object as needed. This is used to implement --as-needed. - if (sym->is_from_dynobj() && sym->in_reg()) + // referenced strongly in a regular object, then mark the + // dynamic object as needed. This is used to implement + // --as-needed. + if (sym->is_from_dynobj() + && sym->in_reg() + && !sym->is_undef_binding_weak()) sym->object()->set_is_needed(); } } + // Record any version information. + for (Symbol_table_type::iterator p = this->table_.begin(); + p != this->table_.end(); + ++p) + { + Symbol* sym = p->second; + + if (sym->has_dynsym_index() + && sym->dynsym_index() != -1U + && sym->version() != NULL) + { + if (!sym->is_from_dynobj() + || !sym->object()->as_needed() + || sym->object()->is_needed()) + versions->record_version(this, dynpool, sym); + else + sym->override_version(NULL); + } + } + // Finish up the versions. In some cases this may add new dynamic // symbols. index = versions->finalize(this, index, syms); -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [GOLD] --as-needed change wrt undefined weak symbols 2013-03-18 7:55 ` [GOLD] " Alan Modra @ 2013-03-18 8:16 ` Alan Modra [not found] ` <CAKOQZ8xsNiO7f2mNVLqy0vs5ws_C3sDXXdM60j5uSmfzyQKYEQ@mail.gmail.com> 0 siblings, 1 reply; 17+ messages in thread From: Alan Modra @ 2013-03-18 8:16 UTC (permalink / raw) To: binutils On Mon, Mar 18, 2013 at 06:24:17PM +1030, Alan Modra wrote: > On Mon, Mar 18, 2013 at 01:11:30PM +1030, Alan Modra wrote: > > http://sourceware.org/ml/binutils/2013-01/msg00188.html > > The corresponding gold change. Not quite so simple as just testing > whether the reference is strong before setting is_needed, since we > can't have symbol versions coming from a non-loaded shared lib. If > you do, ld.so segfaults. OK to apply? That was silly, we've just made a vector of dynamic symbols, so no need to traverse the whole symbol table. * symtab.h (Symbol::override_version): Make public. * symtab.cc (Symbol_table::set_dynsym_indexes): Don't set object is_needed by weak references. Clear version for symbols defined in as-needed objects that are not needed. Index: gold/symtab.h =================================================================== RCS file: /cvs/src/src/gold/symtab.h,v retrieving revision 1.129 diff -u -p -r1.129 symtab.h --- gold/symtab.h 10 Mar 2013 23:08:18 -0000 1.129 +++ gold/symtab.h 18 Mar 2013 08:13:00 -0000 @@ -121,6 +121,10 @@ class Symbol version() const { return this->version_; } + // Override symbol version. + void + override_version(const char* version); + // Return whether this version is the default for this symbol name // (eg, "foo@@V2" is a default version; "foo@V1" is not). Only // meaningful for versioned symbols. @@ -872,10 +876,6 @@ class Symbol void override_base_with_special(const Symbol* from); - // Override symbol version. - void - override_version(const char* version); - // Allocate a common symbol by giving it a location in the output // file. void Index: gold/symtab.cc =================================================================== RCS file: /cvs/src/src/gold/symtab.cc,v retrieving revision 1.169 diff -u -p -r1.169 symtab.cc --- gold/symtab.cc 10 Mar 2013 23:08:18 -0000 1.169 +++ gold/symtab.cc 18 Mar 2013 08:13:01 -0000 @@ -2387,18 +2387,35 @@ Symbol_table::set_dynsym_indexes(unsigne syms->push_back(sym); dynpool->add(sym->name(), false, NULL); - // Record any version information. - if (sym->version() != NULL) - versions->record_version(this, dynpool, sym); - // If the symbol is defined in a dynamic object and is - // referenced in a regular object, then mark the dynamic - // object as needed. This is used to implement --as-needed. - if (sym->is_from_dynobj() && sym->in_reg()) + // referenced strongly in a regular object, then mark the + // dynamic object as needed. This is used to implement + // --as-needed. + if (sym->is_from_dynobj() + && sym->in_reg() + && !sym->is_undef_binding_weak()) sym->object()->set_is_needed(); } } + // Record any version information. + for (std::vector<Symbol*>::iterator p = syms->begin(); + p != syms->end(); + ++p) + { + Symbol* sym = *p; + + if (sym->version() != NULL) + { + if (!sym->is_from_dynobj() + || !sym->object()->as_needed() + || sym->object()->is_needed()) + versions->record_version(this, dynpool, sym); + else + sym->override_version(NULL); + } + } + // Finish up the versions. In some cases this may add new dynamic // symbols. index = versions->finalize(this, index, syms); -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <CAKOQZ8xsNiO7f2mNVLqy0vs5ws_C3sDXXdM60j5uSmfzyQKYEQ@mail.gmail.com>]
[parent not found: <20130319023425.GG18331@bubble.grove.modra.org>]
* Re: [GOLD] --as-needed change wrt undefined weak symbols [not found] ` <20130319023425.GG18331@bubble.grove.modra.org> @ 2013-03-19 21:37 ` Ian Lance Taylor 2013-03-20 0:26 ` Alan Modra 0 siblings, 1 reply; 17+ messages in thread From: Ian Lance Taylor @ 2013-03-19 21:37 UTC (permalink / raw) To: Ian Lance Taylor, Binutils On Mon, Mar 18, 2013 at 7:34 PM, Alan Modra <amodra@gmail.com> wrote: > On Mon, Mar 18, 2013 at 08:15:55AM -0700, Ian Lance Taylor wrote: >> We can build a >> temporary vector of symbols with version != NULL for which that >> condition is not true, and then walk that vector in the second loop. >> There won't be many entries in it. > > Well, OK, but I wonder whether it is really worth doing? Sometimes > having two small loops is much better than having one large one. Yes, but there are executables here with tens of thousands of dynamic symbols, none of which are weak. > * symtab.h (Symbol::override_version): Make public. > * symtab.cc (Symbol_table::set_dynsym_indexes): Don't set object > is_needed by weak references. Clear version for symbols defined > in as-needed objects that are not needed. This is OK. Thanks. Ian ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [GOLD] --as-needed change wrt undefined weak symbols 2013-03-19 21:37 ` Ian Lance Taylor @ 2013-03-20 0:26 ` Alan Modra 0 siblings, 0 replies; 17+ messages in thread From: Alan Modra @ 2013-03-20 0:26 UTC (permalink / raw) To: Ian Lance Taylor; +Cc: Binutils On Tue, Mar 19, 2013 at 01:41:31PM -0700, Ian Lance Taylor wrote: > On Mon, Mar 18, 2013 at 7:34 PM, Alan Modra <amodra@gmail.com> wrote: > > On Mon, Mar 18, 2013 at 08:15:55AM -0700, Ian Lance Taylor wrote: > >> We can build a > >> temporary vector of symbols with version != NULL for which that > >> condition is not true, and then walk that vector in the second loop. > >> There won't be many entries in it. > > > > Well, OK, but I wonder whether it is really worth doing? Sometimes > > having two small loops is much better than having one large one. > > Yes, but there are executables here with tens of thousands of dynamic > symbols, none of which are weak. > > > * symtab.h (Symbol::override_version): Make public. > > * symtab.cc (Symbol_table::set_dynsym_indexes): Don't set object > > is_needed by weak references. Clear version for symbols defined > > in as-needed objects that are not needed. > > This is OK. When compiling with a different gcc and options (previously I used -fno-inline), I hit ...symtab.cc:2426: undefined reference to `gold::Symbol::override_version(char const*)' So I'm committing this slightly different patch. * symtab.h (Symbol::clear_version): New function. * symtab.cc (Symbol_table::set_dynsym_indexes): Don't set object is_needed by weak references. Clear version for symbols defined in as-needed objects that are not needed. Index: gold/symtab.h =================================================================== RCS file: /cvs/src/src/gold/symtab.h,v retrieving revision 1.129 diff -u -p -r1.129 symtab.h --- gold/symtab.h 10 Mar 2013 23:08:18 -0000 1.129 +++ gold/symtab.h 19 Mar 2013 22:33:39 -0000 @@ -121,6 +121,10 @@ class Symbol version() const { return this->version_; } + void + clear_version() + { this->version_ = NULL; } + // Return whether this version is the default for this symbol name // (eg, "foo@@V2" is a default version; "foo@V1" is not). Only // meaningful for versioned symbols. Index: gold/symtab.cc =================================================================== RCS file: /cvs/src/src/gold/symtab.cc,v retrieving revision 1.169 diff -u -p -r1.169 symtab.cc --- gold/symtab.cc 10 Mar 2013 23:08:18 -0000 1.169 +++ gold/symtab.cc 19 Mar 2013 22:33:40 -0000 @@ -2368,6 +2368,8 @@ Symbol_table::set_dynsym_indexes(unsigne Stringpool* dynpool, Versions* versions) { + std::vector<Symbol*> as_needed_sym; + for (Symbol_table_type::iterator p = this->table_.begin(); p != this->table_.end(); ++p) @@ -2387,18 +2389,43 @@ Symbol_table::set_dynsym_indexes(unsigne syms->push_back(sym); dynpool->add(sym->name(), false, NULL); - // Record any version information. - if (sym->version() != NULL) - versions->record_version(this, dynpool, sym); - // If the symbol is defined in a dynamic object and is - // referenced in a regular object, then mark the dynamic - // object as needed. This is used to implement --as-needed. - if (sym->is_from_dynobj() && sym->in_reg()) + // referenced strongly in a regular object, then mark the + // dynamic object as needed. This is used to implement + // --as-needed. + if (sym->is_from_dynobj() + && sym->in_reg() + && !sym->is_undef_binding_weak()) sym->object()->set_is_needed(); + + // Record any version information, except those from + // as-needed libraries not seen to be needed. Note that the + // is_needed state for such libraries can change in this loop. + if (sym->version() != NULL) + { + if (!sym->is_from_dynobj() + || !sym->object()->as_needed() + || sym->object()->is_needed()) + versions->record_version(this, dynpool, sym); + else + as_needed_sym.push_back(sym); + } } } + // Process version information for symbols from as-needed libraries. + for (std::vector<Symbol*>::iterator p = as_needed_sym.begin(); + p != as_needed_sym.end(); + ++p) + { + Symbol* sym = *p; + + if (sym->object()->is_needed()) + versions->record_version(this, dynpool, sym); + else + sym->clear_version(); + } + // Finish up the versions. In some cases this may add new dynamic // symbols. index = versions->finalize(this, index, syms); -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: --as-needed change wrt undefined weak symbols 2013-03-18 2:41 ` Alan Modra 2013-03-18 7:55 ` [GOLD] " Alan Modra @ 2013-03-18 13:43 ` Will Newton [not found] ` <CANu=DmjqJHk2qhcxcGF=8TKVKJbv8TLSpy94MVK9+2yAdaR5Mg@mail.gmail.com> 2013-04-04 17:16 ` H.J. Lu 2 siblings, 1 reply; 17+ messages in thread From: Will Newton @ 2013-03-18 13:43 UTC (permalink / raw) To: binutils On 18 March 2013 02:41, Alan Modra <amodra@gmail.com> wrote: > http://sourceware.org/ml/binutils/2013-01/msg00188.html > > bfd/ > PR ld/12549 > elflink.c (elf_link_add_object_symbols): Exclude weak refs when > considering whether an --as-needed library is needed. > ld/ > * ld.texinfo (--as-needed): Update. > ld/testsuite/ > * ld-elf/pr14862.out: Expect no output. > > I'm committing the above patch. Apologies to the bug reporter that > it's taken so long. Hi Alan, This change looks like it should break the "ELF weak func last DSO" test in ld/testsuite/ld-elfweak/elfweak.exp but there doesn't seem to be any change to that file. Is that the test that is referred to in the thread linked above? Thanks, --- Will Newton Toolchain Working Group, Linaro ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <CANu=DmjqJHk2qhcxcGF=8TKVKJbv8TLSpy94MVK9+2yAdaR5Mg@mail.gmail.com>]
[parent not found: <20130319023947.GH18331@bubble.grove.modra.org>]
* Re: --as-needed change wrt undefined weak symbols [not found] ` <20130319023947.GH18331@bubble.grove.modra.org> @ 2013-03-20 2:27 ` Alan Modra 0 siblings, 0 replies; 17+ messages in thread From: Alan Modra @ 2013-03-20 2:27 UTC (permalink / raw) To: Will Newton, binutils On Tue, Mar 19, 2013 at 01:09:47PM +1030, Alan Modra wrote: > On Mon, Mar 18, 2013 at 02:39:43PM +0000, Will Newton wrote: > > Would the below patch be an acceptable approach to work around this issue? > > Yes, this is OK to apply with a suitable changelog entry. The following incorporates your patch. Applying mainline. * ld-elfvers/vers.exp: Add -Wl,--no-as-needed to all tests linking against shared libraries. * ld-elfweak/elfweak.exp: Likewise. Enable for x86_64-linux. Build main1.o using $picflag. Index: ld/testsuite/ld-elfvers/vers.exp =================================================================== RCS file: /cvs/src/src/ld/testsuite/ld-elfvers/vers.exp,v retrieving revision 1.58 diff -u -p -r1.58 vers.exp --- ld/testsuite/ld-elfvers/vers.exp 19 Feb 2013 01:09:58 -0000 1.58 +++ ld/testsuite/ld-elfvers/vers.exp 20 Mar 2013 01:51:34 -0000 @@ -796,7 +796,7 @@ build_vers_lib_pic "vers2" vers2.c vers2 # # Test #3 - build an executable, and link it against vers1.so. # -build_exec "vers3" vers3.c vers3 "" vers1.so vers3.ver vers3.dsym "" +build_exec "vers3" vers3.c vers3 "-Wl,--no-as-needed" vers1.so vers3.ver vers3.dsym "" # # Test #4 - Make sure a version implicitly defined in an executable @@ -827,7 +827,7 @@ test_ldfail "vers5" "" vers5.c vers5 "" # Now build a test that should reference a bunch of versioned symbols. # All of them should be correctly referenced. # -build_exec "vers6" vers6.c vers6 "" vers1.so vers6.ver vers6.dsym vers6.sym +build_exec "vers6" vers6.c vers6 "-Wl,--no-as-needed" vers1.so vers6.ver vers6.dsym vers6.sym # # Another test to verify that something made local via 'local' is truly not @@ -894,12 +894,12 @@ build_exec "vers15" vers15.c vers15 "-Wl # symbol appears in the dynamic symbol table of the executable. # build_vers_lib_pic "vers16a" vers16a.c vers16a "" vers16.map vers16a.ver vers16a.dsym "" -build_exec "vers16" vers16.c vers16 "" vers16a.so "" vers16.dsym "" +build_exec "vers16" vers16.c vers16 "-Wl,--no-as-needed" vers16a.so "" vers16.dsym "" # Test a weak versioned symbol. build_vers_lib_pic "vers17" vers17.c vers17 "" vers17.map vers17.ver vers17.dsym "" build_vers_lib_pic "vers18" vers18.c vers18 vers17.so vers18.map vers18.ver vers18.dsym vers18.sym -build_exec "vers19" vers19.c vers19 "-Wl,-rpath,. -Wl,-rpath-link,." vers18.so vers19.ver vers19.dsym "" +build_exec "vers19" vers19.c vers19 "-Wl,-rpath,. -Wl,-rpath-link,--no-as-needed" vers18.so vers19.ver vers19.dsym "" build_vers_lib_no_pic "vers20a" vers20.c vers20a "" vers20.map vers20a.ver vers20.dsym "" exec cp $tmpdir/vers20a.so $tmpdir/vers20b.so @@ -924,8 +924,8 @@ if [string match "yes" $pic] then { build_vers_lib_no_pic "vers23a" vers23a.c vers23a "" vers23a.map vers23a.ver vers23a.dsym vers23a.sym build_vers_lib_no_pic "vers23b" vers23b.c vers23b "" vers23b.map vers23b.ver vers23b.dsym "" build_vers_lib_no_pic "vers23c" vers23b.c vers23c "vers23a.so" vers23b.map vers23c.ver vers23b.dsym "" - build_exec "vers23d" vers23.c vers23d "tmpdir/vers23a.so tmpdir/vers23c.so" "" vers23.ver vers23d.dsym "" - build_exec "vers23" vers23.c vers23 "tmpdir/vers23a.so tmpdir/vers23b.o tmpdir/vers23b.so" "" vers23.ver vers23.dsym "" + build_exec "vers23d" vers23.c vers23d "-Wl,--no-as-needed tmpdir/vers23a.so tmpdir/vers23c.so" "" vers23.ver vers23d.dsym "" + build_exec "vers23" vers23.c vers23 "-Wl,--no-as-needed tmpdir/vers23a.so tmpdir/vers23b.o tmpdir/vers23b.so" "" vers23.ver vers23.dsym "" } # Test .symver x,x@VERS.0 Index: ld/testsuite/ld-elfweak/elfweak.exp =================================================================== RCS file: /cvs/src/src/ld/testsuite/ld-elfweak/elfweak.exp,v retrieving revision 1.20 diff -u -p -r1.20 elfweak.exp --- ld/testsuite/ld-elfweak/elfweak.exp 3 Apr 2012 16:01:35 -0000 1.20 +++ ld/testsuite/ld-elfweak/elfweak.exp 20 Mar 2013 00:30:25 -0000 @@ -50,6 +50,7 @@ if { ![istarget alpha*-*-linux*] && ![istarget sparc*-*-elf] && ![istarget sparc*-*-solaris2*] && ![istarget sparc*-*-linux*] + && ![istarget x86_64-*-linux*] && ![istarget *-*-nacl*] } { return } @@ -67,7 +68,7 @@ set diff diff set tmpdir tmpdir set DOBJDUMP_FLAGS --dynamic-syms set SOBJDUMP_FLAGS --syms -set shared --shared +set shared "--shared -Wl,--no-as-needed" # <http://www.gnu.org/software/hurd/open_issues/binutils.html#weak> @@ -316,7 +317,6 @@ proc build_exec { test execname objs fla global CC global objdump global tmpdir - global shared global srcdir global subdir global exec_output @@ -442,7 +442,7 @@ if ![ld_compile "$CC $CFLAGS $picflag" $ return } -if ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/main1.c $tmpdir/main1.o] { +if ![ld_compile "$CC $CFLAGS $picflag" $srcdir/$subdir/main1.c $tmpdir/main1.o] { unresolved "ELF weak" return } @@ -469,9 +469,9 @@ build_lib "ELF DSO weak func last DSO" l build_exec "ELF weak func first" foo "main.o bar.o" "" strong "" strong.sym build_exec "ELF weak func last" foo "bar.o main.o" "" strong "" strong.sym setup_xfail_gnu_hurd -build_exec "ELF weak func first DSO" foo "main.o libbar.so" "-Wl,-rpath,." weak weak.dsym "" +build_exec "ELF weak func first DSO" foo "main.o libbar.so" "-Wl,-rpath,.,--no-as-needed" weak weak.dsym "" setup_xfail_gnu_hurd -build_exec "ELF weak func last DSO" foo "libbar.so main.o" "-Wl,-rpath,." weak weak.dsym "" +build_exec "ELF weak func last DSO" foo "libbar.so main.o" "-Wl,-rpath,.,--no-as-needed" weak weak.dsym "" build_lib "ELF DSO weak data first" libfoo "bar1a.o foo1a.o" dsodata.dsym build_lib "ELF DSO weak data last" libfoo "foo1a.o bar1a.o" dsodata.dsym @@ -484,13 +484,13 @@ build_exec "ELF weak data last" foo "foo build_exec "ELF weak data first common" foo "main1.o bar1a.o foo1b.o" "" strongdata "" strongcomm.sym build_exec "ELF weak data last common" foo "foo1b.o main1.o bar1a.o" "" strongdata "" strongcomm.sym setup_xfail_gnu_hurd -build_exec "ELF weak data first DSO" foo "main1.o libbar1a.so libfoo1a.so" "-Wl,-rpath,." weakdata weakdata.dsym "" +build_exec "ELF weak data first DSO" foo "main1.o libbar1a.so libfoo1a.so" "-Wl,-rpath,.,--no-as-needed" weakdata weakdata.dsym "" setup_xfail_gnu_hurd -build_exec "ELF weak data last DSO" foo "libfoo1a.so main1.o libbar1a.so" "-Wl,-rpath,." weakdata weakdata.dsym "" +build_exec "ELF weak data last DSO" foo "libfoo1a.so main1.o libbar1a.so" "-Wl,-rpath,.,--no-as-needed" weakdata weakdata.dsym "" setup_xfail_gnu_hurd -build_exec "ELF weak data first DSO common" foo "main1.o libbar1a.so libfoo1b.so" "-Wl,-rpath,." weakdata weakdata.dsym "" +build_exec "ELF weak data first DSO common" foo "main1.o libbar1a.so libfoo1b.so" "-Wl,-rpath,.,--no-as-needed" weakdata weakdata.dsym "" setup_xfail_gnu_hurd -build_exec "ELF weak data last DSO common" foo "libfoo1b.so main1.o libbar1a.so" "-Wl,-rpath,." weakdata weakdata.dsym "" +build_exec "ELF weak data last DSO common" foo "libfoo1b.so main1.o libbar1a.so" "-Wl,-rpath,.,--no-as-needed" weakdata weakdata.dsym "" if ![ld_compile "$CC $CFLAGS $picflag" $srcdir/$subdir/size_foo.c $tmpdir/size_foo.o] { unresolved "ELF weak (size)" @@ -517,7 +517,7 @@ if ![ld_compile "$CC $CFLAGS" $srcdir/$s return } -build_exec "ELF weak size" size_main "size_main.o libsize_foo.so libsize_bar.so" "-Wl,-rpath,." size "" "" +build_exec "ELF weak size" size_main "size_main.o libsize_foo.so libsize_bar.so" "-Wl,-rpath,.,--no-as-needed" size "" "" verbose "size2" run_dump_test $srcdir/$subdir/size2 -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: --as-needed change wrt undefined weak symbols 2013-03-18 2:41 ` Alan Modra 2013-03-18 7:55 ` [GOLD] " Alan Modra 2013-03-18 13:43 ` Will Newton @ 2013-04-04 17:16 ` H.J. Lu 2013-04-04 22:39 ` Alan Modra 2 siblings, 1 reply; 17+ messages in thread From: H.J. Lu @ 2013-04-04 17:16 UTC (permalink / raw) To: binutils On Sun, Mar 17, 2013 at 7:41 PM, Alan Modra <amodra@gmail.com> wrote: > http://sourceware.org/ml/binutils/2013-01/msg00188.html > > bfd/ > PR ld/12549 > elflink.c (elf_link_add_object_symbols): Exclude weak refs when > considering whether an --as-needed library is needed. > ld/ > * ld.texinfo (--as-needed): Update. > ld/testsuite/ > * ld-elf/pr14862.out: Expect no output. > > I'm committing the above patch. Apologies to the bug reporter that > it's taken so long. > > -- > Alan Modra > Australia Development Lab, IBM Just for the record, this patch may change the behavior of the resulting executables for extern void bar () __attribute__((weak)); if (bar) bar (); if bar is defined in the DT_NEEDED library. Binutils 2.22 will resolve bar and add a DT_NEEDED entry. The new linker will resolve bar to 0. We will see if it causes any problems. -- H.J. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: --as-needed change wrt undefined weak symbols 2013-04-04 17:16 ` H.J. Lu @ 2013-04-04 22:39 ` Alan Modra 2013-04-04 23:11 ` H.J. Lu 0 siblings, 1 reply; 17+ messages in thread From: Alan Modra @ 2013-04-04 22:39 UTC (permalink / raw) To: H.J. Lu; +Cc: binutils On Thu, Apr 04, 2013 at 10:15:58AM -0700, H.J. Lu wrote: > On Sun, Mar 17, 2013 at 7:41 PM, Alan Modra <amodra@gmail.com> wrote: > > PR ld/12549 > > elflink.c (elf_link_add_object_symbols): Exclude weak refs when > > considering whether an --as-needed library is needed. > > Just for the record, this patch may change the behavior of > the resulting executables for > > extern void bar () __attribute__((weak)); > > if (bar) > bar (); > > if bar is defined in the DT_NEEDED library. Binutils 2.22 > will resolve bar and add a DT_NEEDED entry. The new > linker will resolve bar to 0. We will see if it causes any > problems. Your testcase in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56431 shows that gold resolves bar to zero, but on powerpc and powerpc64 ld keeps bar dynamic. Hmm, I see x86_64 ld resolves bar to 0. Isn't this just an x86_64 backend ld bug? -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: --as-needed change wrt undefined weak symbols 2013-04-04 22:39 ` Alan Modra @ 2013-04-04 23:11 ` H.J. Lu 2013-04-05 1:07 ` Alan Modra 0 siblings, 1 reply; 17+ messages in thread From: H.J. Lu @ 2013-04-04 23:11 UTC (permalink / raw) To: Binutils On Thu, Apr 4, 2013 at 3:39 PM, Alan Modra <amodra@gmail.com> wrote: > On Thu, Apr 04, 2013 at 10:15:58AM -0700, H.J. Lu wrote: >> On Sun, Mar 17, 2013 at 7:41 PM, Alan Modra <amodra@gmail.com> wrote: >> > PR ld/12549 >> > elflink.c (elf_link_add_object_symbols): Exclude weak refs when >> > considering whether an --as-needed library is needed. >> >> Just for the record, this patch may change the behavior of >> the resulting executables for >> >> extern void bar () __attribute__((weak)); >> >> if (bar) >> bar (); >> >> if bar is defined in the DT_NEEDED library. Binutils 2.22 >> will resolve bar and add a DT_NEEDED entry. The new >> linker will resolve bar to 0. We will see if it causes any >> problems. > > Your testcase in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56431 > shows that gold resolves bar to zero, but on powerpc and powerpc64 ld > keeps bar dynamic. > > Hmm, I see x86_64 ld resolves bar to 0. Isn't this just an x86_64 > backend ld bug? For executable: 0000000000000000 <main>: 0: b8 00 00 00 00 mov $0x0,%eax 1: R_X86_64_32 bar 5: 48 85 c0 test %rax,%rax 8: 74 18 je 22 <main+0x22> a: 48 83 ec 08 sub $0x8,%rsp e: b8 00 00 00 00 mov $0x0,%eax 13: e8 00 00 00 00 callq 18 <main+0x18> 14: R_X86_64_PC32 bar-0x4 18: b8 00 00 00 00 mov $0x0,%eax 1d: 48 83 c4 08 add $0x8,%rsp 21: c3 retq 22: b8 00 00 00 00 mov $0x0,%eax 27: c3 retq It is resolved to either 0, if it is undefined, or its PLT entry, if it is defined. Once it is resolved to 0 at link-time, change to defined at run-time won't affect executable. If it is resolved to defined at link-time, change it to undefined at run-time will lead to seg-fault. If the executable is compiled with PIC/PIE, it works fine. -- H.J. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: --as-needed change wrt undefined weak symbols 2013-04-04 23:11 ` H.J. Lu @ 2013-04-05 1:07 ` Alan Modra 0 siblings, 0 replies; 17+ messages in thread From: Alan Modra @ 2013-04-05 1:07 UTC (permalink / raw) To: H.J. Lu; +Cc: Binutils On Thu, Apr 04, 2013 at 04:11:54PM -0700, H.J. Lu wrote: > It is resolved to either 0, if it is undefined, or its PLT entry, if > it is defined. Once it is resolved to 0 at link-time, change to > defined at run-time won't affect executable. If it is resolved > to defined at link-time, change it to undefined at run-time > will lead to seg-fault. Yes, that is a problem with any target that resolves function addresses to a PLT entry. For such targets, this is generally true for any usage of weak functions in a non-PIC executable. In other words, the problem you raise here is entirely orthogonal to --as-needed behaviour. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2013-04-05 1:07 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-01-14 3:58 --as-needed change wrt undefined weak symbols Alan Modra 2013-01-14 15:31 ` H.J. Lu 2013-01-15 2:23 ` Alan Modra 2013-01-15 3:30 ` H.J. Lu 2013-01-15 5:23 ` Alan Modra 2013-01-15 15:04 ` H.J. Lu 2013-03-18 2:41 ` Alan Modra 2013-03-18 7:55 ` [GOLD] " Alan Modra 2013-03-18 8:16 ` Alan Modra [not found] ` <CAKOQZ8xsNiO7f2mNVLqy0vs5ws_C3sDXXdM60j5uSmfzyQKYEQ@mail.gmail.com> [not found] ` <20130319023425.GG18331@bubble.grove.modra.org> 2013-03-19 21:37 ` Ian Lance Taylor 2013-03-20 0:26 ` Alan Modra 2013-03-18 13:43 ` Will Newton [not found] ` <CANu=DmjqJHk2qhcxcGF=8TKVKJbv8TLSpy94MVK9+2yAdaR5Mg@mail.gmail.com> [not found] ` <20130319023947.GH18331@bubble.grove.modra.org> 2013-03-20 2:27 ` Alan Modra 2013-04-04 17:16 ` H.J. Lu 2013-04-04 22:39 ` Alan Modra 2013-04-04 23:11 ` H.J. Lu 2013-04-05 1:07 ` Alan Modra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).