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

[Alan Modra <amodra at bigpond dot net dot au>]

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