--as-needed change wrt undefined weak symbols

--as-needed change wrt undefined weak symbols

[Alan Modra]

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

Re: --as-needed change wrt undefined weak symbols

[H.J. Lu]

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.

Re: --as-needed change wrt undefined weak symbols

[Alan Modra]

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

Re: --as-needed change wrt undefined weak symbols

[H.J. Lu]

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.

Re: --as-needed change wrt undefined weak symbols

[Alan Modra]

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

Re: --as-needed change wrt undefined weak symbols

[H.J. Lu]

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.

Re: --as-needed change wrt undefined weak symbols

[Alan Modra]

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

[GOLD] --as-needed change wrt undefined weak symbols

[Alan Modra]

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

Re: [GOLD] --as-needed change wrt undefined weak symbols

[Alan Modra]

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

Re: --as-needed change wrt undefined weak symbols

[Will Newton]

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

Re: [GOLD] --as-needed change wrt undefined weak symbols

[Ian Lance Taylor]

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

Re: [GOLD] --as-needed change wrt undefined weak symbols

[Alan Modra]

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

Re: --as-needed change wrt undefined weak symbols

[Alan Modra]

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

Re: --as-needed change wrt undefined weak symbols

[H.J. Lu]

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.

Re: --as-needed change wrt undefined weak symbols

[Alan Modra]

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

Re: --as-needed change wrt undefined weak symbols

[H.J. Lu]

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.

Re: --as-needed change wrt undefined weak symbols

[Alan Modra]

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