[PATCH] Fix DT_NEEDED when using -l:namespec

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Fix DT_NEEDED when using -l:namespec

Romain Geissler
Hi,

There is a behavior difference between ld (elf 64) and gold when specifying a lib with -l:mylib.so that is not the current working directory.
If y have a lib without DT_SONAME at path lib/liblib.so and if y try to link with both linkers using this command: g++ -o test test.cpp -Ilib -Llib -l:liblib.so

 - with ld.bfd:
readelf -d test|grep "liblib\.so"
 0x0000000000000001 (NEEDED)             Shared library: [lib/liblib.so]  

 - with ld.gold:
readelf -d test|grep "liblib\.so"  
 0x0000000000000001 (NEEDED)             Shared library: [liblib.so]  

In the first case, the search path is prepended to the DT_NEEDED entry. I think it’s a bug from ld.bfd, when we use -l flag, we never want the search path to be present when the lib has no SONAME.

This patch solves this issue, tested on elf x86_64 SLES 11 SP1 without regression.

ld/
2014-02-16  Romain Geissler  <[hidden email]>

        * ldlang.h (full_name_provided): New input flag.
        * ldlang.c (new_afile): Set full_name_provided flag.
        * ldlfile.c (ldfile_open_file_search): Don't complete lib name if
        full_name_provided flag is set.
        * emultempl/elf32.em (gld${EMULATION_NAME}_open_dynamic_archive):
        Correctly set DT_NEEDED for dynamic libraries with loaded with
        -l:namespec.
        * emultempl/aix.em (ppc_after_open_output): Don't try to load lib specified
        with -l:namespec.
        * emultempl/linux.em (gld${EMULATION_NAME}_open_dynamic_archive): Likewise.
        * emultempl/pe.em (gld${EMULATION_NAME}_open_dynamic_archive): Likewise.
        * emultempl/pep.em (gld${EMULATION_NAME}_open_dynamic_archive): Likewise.
        * emultempl/vms.em (gld${EMULATION_NAME}_open_dynamic_archive): Likewise.

---
 ld/emultempl/aix.em   |  2 +-
 ld/emultempl/elf32.em | 61 ++++++++++++++++++++++++++++++++-------------------
 ld/emultempl/linux.em |  2 +-
 ld/emultempl/pe.em    |  2 +-
 ld/emultempl/pep.em   |  2 +-
 ld/emultempl/vms.em   |  2 +-
 ld/ldfile.c           |  2 +-
 ld/ldlang.c           | 15 ++++++-------
 ld/ldlang.h           |  3 +++
 9 files changed, 55 insertions(+), 36 deletions(-)

