[PATCH] Fix DT_NEEDED search with --as-needed libraries (PR ld/2721)

[PATCH] Fix DT_NEEDED search with --as-needed libraries (PR ld/2721)

[Jakub Jelinek]

Hi!

If some --as-needed library mentioned on the command line isn't directly
needed by executable being linked, but it is mentioned in DT_NEEDED of some
needed library, then without this patch ld will disregard the library
mentioned on the command line and look for a different one (e.g. in system
paths).

2006-06-01  Jakub Jelinek  <jakub@redhat.com>

	PR ld/2721
	* emultempl/elf32.em (global_found_as_needed): New variable.
	(gld${EMULATION_NAME}_check_needed): Succeed even for DYN_AS_NEEDED
	libraries.
	(gld${EMULATION_NAME}_after_open): If needed library is already
	loaded with DYN_AS_NEEDED flag set, try to open that library again.

--- ld/emultempl/elf32.em.jj	2006-06-01 15:50:34.000000000 +0200
+++ ld/emultempl/elf32.em	2006-06-01 16:55:02.000000000 +0200
@@ -149,6 +149,7 @@ cat >>e${EMULATION_NAME}.c <<EOF
 static struct bfd_link_needed_list *global_needed;
 static struct stat global_stat;
 static bfd_boolean global_found;
+static const char *global_found_as_needed;
 static struct bfd_link_needed_list *global_vercheck_needed;
 static bfd_boolean global_vercheck_failed;
 