diff --git a/ld/emultempl/aix.em b/ld/emultempl/aix.em
index d080133..4919ae6 100644
--- a/ld/emultempl/aix.em
+++ b/ld/emultempl/aix.em
@@ -1506,7 +1506,7 @@ gld${EMULATION_NAME}_open_dynamic_archive (const char *arch,
 {
   char *path;
 
-  if (!entry->flags.maybe_archive)
+  if (!entry->flags.maybe_archive || entry->flags.full_name_provided)
     return FALSE;
 
   path = concat (search->name, "/lib", entry->filename, arch, ".a", NULL);
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index fda0e68..89ad0d0 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -1664,36 +1664,52 @@ gld${EMULATION_NAME}_open_dynamic_archive
 
   filename = entry->filename;
 
-  /* This allocates a few bytes too many when EXTRA_SHLIB_EXTENSION
-     is defined, but it does not seem worth the headache to optimize
-     away those two bytes of space.  */
-  string = (char *) xmalloc (strlen (search->name)
-     + strlen (filename)
-     + strlen (arch)
+  if (entry->flags.full_name_provided)
+    {
+      string = (char *) xmalloc (strlen (search->name) + strlen (filename));
+      sprintf (string, "%s/%s", search->name, filename);
+
+      if (! (ldfile_try_open_bfd (string, entry)
+             && bfd_check_format (entry->the_bfd, bfd_object)
+             && (entry->the_bfd->flags & DYNAMIC) != 0))
+        {
+          free (string);
+          return FALSE;
+        }
+    }
+  else
+    {
+      /* This allocates a few bytes too many when EXTRA_SHLIB_EXTENSION
+         is defined, but it does not seem worth the headache to optimize
+         away those two bytes of space.  */
+      string = (char *) xmalloc (strlen (search->name)
+                                + strlen (filename)
+                                + strlen (arch)
 #ifdef EXTRA_SHLIB_EXTENSION
-     + strlen (EXTRA_SHLIB_EXTENSION)
+                                + strlen (EXTRA_SHLIB_EXTENSION)
 #endif
-     + sizeof "/lib.so");
+                                + sizeof "/lib.so");
 
-  sprintf (string, "%s/lib%s%s.so", search->name, filename, arch);
+      sprintf (string, "%s/lib%s%s.so", search->name, filename, arch);
 
 #ifdef EXTRA_SHLIB_EXTENSION
-  /* Try the .so extension first.  If that fails build a new filename
-     using EXTRA_SHLIB_EXTENSION.  */
-  if (! ldfile_try_open_bfd (string, entry))
-    {
-      sprintf (string, "%s/lib%s%s%s", search->name,
-       filename, arch, EXTRA_SHLIB_EXTENSION);
+      /* Try the .so extension first.  If that fails build a new filename
+         using EXTRA_SHLIB_EXTENSION.  */
+      if (! ldfile_try_open_bfd (string, entry))
+        {
+          sprintf (string, "%s/lib%s%s%s", search->name,
+               filename, arch, EXTRA_SHLIB_EXTENSION);
 #endif
 
-  if (! ldfile_try_open_bfd (string, entry))
-    {
-      free (string);
-      return FALSE;
-    }
+      if (! ldfile_try_open_bfd (string, entry))
+        {
+          free (string);
+          return FALSE;
+        }
 #ifdef EXTRA_SHLIB_EXTENSION
-    }
+        }
 #endif
+    }
 
   entry->filename = string;
 
@@ -1718,7 +1734,8 @@ gld${EMULATION_NAME}_open_dynamic_archive
       /* Rather than duplicating the logic above.  Just use the
  filename we recorded earlier.  */
 
-      filename = lbasename (entry->filename);
+      if (!entry->flags.full_name_provided)
+        filename = lbasename (entry->filename);
       bfd_elf_set_dt_needed_name (entry->the_bfd, filename);
     }
 
diff --git a/ld/emultempl/linux.em b/ld/emultempl/linux.em
index 5cf5bfa..d9571ab 100644
--- a/ld/emultempl/linux.em
+++ b/ld/emultempl/linux.em
@@ -62,7 +62,7 @@ gld${EMULATION_NAME}_open_dynamic_archive
 {
   char *string;
 
-  if (! entry->flags.maybe_archive)
+  if (! entry->flags.maybe_archive || entry->flags.full_name_provided)
     return FALSE;
 
   string = (char *) xmalloc (strlen (search->name)
diff --git a/ld/emultempl/pe.em b/ld/emultempl/pe.em
index 5d6da9e..a85c8a3 100644
--- a/ld/emultempl/pe.em
+++ b/ld/emultempl/pe.em
@@ -2108,7 +2108,7 @@ gld_${EMULATION_NAME}_open_dynamic_archive
   unsigned int i;
 
 
-  if (! entry->flags.maybe_archive)
+  if (! entry->flags.maybe_archive || entry->flags.full_name_provided)
     return FALSE;
 
   filename = entry->filename;
diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em
index b738800..70469db 100644
--- a/ld/emultempl/pep.em
+++ b/ld/emultempl/pep.em
@@ -1879,7 +1879,7 @@ gld_${EMULATION_NAME}_open_dynamic_archive
   unsigned int i;
 
 
-  if (! entry->flags.maybe_archive)
+  if (! entry->flags.maybe_archive || entry->flags.full_name_provided)
     return FALSE;
 
   filename = entry->filename;
diff --git a/ld/emultempl/vms.em b/ld/emultempl/vms.em
index 30c1a16..6682208 100644
--- a/ld/emultempl/vms.em
+++ b/ld/emultempl/vms.em
@@ -58,7 +58,7 @@ gld${EMULATION_NAME}_open_dynamic_archive (const char *arch ATTRIBUTE_UNUSED,
 {
   char *string;
 
-  if (! entry->flags.maybe_archive)
+  if (! entry->flags.maybe_archive || entry->flags.full_name_provided)
     return FALSE;
 
   string = (char *) xmalloc (strlen (search->name)
diff --git a/ld/ldfile.c b/ld/ldfile.c
index 16baef8..0048183 100644
--- a/ld/ldfile.c
+++ b/ld/ldfile.c
@@ -369,7 +369,7 @@ ldfile_open_file_search (const char *arch,
     return TRUE;
  }
 
-      if (entry->flags.maybe_archive)
+      if (entry->flags.maybe_archive && !entry->flags.full_name_provided)
  string = concat (search->name, slash, lib, entry->filename,
  arch, suffix, (const char *) NULL);
       else
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 4768af7..e4a595c 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -1063,13 +1063,6 @@ new_afile (const char *name,
   p->flags.whole_archive = input_flags.whole_archive;
   p->flags.sysrooted = input_flags.sysrooted;
 
-  if (file_type == lang_input_file_is_l_enum
-      && name[0] == ':' && name[1] != '\0')
-    {
-      file_type = lang_input_file_is_search_file_enum;
-      name = name + 1;
-    }
-
   switch (file_type)
     {
     case lang_input_file_is_symbols_only_enum:
@@ -1083,7 +1076,13 @@ new_afile (const char *name,
       p->local_sym_name = name;
       break;
     case lang_input_file_is_l_enum:
-      p->filename = name;
+      if (name[0] == ':' && name[1] != '\0')
+        {
+          p->filename = name + 1;
+          p->flags.full_name_provided = TRUE;
+        }
+      else
+        p->filename = name;
       p->local_sym_name = concat ("-l", name, (const char *) NULL);
       p->flags.maybe_archive = TRUE;
       p->flags.real = TRUE;
diff --git a/ld/ldlang.h b/ld/ldlang.h
index 7236c1c..b4624d8 100644
--- a/ld/ldlang.h
+++ b/ld/ldlang.h
@@ -235,6 +235,9 @@ struct lang_input_statement_flags
   /* 1 means this file was specified in a -l option.  */
   unsigned int maybe_archive : 1;
 
+  /* 1 means this file was specified in a -l:namespec option.  */
+  unsigned int full_name_provided : 1;
+
   /* 1 means search a set of directories for this file.  */
   unsigned int search_dirs : 1;
 
--

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix DT_NEEDED when using -l:namespec

Alan Modra-3
On Sun, Feb 16, 2014 at 10:02:56PM +0100, Romain Geissler wrote:

> * ldlang.h (full_name_provided): New input flag.
> * ldlang.c (new_afile): Set full_name_provided flag.
> * ldlfile.c (ldfile_open_file_search): Don't complete lib name if
> full_name_provided flag is set.
> * emultempl/elf32.em (gld${EMULATION_NAME}_open_dynamic_archive):
> Correctly set DT_NEEDED for dynamic libraries with loaded with
> -l:namespec.
> * emultempl/aix.em (ppc_after_open_output): Don't try to load lib specified
> with -l:namespec.
> * emultempl/linux.em (gld${EMULATION_NAME}_open_dynamic_archive): Likewise.
> * emultempl/pe.em (gld${EMULATION_NAME}_open_dynamic_archive): Likewise.
> * emultempl/pep.em (gld${EMULATION_NAME}_open_dynamic_archive): Likewise.
> * emultempl/vms.em (gld${EMULATION_NAME}_open_dynamic_archive): Likewise.

OK, makes sense to have -l:libfoo.so give the same DT_NEEDED as -lfoo.

--
Alan Modra
Australia Development Lab, IBM
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix DT_NEEDED when using -l:namespec

Romain Geissler
Le 13 mars 2014 à 14:23, Alan Modra <[hidden email]> a écrit :

> OK, makes sense to have -l:libfoo.so give the same DT_NEEDED as -lfoo.

Thanks. Can you please integrate it, I have no write permission on the official repository.

Cheers,
Romain

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix DT_NEEDED when using -l:namespec

Alan Modra-3
On Thu, Mar 13, 2014 at 03:08:56PM +0100, Romain Geissler wrote:
> Le 13 mars 2014 à 14:23, Alan Modra <[hidden email]> a écrit :
>
> > OK, makes sense to have -l:libfoo.so give the same DT_NEEDED as -lfoo.
>
> Thanks. Can you please integrate it, I have no write permission on the official repository.

This is the patch I committed.  Changes from your patch are:

- aix.em open_dynamic_archive handles -l:namespec libs, so the import
  path is correctly set.

- elf32.em open_dynamic_archive EXTRA_SHLIB_EXTENSION code tidied,
  and a couple of bugs in your patch fixed.  xmalloc size was wrong
  for full_name_provided, and we want open_dynamic_archive to return
  true whenever ldfile_try_open_bfd succeeds, not return false and
  free the string with a reference remaining in the bfd.


diff --git a/ld/ChangeLog b/ld/ChangeLog
index cc5f828..9c728ae 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,22 @@
+2014-03-14  Romain Geissler  <[hidden email]>
+    Alan Modra  <[hidden email]>
+
+ * ldlang.h (full_name_provided): New input flag.
+ * ldlang.c (new_afile): Don't use lang_input_file_is_search_file_enum
+ for -l:namespec.  Instead use lang_input_file_is_l_enum with
+ full_name_provided flag.
+ * ldlfile.c (ldfile_open_file_search): Don't complete lib name if
+ full_name_provided flag is set.
+ * emultempl/elf32.em (gld${EMULATION_NAME}_open_dynamic_archive):
+ Handle full_name_provided libraries.  Tidy EXTRA_SHLIB_EXTENSION
+ support.  Set DT_NEEDED for -l:namespec as namespec.
+ * emultempl/aix.em (ppc_after_open_output): Handle full_name_provided.
+ * emultempl/linux.em (gld${EMULATION_NAME}_open_dynamic_archive):
+ Don't handle full_name_provided libraries.
+ * emultempl/pe.em (gld${EMULATION_NAME}_open_dynamic_archive): Ditto.
+ * emultempl/pep.em (gld${EMULATION_NAME}_open_dynamic_archive): Ditto.
+ * emultempl/vms.em (gld${EMULATION_NAME}_open_dynamic_archive): Ditto.
+
 2014-03-12  Alan Modra  <[hidden email]>
 
  * Makefile.in: Regenerate.
diff --git a/ld/emultempl/aix.em b/ld/emultempl/aix.em
index 4a06c71..caa74a9 100644
--- a/ld/emultempl/aix.em
+++ b/ld/emultempl/aix.em
@@ -1509,7 +1509,13 @@ gld${EMULATION_NAME}_open_dynamic_archive (const char *arch,
   if (!entry->flags.maybe_archive)
     return FALSE;
 
-  path = concat (search->name, "/lib", entry->filename, arch, ".a", NULL);
+  if (entry->flags.full_name_provided)
+    path = concat (search->name, "/", entry->filename,
+   (const char *) NULL);
+  else
+    path = concat (search->name, "/lib", entry->filename, arch, ".a",
+   (const char *) NULL);
+
   if (!ldfile_try_open_bfd (path, entry))
     {
       free (path);
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index 789e12c..7ea5adc 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -1656,42 +1656,46 @@ gld${EMULATION_NAME}_open_dynamic_archive
 {
   const char *filename;
   char *string;
+  size_t len;
+  bfd_boolean opened = FALSE;
 
   if (! entry->flags.maybe_archive)
     return FALSE;
 
   filename = entry->filename;
+  len = strlen (search->name) + strlen (filename);
+  if (entry->flags.full_name_provided)
+    {
+      len += sizeof "/";
+      string = (char *) xmalloc (len);
+      sprintf (string, "%s/%s", search->name, filename);
+    }
+  else
+    {
+      size_t xlen = 0;
 
-  /* This allocates a few bytes too many when EXTRA_SHLIB_EXTENSION
-     is defined, but it does not seem worth the headache to optimize
-     away those two bytes of space.  */
-  string = (char *) xmalloc (strlen (search->name)
-     + strlen (filename)
-     + strlen (arch)
+      len += strlen (arch) + sizeof "/lib.so";
 #ifdef EXTRA_SHLIB_EXTENSION
-     + strlen (EXTRA_SHLIB_EXTENSION)
+      xlen = (strlen (EXTRA_SHLIB_EXTENSION) > 3
+      ? strlen (EXTRA_SHLIB_EXTENSION) - 3
+      : 0);
 #endif
-     + sizeof "/lib.so");
-
-  sprintf (string, "%s/lib%s%s.so", search->name, filename, arch);
-
+      string = (char *) xmalloc (len + xlen);
+      sprintf (string, "%s/lib%s%s.so", search->name, filename, arch);
 #ifdef EXTRA_SHLIB_EXTENSION
-  /* Try the .so extension first.  If that fails build a new filename
-     using EXTRA_SHLIB_EXTENSION.  */
-  if (! ldfile_try_open_bfd (string, entry))
-    {
-      sprintf (string, "%s/lib%s%s%s", search->name,
-       filename, arch, EXTRA_SHLIB_EXTENSION);
+      /* Try the .so extension first.  If that fails build a new filename
+ using EXTRA_SHLIB_EXTENSION.  */
+      opened = ldfile_try_open_bfd (string, entry);
+      if (!opened)
+ strcpy (string + len - 4, EXTRA_SHLIB_EXTENSION);
 #endif
+    }
 
-  if (! ldfile_try_open_bfd (string, entry))
+  if (!opened && !ldfile_try_open_bfd (string, entry))
     {
       free (string);
       return FALSE;
     }
-#ifdef EXTRA_SHLIB_EXTENSION
-    }
-#endif
 
   entry->filename = string;
 
@@ -1716,7 +1720,8 @@ gld${EMULATION_NAME}_open_dynamic_archive
       /* Rather than duplicating the logic above.  Just use the
  filename we recorded earlier.  */
 
-      filename = lbasename (entry->filename);
+      if (!entry->flags.full_name_provided)
+        filename = lbasename (entry->filename);
       bfd_elf_set_dt_needed_name (entry->the_bfd, filename);
     }
 
diff --git a/ld/emultempl/linux.em b/ld/emultempl/linux.em
index e7fa35d..b30e872 100644
--- a/ld/emultempl/linux.em
+++ b/ld/emultempl/linux.em
@@ -61,7 +61,7 @@ gld${EMULATION_NAME}_open_dynamic_archive
 {
   char *string;
 
-  if (! entry->flags.maybe_archive)
+  if (! entry->flags.maybe_archive || entry->flags.full_name_provided)
     return FALSE;
 
   string = (char *) xmalloc (strlen (search->name)
diff --git a/ld/emultempl/pe.em b/ld/emultempl/pe.em
index 6de71bd..67df2bc 100644
--- a/ld/emultempl/pe.em
+++ b/ld/emultempl/pe.em
@@ -2108,7 +2108,7 @@ gld_${EMULATION_NAME}_open_dynamic_archive
   unsigned int i;
 
 
-  if (! entry->flags.maybe_archive)
+  if (! entry->flags.maybe_archive || entry->flags.full_name_provided)
     return FALSE;
 
   filename = entry->filename;
diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em
index 9309c11..dca36cc 100644
--- a/ld/emultempl/pep.em
+++ b/ld/emultempl/pep.em
@@ -1879,7 +1879,7 @@ gld_${EMULATION_NAME}_open_dynamic_archive
   unsigned int i;
 
 
-  if (! entry->flags.maybe_archive)
+  if (! entry->flags.maybe_archive || entry->flags.full_name_provided)
     return FALSE;
 
   filename = entry->filename;
diff --git a/ld/emultempl/vms.em b/ld/emultempl/vms.em
index 08650f5..b1f61d5 100644
--- a/ld/emultempl/vms.em
+++ b/ld/emultempl/vms.em
@@ -57,7 +57,7 @@ gld${EMULATION_NAME}_open_dynamic_archive (const char *arch ATTRIBUTE_UNUSED,
 {
   char *string;
 
-  if (! entry->flags.maybe_archive)
+  if (! entry->flags.maybe_archive || entry->flags.full_name_provided)
     return FALSE;
 
   string = (char *) xmalloc (strlen (search->name)
diff --git a/ld/ldfile.c b/ld/ldfile.c
index 61660ab..782ed7f 100644
--- a/ld/ldfile.c
+++ b/ld/ldfile.c
@@ -367,7 +367,7 @@ ldfile_open_file_search (const char *arch,
     return TRUE;
  }
 
-      if (entry->flags.maybe_archive)
+      if (entry->flags.maybe_archive && !entry->flags.full_name_provided)
  string = concat (search->name, slash, lib, entry->filename,
  arch, suffix, (const char *) NULL);
       else
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 57e2ee8..37ef265 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -1063,13 +1063,6 @@ new_afile (const char *name,
   p->flags.whole_archive = input_flags.whole_archive;
   p->flags.sysrooted = input_flags.sysrooted;
 
-  if (file_type == lang_input_file_is_l_enum
-      && name[0] == ':' && name[1] != '\0')
-    {
-      file_type = lang_input_file_is_search_file_enum;
-      name = name + 1;
-    }
-
   switch (file_type)
     {
     case lang_input_file_is_symbols_only_enum:
@@ -1083,7 +1076,13 @@ new_afile (const char *name,
       p->local_sym_name = name;
       break;
     case lang_input_file_is_l_enum:
-      p->filename = name;
+      if (name[0] == ':' && name[1] != '\0')
+        {
+          p->filename = name + 1;
+          p->flags.full_name_provided = TRUE;
+        }
+      else
+        p->filename = name;
       p->local_sym_name = concat ("-l", name, (const char *) NULL);
       p->flags.maybe_archive = TRUE;
       p->flags.real = TRUE;
diff --git a/ld/ldlang.h b/ld/ldlang.h
index 8f525d1..aacd5dc 100644
--- a/ld/ldlang.h
+++ b/ld/ldlang.h
@@ -235,6 +235,9 @@ struct lang_input_statement_flags
   /* 1 means this file was specified in a -l option.  */
   unsigned int maybe_archive : 1;
 
+  /* 1 means this file was specified in a -l:namespec option.  */
+  unsigned int full_name_provided : 1;
+
   /* 1 means search a set of directories for this file.  */
   unsigned int search_dirs : 1;
 
 
--
Alan Modra
Australia Development Lab, IBM