@@ -809,7 +810,9 @@ cat >>e${EMULATION_NAME}.c <<EOF
 static void
 gld${EMULATION_NAME}_check_needed (lang_input_statement_type *s)
 {
-  if (global_found)
+  const char *found_as_needed = NULL;
+
+  if (global_found && global_found_as_needed == NULL)
     return;
 
   /* If this input file was an as-needed entry, and wasn't found to be
@@ -817,7 +820,11 @@ gld${EMULATION_NAME}_check_needed (lang_
   if (s->as_needed
       && (s->the_bfd == NULL
 	  || (bfd_elf_get_dyn_lib_class (s->the_bfd) & DYN_AS_NEEDED) != 0))
-    return;
+    {
+      if (global_found || s->filename == NULL)
+	return;
+      found_as_needed = s->filename;
+    }
 
   if (s->filename != NULL)
     {
@@ -826,6 +833,7 @@ gld${EMULATION_NAME}_check_needed (lang_
       if (strcmp (s->filename, global_needed->name) == 0)
 	{
 	  global_found = TRUE;
+	  global_found_as_needed = found_as_needed;
 	  return;
 	}
 
@@ -836,6 +844,7 @@ gld${EMULATION_NAME}_check_needed (lang_
 	      && strcmp (f + 1, global_needed->name) == 0)
 	    {
 	      global_found = TRUE;
+	      global_found_as_needed = found_as_needed;
 	      return;
 	    }
 	}
@@ -850,6 +859,7 @@ gld${EMULATION_NAME}_check_needed (lang_
 	  && strcmp (soname, global_needed->name) == 0)
 	{
 	  global_found = TRUE;
+	  global_found_as_needed = found_as_needed;
 	  return;
 	}
     }
@@ -886,6 +896,7 @@ gld${EMULATION_NAME}_after_open (void)
       struct bfd_link_needed_list *ll;
       struct dt_needed n, nn;
       int force;
+      const char *found_as_needed;
 
       /* If the lib that needs this one was --as-needed and wasn't
 	 found to be needed, then this lib isn't needed either.  */
@@ -906,8 +917,14 @@ gld${EMULATION_NAME}_after_open (void)
       global_needed = l;
       global_found = FALSE;
       lang_for_each_input_file (gld${EMULATION_NAME}_check_needed);
+      found_as_needed = NULL;
       if (global_found)
-	continue;
+	{
+	  if (global_found_as_needed == NULL)
+	    continue;
+	  else
+	    found_as_needed = global_found_as_needed;
+	}
 
       n.by = l->by;
       n.name = l->name;
@@ -944,6 +961,13 @@ EOF
 fi
 cat >>e${EMULATION_NAME}.c <<EOF
 
+	  if (found_as_needed != NULL)
+	    {
+	      nn.name = found_as_needed;
+	      if (gld${EMULATION_NAME}_try_needed (&nn, force))
+		break;
+	    }
+
 	  if (gld${EMULATION_NAME}_search_needed (command_line.rpath_link,
 						  &n, force))
 	    break;

	Jakub

[PATCH] Fix DT_NEEDED search with --as-needed libraries (PR ld/2721)

[Jakub Jelinek]

Hi!

If some --as-needed library mentioned on the command line isn't directly
needed by executable being linked, but it is mentioned in DT_NEEDED of some
needed library, then without this patch ld will disregard the library
mentioned on the command line and look for a different one (e.g. in system
paths).

2006-06-01  Jakub Jelinek  <jakub@redhat.com>

	PR ld/2721
	* emultempl/elf32.em (global_found_as_needed): New variable.
	(gld${EMULATION_NAME}_check_needed): Succeed even for DYN_AS_NEEDED
	libraries.
	(gld${EMULATION_NAME}_after_open): If needed library is already
	loaded with DYN_AS_NEEDED flag set, try to open that library again.

--- ld/emultempl/elf32.em.jj	2006-06-01 15:50:34.000000000 +0200
+++ ld/emultempl/elf32.em	2006-06-01 16:55:02.000000000 +0200
@@ -149,6 +149,7 @@ cat >>e${EMULATION_NAME}.c <<EOF
 static struct bfd_link_needed_list *global_needed;
 static struct stat global_stat;
 static bfd_boolean global_found;
+static const char *global_found_as_needed;
 static struct bfd_link_needed_list *global_vercheck_needed;
 static bfd_boolean global_vercheck_failed;
 
@@ -809,7 +810,9 @@ cat >>e${EMULATION_NAME}.c <<EOF
 static void
 gld${EMULATION_NAME}_check_needed (lang_input_statement_type *s)
 {
-  if (global_found)
+  const char *found_as_needed = NULL;
+
+  if (global_found && global_found_as_needed == NULL)
     return;
 
   /* If this input file was an as-needed entry, and wasn't found to be
@@ -817,7 +820,11 @@ gld${EMULATION_NAME}_check_needed (lang_
   if (s->as_needed
       && (s->the_bfd == NULL
 	  || (bfd_elf_get_dyn_lib_class (s->the_bfd) & DYN_AS_NEEDED) != 0))
-    return;
+    {
+      if (global_found || s->filename == NULL)
+	return;
+      found_as_needed = s->filename;
+    }
 
   if (s->filename != NULL)
     {
@@ -826,6 +833,7 @@ gld${EMULATION_NAME}_check_needed (lang_
       if (strcmp (s->filename, global_needed->name) == 0)
 	{
 	  global_found = TRUE;
+	  global_found_as_needed = found_as_needed;
 	  return;
 	}
 
@@ -836,6 +844,7 @@ gld${EMULATION_NAME}_check_needed (lang_
 	      && strcmp (f + 1, global_needed->name) == 0)
 	    {
 	      global_found = TRUE;
+	      global_found_as_needed = found_as_needed;
 	      return;
 	    }
 	}
@@ -850,6 +859,7 @@ gld${EMULATION_NAME}_check_needed (lang_
 	  && strcmp (soname, global_needed->name) == 0)
 	{
 	  global_found = TRUE;
+	  global_found_as_needed = found_as_needed;
 	  return;
 	}
     }
@@ -886,6 +896,7 @@ gld${EMULATION_NAME}_after_open (void)
       struct bfd_link_needed_list *ll;
       struct dt_needed n, nn;
       int force;
+      const char *found_as_needed;
 
       /* If the lib that needs this one was --as-needed and wasn't
 	 found to be needed, then this lib isn't needed either.  */
@@ -906,8 +917,14 @@ gld${EMULATION_NAME}_after_open (void)
       global_needed = l;
       global_found = FALSE;
       lang_for_each_input_file (gld${EMULATION_NAME}_check_needed);
+      found_as_needed = NULL;
       if (global_found)
-	continue;
+	{
+	  if (global_found_as_needed == NULL)
+	    continue;
+	  else
+	    found_as_needed = global_found_as_needed;
+	}
 
       n.by = l->by;
       n.name = l->name;
@@ -944,6 +961,13 @@ EOF
 fi
 cat >>e${EMULATION_NAME}.c <<EOF
 
+	  if (found_as_needed != NULL)
+	    {
+	      nn.name = found_as_needed;
+	      if (gld${EMULATION_NAME}_try_needed (&nn, force))
+		break;
+	    }
+
 	  if (gld${EMULATION_NAME}_search_needed (command_line.rpath_link,
 						  &n, force))
 	    break;

	Jakub

Re: [PATCH] Fix DT_NEEDED search with --as-needed libraries (PR ld/2721)

[H. J. Lu]

On Thu, Jun 01, 2006 at 05:09:07PM +0200, Jakub Jelinek wrote:
> Hi!
> 
> If some --as-needed library mentioned on the command line isn't directly
> needed by executable being linked, but it is mentioned in DT_NEEDED of some
> needed library, then without this patch ld will disregard the library
> mentioned on the command line and look for a different one (e.g. in system
> paths).
> 

Can we also add a testcase?


H.J.

Re: [PATCH] Fix DT_NEEDED search with --as-needed libraries (PR ld/2721)

[Alan Modra]

On Thu, Jun 01, 2006 at 05:09:07PM +0200, Jakub Jelinek wrote:
> If some --as-needed library mentioned on the command line isn't directly
> needed by executable being linked, but it is mentioned in DT_NEEDED of some
> needed library, then without this patch ld will disregard the library
> mentioned on the command line and look for a different one (e.g. in system
> paths).

According to the current specification of --as-needed, isn't that
exactly what ld should do?  --as-needed is supposed to cause ld to only
link a lib if it satisfies some reference from a regular object
(ie. non-dynamic) at that point in the link process.  This patch makes
--as-needed have another meaning entirely.

I'm willing to consider a behaviour change for --as-needed, eg. that
references from other dynamic libs cause a lib to be linked, but this
particular change seems a little odd.  Why should a library that you
decide isn't needed be considered again, unless it is also found by the
normal search mechanism?

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [PATCH] Fix DT_NEEDED search with --as-needed libraries (PR ld/2721)

[Jakub Jelinek]

On Fri, Jun 02, 2006 at 02:43:44PM +0930, Alan Modra wrote:
> On Thu, Jun 01, 2006 at 05:09:07PM +0200, Jakub Jelinek wrote:
> > If some --as-needed library mentioned on the command line isn't directly
> > needed by executable being linked, but it is mentioned in DT_NEEDED of some
> > needed library, then without this patch ld will disregard the library
> > mentioned on the command line and look for a different one (e.g. in system
> > paths).
> 
> According to the current specification of --as-needed, isn't that
> exactly what ld should do?  --as-needed is supposed to cause ld to only
> link a lib if it satisfies some reference from a regular object
> (ie. non-dynamic) at that point in the link process.  This patch makes
> --as-needed have another meaning entirely.
> 
> I'm willing to consider a behaviour change for --as-needed, eg. that
> references from other dynamic libs cause a lib to be linked, but this
> particular change seems a little odd.  Why should a library that you
> decide isn't needed be considered again, unless it is also found by the
> normal search mechanism?

Because without it --as-needed isn't really usable with libtool,
unless libtool is changed to pass -Wl,--rpath-link with every single library
it adds (and even in that case it might do unexpected things if there are
multiple libraries in the same directory).
The testcase in the PR is a simplified version of
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=193689
which is I think quite commonly used scenario.  A project that builds
2 or more shared libraries in the build process, where the second of these
depends on the first one, and then build a binary that depends on the second
of the shared library (but doesn't have direct dependencies on the first).
The first library is actually needed by the program, as it is a dependency
of the second, but as it is not a direct dependency, it is not needed
in DT_NEEDED of the program.
The --as-needed documentation doesn't talk about linking/not linking some
library, it solely talks about DT_NEEDED:
`--as-needed'
`--no-as-needed'
     This option affects ELF DT_NEEDED tags for dynamic libraries
     mentioned on the command line after the `--as-needed' option.
     Normally, 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.  `--as-needed' causes DT_NEEDED tags
     to only be emitted for libraries that satisfy some symbol
     reference from regular objects which is undefined at the point
     that the library was linked.  `--no-as-needed' restores the
     default behaviour.
With the patch I posted, the first library won't be in binary's DT_NEEDED,
which matches the documentation, but there is no word about --as-needed
affecting anything else, particularly what libraries are searched when
trying to satisfy undefined references in shared libraries mentioned
in DT_NEEDED.

The second testcase in the PR is a regression from 2.16.90.x.y btw.

	Jakub

Re: [PATCH] Fix DT_NEEDED search with --as-needed libraries (PR ld/2721)

[Diego 'Flameeyes' Pettenò]

[-- Attachment #1: Type: text/plain, Size: 859 bytes --]

On Friday 02 June 2006 10:49, Jakub Jelinek wrote:
> Because without it --as-needed isn't really usable with libtool,
> unless libtool is changed to pass -Wl,--rpath-link with every single
> library it adds (and even in that case it might do unexpected things if
> there are multiple libraries in the same directory).
Quite a common scenario indeed, I just (well, two days ago) hit this kind of 
problem in kdelibs update: a just built library was linked against the 
installed copy of another library because of this, but there was a missing 
function that's only present in the new copy of them.

Thanks for this patch Jakub, this was one of the main issues I've encountered 
with --as-needed up to now.

-- 
Diego "Flameeyes" Pettenò - http://farragut.flameeyes.is-a-geek.org/
Gentoo/Alt lead, Gentoo/FreeBSD, Video, AMD64, Sound, PAM, KDE

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Fix DT_NEEDED search with --as-needed libraries (PR ld/2721)

[Alan Modra]

On Fri, Jun 02, 2006 at 10:49:38AM +0200, Jakub Jelinek wrote:
> The --as-needed documentation doesn't talk about linking/not linking some
> library, it solely talks about DT_NEEDED:

I can't argue with that, especially since I wrote the documentation!
At least, not with the testcase you gave.  A different link situation,

  uses_bar.o --as-needed libbar.so libfoo.so --no-as-needed uses_foo.o

does change with your patch, from giving a link error to succeeding,
*and* results in a DT_NEEDED entry for libfoo.so in the main app.  So
there really is a behaviour change that disagrees with the doco.

That's really only a nitpick.  This comment of yours
> Because without it --as-needed isn't really usable with libtool
can't be ignored.  I don't want ld -as-needed less useful in practice.

So..  On to the patch itself.  Firstly, I don't see any need for version
checking a library specified on the command line, so reloading an
as-needed lib doesn't need to happen inside the "force" loop.  Secondly,
some small changes to the way these functions work will avoid
Yet Another Variable disease.  Something like the following looks
better to me.  Please check that I haven't made some stupid mistake, and
that this version works on real-world situations!

	* emultempl/elf32.em (global_found): Make it a pointer.
	(stat_needed, try_needed): Adjust.
	(check_needed): Don't skip non-loaded as-needed entries.
	(after_open): Try loading non-loaded as-needed libs to satisfy
	DT_NEEDED libs.

Index: ld/emultempl/elf32.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/elf32.em,v
retrieving revision 1.165
diff -u -p -r1.165 elf32.em
--- ld/emultempl/elf32.em	30 May 2006 16:45:32 -0000	1.165
+++ ld/emultempl/elf32.em	2 Jun 2006 11:56:37 -0000
@@ -148,7 +148,7 @@ cat >>e${EMULATION_NAME}.c <<EOF
 
 static struct bfd_link_needed_list *global_needed;
 static struct stat global_stat;
-static bfd_boolean global_found;
+static lang_input_statement_type *global_found;
 static struct bfd_link_needed_list *global_vercheck_needed;
 static bfd_boolean global_vercheck_failed;
 
@@ -229,12 +229,14 @@ gld${EMULATION_NAME}_stat_needed (lang_i
   const char *suffix;
   const char *soname;
 
-  if (global_found)
+  if (global_found != NULL)
     return;
   if (s->the_bfd == NULL)
     return;
-  if (s->as_needed
-      && (bfd_elf_get_dyn_lib_class (s->the_bfd) & DYN_AS_NEEDED) != 0)
+
+  /* If this input file was an as-needed entry, and wasn't found to be
+     needed at the stage it was linked, then don't say we have loaded it.  */
+  if ((bfd_elf_get_dyn_lib_class (s->the_bfd) & DYN_AS_NEEDED) != 0)
     return;
 
   if (bfd_stat (s->the_bfd, &st) != 0)
@@ -254,7 +256,7 @@ gld${EMULATION_NAME}_stat_needed (lang_i
       && st.st_ino == global_stat.st_ino
       && st.st_ino != 0)
     {
-      global_found = TRUE;
+      global_found = s;
       return;
     }
 
@@ -398,9 +400,9 @@ cat >>e${EMULATION_NAME}.c <<EOF
   if (trace_file_tries)
     info_msg (_("found %s at %s\n"), soname, name);
 
-  global_found = FALSE;
+  global_found = NULL;
   lang_for_each_input_file (gld${EMULATION_NAME}_stat_needed);
-  if (global_found)
+  if (global_found != NULL)
     {
       /* Return TRUE to indicate that we found the file, even though
 	 we aren't going to do anything with it.  */
@@ -809,14 +811,11 @@ cat >>e${EMULATION_NAME}.c <<EOF
 static void
 gld${EMULATION_NAME}_check_needed (lang_input_statement_type *s)
 {
-  if (global_found)
-    return;
-
-  /* If this input file was an as-needed entry, and wasn't found to be
-     needed at the stage it was linked, then don't say we have loaded it.  */
-  if (s->as_needed
-      && (s->the_bfd == NULL
-	  || (bfd_elf_get_dyn_lib_class (s->the_bfd) & DYN_AS_NEEDED) != 0))
+  /* Stop looking if we've found a loaded lib.  */
+  if (global_found != NULL
+      && global_found->the_bfd != NULL
+      && (bfd_elf_get_dyn_lib_class (global_found->the_bfd)
+	  & DYN_AS_NEEDED) == 0)
     return;
 
   if (s->filename != NULL)
@@ -825,7 +824,7 @@ gld${EMULATION_NAME}_check_needed (lang_
 
       if (strcmp (s->filename, global_needed->name) == 0)
 	{
-	  global_found = TRUE;
+	  global_found = s;
 	  return;
 	}
 
@@ -835,7 +834,7 @@ gld${EMULATION_NAME}_check_needed (lang_
 	  if (f != NULL
 	      && strcmp (f + 1, global_needed->name) == 0)
 	    {
-	      global_found = TRUE;
+	      global_found = s;
 	      return;
 	    }
 	}
@@ -849,7 +848,7 @@ gld${EMULATION_NAME}_check_needed (lang_
       if (soname != NULL
 	  && strcmp (soname, global_needed->name) == 0)
 	{
-	  global_found = TRUE;
+	  global_found = s;
 	  return;
 	}
     }
@@ -904,9 +903,12 @@ gld${EMULATION_NAME}_after_open (void)
 
       /* See if this file was included in the link explicitly.  */
       global_needed = l;
-      global_found = FALSE;
+      global_found = NULL;
       lang_for_each_input_file (gld${EMULATION_NAME}_check_needed);
-      if (global_found)
+      if (global_found != NULL
+	  && global_found->the_bfd != NULL
+	  && (bfd_elf_get_dyn_lib_class (global_found->the_bfd)
+	      & DYN_AS_NEEDED) == 0)
 	continue;
 
       n.by = l->by;
@@ -915,6 +917,13 @@ gld${EMULATION_NAME}_after_open (void)
       if (trace_file_tries)
 	info_msg (_("%s needed by %B\n"), l->name, l->by);
 
+      if (global_found != NULL)
+	{
+	  nn.name = global_found->filename;
+	  if (gld${EMULATION_NAME}_try_needed (&nn, TRUE))
+	    continue;
+	}
+
       /* We need to find this file and include the symbol table.  We
 	 want to search for the file in the same way that the dynamic
 	 linker will search.  That means that we want to use

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [PATCH] Fix DT_NEEDED search with --as-needed libraries (PR ld/2721)

[Jakub Jelinek]

On Fri, Jun 02, 2006 at 11:38:00PM +0930, Alan Modra wrote:
> So..  On to the patch itself.  Firstly, I don't see any need for version
> checking a library specified on the command line, so reloading an
> as-needed lib doesn't need to happen inside the "force" loop.  Secondly,
> some small changes to the way these functions work will avoid
> Yet Another Variable disease.  Something like the following looks
> better to me.  Please check that I haven't made some stupid mistake, and
> that this version works on real-world situations!

Yeah, surely looks better.

> @@ -229,12 +229,14 @@ gld${EMULATION_NAME}_stat_needed (lang_i
>    const char *suffix;
>    const char *soname;
>  
> -  if (global_found)
> +  if (global_found != NULL)
>      return;
>    if (s->the_bfd == NULL)
>      return;
> -  if (s->as_needed
> -      && (bfd_elf_get_dyn_lib_class (s->the_bfd) & DYN_AS_NEEDED) != 0)
> +
> +  /* If this input file was an as-needed entry, and wasn't found to be
> +     needed at the stage it was linked, then don't say we have loaded it.  */
> +  if ((bfd_elf_get_dyn_lib_class (s->the_bfd) & DYN_AS_NEEDED) != 0)
>      return;

Does (bfd_elf_get_dyn_lib_class (s->the_bfd) & DYN_AS_NEEDED) != 0 imply
s->as_needed?

> @@ -809,14 +811,11 @@ cat >>e${EMULATION_NAME}.c <<EOF
>  static void
>  gld${EMULATION_NAME}_check_needed (lang_input_statement_type *s)
>  {
> -  if (global_found)
> -    return;
> -
> -  /* If this input file was an as-needed entry, and wasn't found to be
> -     needed at the stage it was linked, then don't say we have loaded it.  */
> -  if (s->as_needed
> -      && (s->the_bfd == NULL
> -	  || (bfd_elf_get_dyn_lib_class (s->the_bfd) & DYN_AS_NEEDED) != 0))
> +  /* Stop looking if we've found a loaded lib.  */
> +  if (global_found != NULL
> +      && global_found->the_bfd != NULL
> +      && (bfd_elf_get_dyn_lib_class (global_found->the_bfd)
> +	  & DYN_AS_NEEDED) == 0)
>      return;
>  
>    if (s->filename != NULL)

Can we rely on at most one DYN_AS_NEEDED library matching?  Otherwise
it changes from first as needed library wins to last as needed library wins.
Also, if there is some ->the_bfd == NULL library, we'll keep searching, no
matter if it is global_found->as_needed or not.

> @@ -904,9 +903,12 @@ gld${EMULATION_NAME}_after_open (void)
>  
>        /* See if this file was included in the link explicitly.  */
>        global_needed = l;
> -      global_found = FALSE;
> +      global_found = NULL;
>        lang_for_each_input_file (gld${EMULATION_NAME}_check_needed);
> -      if (global_found)
> +      if (global_found != NULL
> +	  && global_found->the_bfd != NULL
> +	  && (bfd_elf_get_dyn_lib_class (global_found->the_bfd)
> +	      & DYN_AS_NEEDED) == 0)
>  	continue;

Similarly.

> @@ -915,6 +917,13 @@ gld${EMULATION_NAME}_after_open (void)
>        if (trace_file_tries)
>  	info_msg (_("%s needed by %B\n"), l->name, l->by);
>  
> +      if (global_found != NULL)
> +	{
> +	  nn.name = global_found->filename;
> +	  if (gld${EMULATION_NAME}_try_needed (&nn, TRUE))
> +	    continue;
> +	}

Can we rely on global_found->filename != NULL?

	Jakub

Re: [PATCH] Fix DT_NEEDED search with --as-needed libraries (PR ld/2721)

[Alan Modra]

On Fri, Jun 02, 2006 at 04:50:54PM +0200, Jakub Jelinek wrote:
> Does (bfd_elf_get_dyn_lib_class (s->the_bfd) & DYN_AS_NEEDED) != 0 imply
> s->as_needed?

Yes.

> Can we rely on at most one DYN_AS_NEEDED library matching?  Otherwise
> it changes from first as needed library wins to last as needed library wins.

Sigh.  I suppose we might have two matching as-needed libs.  That needs
fixing.

> Also, if there is some ->the_bfd == NULL library, we'll keep searching, no
> matter if it is global_found->as_needed or not.

Hmm, to get global_found->the_bfd NULL, you'd need to specify a linker
script on the command line that looked like a soname.  ie.  You've found
a bug in check_needed.  It ought to only look at entries with both
s->filename and s->the_bfd, I think.

> Can we rely on global_found->filename != NULL?

Yes.

This is getting way too late for me to post patches, but here goes.

	* emultempl/elf32.em (global_found): Make it a pointer.
	(stat_needed, try_needed): Adjust.
	(check_needed): Don't skip non-loaded as-needed entries.  Only
	consider entries with both filename and the_bfd non-null.
	(after_open): Try loading non-loaded as-needed libs to satisfy
	DT_NEEDED libs.

Index: ld/emultempl/elf32.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/elf32.em,v
retrieving revision 1.165
diff -u -p -r1.165 elf32.em
--- ld/emultempl/elf32.em	30 May 2006 16:45:32 -0000	1.165
+++ ld/emultempl/elf32.em	2 Jun 2006 16:20:18 -0000
@@ -148,7 +148,7 @@ cat >>e${EMULATION_NAME}.c <<EOF
 
 static struct bfd_link_needed_list *global_needed;
 static struct stat global_stat;
-static bfd_boolean global_found;
+static lang_input_statement_type *global_found;
 static struct bfd_link_needed_list *global_vercheck_needed;
 static bfd_boolean global_vercheck_failed;
 
@@ -229,12 +229,14 @@ gld${EMULATION_NAME}_stat_needed (lang_i
   const char *suffix;
   const char *soname;
 
-  if (global_found)
+  if (global_found != NULL)
     return;
   if (s->the_bfd == NULL)
     return;
-  if (s->as_needed
-      && (bfd_elf_get_dyn_lib_class (s->the_bfd) & DYN_AS_NEEDED) != 0)
+
+  /* If this input file was an as-needed entry, and wasn't found to be
+     needed at the stage it was linked, then don't say we have loaded it.  */
+  if ((bfd_elf_get_dyn_lib_class (s->the_bfd) & DYN_AS_NEEDED) != 0)
     return;
 
   if (bfd_stat (s->the_bfd, &st) != 0)
@@ -254,7 +256,7 @@ gld${EMULATION_NAME}_stat_needed (lang_i
       && st.st_ino == global_stat.st_ino
       && st.st_ino != 0)
     {
-      global_found = TRUE;
+      global_found = s;
       return;
     }
 
@@ -398,9 +400,9 @@ cat >>e${EMULATION_NAME}.c <<EOF
   if (trace_file_tries)
     info_msg (_("found %s at %s\n"), soname, name);
 
-  global_found = FALSE;
+  global_found = NULL;
   lang_for_each_input_file (gld${EMULATION_NAME}_stat_needed);
-  if (global_found)
+  if (global_found != NULL)
     {
       /* Return TRUE to indicate that we found the file, even though
 	 we aren't going to do anything with it.  */
@@ -809,49 +811,45 @@ cat >>e${EMULATION_NAME}.c <<EOF
 static void
 gld${EMULATION_NAME}_check_needed (lang_input_statement_type *s)
 {
-  if (global_found)
+  const char *soname;
+
+  /* Stop looking if we've found a loaded lib.  */
+  if (global_found != NULL
+      && (bfd_elf_get_dyn_lib_class (global_found->the_bfd)
+	  & DYN_AS_NEEDED) == 0)
     return;
 
-  /* If this input file was an as-needed entry, and wasn't found to be
-     needed at the stage it was linked, then don't say we have loaded it.  */
-  if (s->as_needed
-      && (s->the_bfd == NULL
-	  || (bfd_elf_get_dyn_lib_class (s->the_bfd) & DYN_AS_NEEDED) != 0))
+  if (s->filename == NULL || s->the_bfd == NULL)
+    return;
+
+  /* Don't look for a second non-loaded as-needed lib.  */
+  if (global_found != NULL
+      && (bfd_elf_get_dyn_lib_class (s->the_bfd) & DYN_AS_NEEDED) != 0)
     return;
 
-  if (s->filename != NULL)
+  if (strcmp (s->filename, global_needed->name) == 0)
     {
-      const char *f;
+      global_found = s;
+      return;
+    }
 
-      if (strcmp (s->filename, global_needed->name) == 0)
+  if (s->search_dirs_flag)
+    {
+      const char *f = strrchr (s->filename, '/');
+      if (f != NULL
+	  && strcmp (f + 1, global_needed->name) == 0)
 	{
-	  global_found = TRUE;
+	  global_found = s;
 	  return;
 	}
-
-      if (s->search_dirs_flag)
-	{
-	  f = strrchr (s->filename, '/');
-	  if (f != NULL
-	      && strcmp (f + 1, global_needed->name) == 0)
-	    {
-	      global_found = TRUE;
-	      return;
-	    }
-	}
     }
 
-  if (s->the_bfd != NULL)
+  soname = bfd_elf_get_dt_soname (s->the_bfd);
+  if (soname != NULL
+      && strcmp (soname, global_needed->name) == 0)
     {
-      const char *soname;
-
-      soname = bfd_elf_get_dt_soname (s->the_bfd);
-      if (soname != NULL
-	  && strcmp (soname, global_needed->name) == 0)
-	{
-	  global_found = TRUE;
-	  return;
-	}
+      global_found = s;
+      return;
     }
 }
 
@@ -904,9 +902,11 @@ gld${EMULATION_NAME}_after_open (void)
 
       /* See if this file was included in the link explicitly.  */
       global_needed = l;
-      global_found = FALSE;
+      global_found = NULL;
       lang_for_each_input_file (gld${EMULATION_NAME}_check_needed);
-      if (global_found)
+      if (global_found != NULL
+	  && (bfd_elf_get_dyn_lib_class (global_found->the_bfd)
+	      & DYN_AS_NEEDED) == 0)
 	continue;
 
       n.by = l->by;
@@ -915,6 +915,13 @@ gld${EMULATION_NAME}_after_open (void)
       if (trace_file_tries)
 	info_msg (_("%s needed by %B\n"), l->name, l->by);
 
+      if (global_found != NULL)
+	{
+	  nn.name = global_found->filename;
+	  if (gld${EMULATION_NAME}_try_needed (&nn, TRUE))
+	    continue;
+	}
+
       /* We need to find this file and include the symbol table.  We
 	 want to search for the file in the same way that the dynamic
 	 linker will search.  That means that we want to use

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [PATCH] Fix DT_NEEDED search with --as-needed libraries (PR ld/2721)

[Alan Modra]

On Sat, Jun 03, 2006 at 01:55:44AM +0930, Alan Modra wrote:
> 	* emultempl/elf32.em (global_found): Make it a pointer.
> 	(stat_needed, try_needed): Adjust.
> 	(check_needed): Don't skip non-loaded as-needed entries.  Only
> 	consider entries with both filename and the_bfd non-null.
> 	(after_open): Try loading non-loaded as-needed libs to satisfy
> 	DT_NEEDED libs.

Committed with an extra comment.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [PATCH] Fix DT_NEEDED search with --as-needed libraries (PR ld/2721)

[Daniel Jacobowitz]

On Sat, Jun 03, 2006 at 12:34:33PM +0930, Alan Modra wrote:
> On Sat, Jun 03, 2006 at 01:55:44AM +0930, Alan Modra wrote:
> > 	* emultempl/elf32.em (global_found): Make it a pointer.
> > 	(stat_needed, try_needed): Adjust.
> > 	(check_needed): Don't skip non-loaded as-needed entries.  Only
> > 	consider entries with both filename and the_bfd non-null.
> > 	(after_open): Try loading non-loaded as-needed libs to satisfy
> > 	DT_NEEDED libs.
> 
> Committed with an extra comment.

So: still want this included in 2.17?  If so, please commit it there,
or let me know and I will.

Does anyone think this justifies another prerelease?  I'm not planning
to; I think we're past the point of diminishing returns in terms of
finding testers.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Fix DT_NEEDED search with --as-needed libraries (PR ld/2721)

[Alan Modra]

On Wed, Jun 07, 2006 at 11:13:50AM -0400, Daniel Jacobowitz wrote:
> On Sat, Jun 03, 2006 at 12:34:33PM +0930, Alan Modra wrote:
> > On Sat, Jun 03, 2006 at 01:55:44AM +0930, Alan Modra wrote:
> > > 	* emultempl/elf32.em (global_found): Make it a pointer.
> > > 	(stat_needed, try_needed): Adjust.
> > > 	(check_needed): Don't skip non-loaded as-needed entries.  Only
> > > 	consider entries with both filename and the_bfd non-null.
> > > 	(after_open): Try loading non-loaded as-needed libs to satisfy
> > > 	DT_NEEDED libs.
> > 
> > Committed with an extra comment.
> 
> So: still want this included in 2.17?  If so, please commit it there,
> or let me know and I will.

Committed.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre