dependency list for static libraries

dependency list for static libraries

[Howard Chu]

We often get link errors when switching from dynamic linking to static 
linking, because dynamic libraries list all of their own dependency libraries, 
and static libraries don't. (And the program being linked doesn't know about 
those indirect dependencies.)

It Would Be Nice if we could add a dependency list to a static library too. 
Much like ranlib creates an index for a static library already, store the list 
of dependencies in a reserved module.

It seems to me I've made this suggestion before but Google and my memory have 
failed to locate it. We'd need either a new tool like ranlib, or a new flag to 
ar to store a list of dependencies, and then linker support to lookup the 
dependency list whenever referencing a static library.
-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: dependency list for static libraries

[Simon Richter]

[-- Attachment #1.1: Type: text/plain, Size: 454 bytes --]

Hi,

On 19.09.2017 17:49, Howard Chu wrote:

> We often get link errors when switching from dynamic linking to static
> linking, because dynamic libraries list all of their own dependency
> libraries, and static libraries don't. (And the program being linked
> doesn't know about those indirect dependencies.)

That is what libtool's .la files do — these have a list of extra
dependencies to be linked when linking statically.

   Simon


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: dependency list for static libraries

[Howard Chu]

Simon Richter wrote:
> Hi,
> 
> On 19.09.2017 17:49, Howard Chu wrote:
> 
>> We often get link errors when switching from dynamic linking to static
>> linking, because dynamic libraries list all of their own dependency
>> libraries, and static libraries don't. (And the program being linked
>> doesn't know about those indirect dependencies.)
> 
> That is what libtool's .la files do — these have a list of extra
> dependencies to be linked when linking statically.

Libtool files are a poor solution and an additional admin overhead. Not every 
project uses libtool. Indeed, we only use it when we're creating shared 
libraries; for a project that only uses static archives nobody bothers with 
it. This functionality rightfully belongs in the linker itself.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: dependency list for static libraries

[R0b0t1]

On Tue, Sep 19, 2017 at 11:03 AM, Howard Chu <hyc@symas.com> wrote:
> Simon Richter wrote:
>>
>> Hi,
>>
>> On 19.09.2017 17:49, Howard Chu wrote:
>>
>>> We often get link errors when switching from dynamic linking to static
>>> linking, because dynamic libraries list all of their own dependency
>>> libraries, and static libraries don't. (And the program being linked
>>> doesn't know about those indirect dependencies.)
>>
>>
>> That is what libtool's .la files do — these have a list of extra
>> dependencies to be linked when linking statically.
>
>
> Libtool files are a poor solution and an additional admin overhead.

To add to this, though it may not be extremely relevant, there are a
lot of automake tests that seem to be broken for unknown reasons
during cross-compilation. Binutils itself doesn't use any broken tests
(last I checked), but if one tries to cross-compile an entire
GNU/Linux distribution it is likely that the problem will manifest
itself.

It seems like the best way to avoid this is to avoid using the autotools.

Cheers,
     R0b0t1

Re: dependency list for static libraries

[Joseph Myers]

You can install a linker script under a .a file name to achieve this 
effect.  On x86_64, glibc's libm.a now looks like:

/* GNU ld script
*/
OUTPUT_FORMAT(elf64-x86-64)
GROUP ( /usr/lib64/libm-2.26.90.a /usr/lib64/libmvec.a )

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: dependency list for static libraries

[Howard Chu]

Joseph Myers wrote:
> You can install a linker script under a .a file name to achieve this
> effect.  On x86_64, glibc's libm.a now looks like:
> 
> /* GNU ld script
> */
> OUTPUT_FORMAT(elf64-x86-64)
> GROUP ( /usr/lib64/libm-2.26.90.a /usr/lib64/libmvec.a )

Thanks for the suggestion, but that still means introducing 
additional/auxiliary files, so it has much the same drawback as libtool files. 
I also find this particularly gross because it breaks the principle of least 
surprise; when I see a *.a file I expect ar, nm, and ranlib to work on it. I'm 
kind of shocked that such a solution ever flew.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: dependency list for static libraries

[Joseph Myers]

On Tue, 19 Sep 2017, Howard Chu wrote:

> Thanks for the suggestion, but that still means introducing
> additional/auxiliary files, so it has much the same drawback as libtool files.

But it works with the linker itself, without requiring an external tool.

> I also find this particularly gross because it breaks the principle of least
> surprise; when I see a *.a file I expect ar, nm, and ranlib to work on it. I'm
> kind of shocked that such a solution ever flew.

It's just doing what glibc has done with libc.so (a linker script) for a 
very long time (decades?).  And libieee.a (removed in glibc 2.27) is/was a 
single object file, not an archive, because archive semantics wouldn't 
have been right for it (the object needed to be linked in with -lieee, 
regardless of whether there were undefined references to any symbol from 
it).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: dependency list for static libraries

[Howard Chu]

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

Howard Chu wrote:
> Joseph Myers wrote:
>> You can install a linker script under a .a file name to achieve this
>> effect.  On x86_64, glibc's libm.a now looks like:
>>
>> /* GNU ld script
>> */
>> OUTPUT_FORMAT(elf64-x86-64)
>> GROUP ( /usr/lib64/libm-2.26.90.a /usr/lib64/libmvec.a )
> 
> Thanks for the suggestion, but that still means introducing additional/auxiliary files, so it has much the same drawback as libtool files. I also find this
> particularly gross because it breaks the principle of least surprise; when I see a *.a file I expect ar, nm, and ranlib to work on it. I'm kind of shocked that
> such a solution ever flew.
> 
Here's a proposed patch to ar to implement the first half of the solution:
storing the dependencies into a static library. It looks like I may be able
to use the linker plugin facility to handle the ld side of things. But it's
not clear to me that it won't clash with other plugins. I.e., if one plugin
claims an archive file, will that prevent other plugins from being able to
process it?

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

[-- Attachment #2: 0001-Add-ar-support-for-dependencies.patch --]
[-- Type: text/x-patch, Size: 9612 bytes --]

From a2ceba11321c89dc9cc3e874a8e1f5bb98d28b10 Mon Sep 17 00:00:00 2001
From: Howard Chu <hyc@symas.com>
Date: Thu, 3 Sep 2020 17:34:16 +0100
Subject: [PATCH] Add ar support for dependencies

Record -L/-l dependencies of a static library in a __.LIBDEP entry.
Options should be passed literally as a single argument, e.g.
 ar rL "-L/usr/local/lib -lfoo -lbar" libnew.a obj1.o obj2.o
---
 binutils/ar.c       | 59 +++++++++++++++++++++++++++++++++++++++------
 binutils/binemul.c  | 53 ++++++++++++++++++++++++++++------------
 binutils/binemul.h  | 15 +++++++-----
 binutils/emul_aix.c | 16 +++---------
 4 files changed, 101 insertions(+), 42 deletions(-)

diff --git a/binutils/ar.c b/binutils/ar.c
index 85b342a650..af554acdba 100644
--- a/binutils/ar.c
+++ b/binutils/ar.c
@@ -138,6 +138,11 @@ static bfd_boolean full_pathname = FALSE;
 /* Whether to create a "thin" archive (symbol index only -- no files).  */
 static bfd_boolean make_thin_archive = FALSE;
 
+#define LIBDEPS	"__.LIBDEP"
+/* Dependencies to store in __.LIBDEP for linker to use */
+char *Libdeps;
+static bfd * Libdeps_bfd;
+
 static int show_version = 0;
 
 static int show_help = 0;
@@ -487,7 +492,7 @@ decode_options (int argc, char **argv)
       argv = new_argv;
     }
 
-  while ((c = getopt_long (argc, argv, "hdmpqrtxlcoOVsSuvabiMNfPTDU",
+  while ((c = getopt_long (argc, argv, "hdmpqrtxlL:coOVsSuvabiMNfPTDU",
 			   long_options, NULL)) != EOF)
     {
       switch (c)
@@ -536,6 +541,9 @@ decode_options (int argc, char **argv)
           break;
         case 'l':
           break;
+        case 'L':
+          Libdeps = optarg;
+          break;
         case 'c':
           silent_create = 1;
           break;
@@ -847,6 +855,27 @@ main (int argc, char **argv)
       if (operation == extract && bfd_is_thin_archive (arch))
 	fatal (_("`x' cannot be used on thin archives."));
 
+      if (Libdeps)
+        {
+          char **new_files;
+
+          /* create a bfd to contain the arguments */
+          Libdeps_bfd = bfd_create(LIBDEPS, arch);
+          bfd_find_target("binary", Libdeps_bfd);
+          bfd_set_format(Libdeps_bfd, bfd_object);
+          bfd_make_writable(Libdeps_bfd);
+          bfd_bwrite(Libdeps, strlen(Libdeps)+1, Libdeps_bfd);
+          bfd_make_readable(Libdeps_bfd);
+          bfd_find_target("plugin", Libdeps_bfd);
+          new_files = xmalloc((file_count+2) * sizeof(char *));
+          for (i=0; i<file_count; i++)
+            new_files[i] = files[i];
+          new_files[i++] = LIBDEPS;
+          file_count = i;
+          files = new_files;
+          files[i] = NULL;
+        }
+
       switch (operation)
 	{
 	case print_table:
@@ -1453,12 +1482,21 @@ replace_members (bfd *arch, char **files_to_move, bfd_boolean quick)
 
 		  after_bfd = get_pos_bfd (&arch->archive_next, pos_after,
 					   bfd_get_filename (current));
-		  if (ar_emul_replace (after_bfd, *files_to_move,
-				       target, verbose))
+		  if (Libdeps && FILENAME_CMP (normalize (*files_to_move, arch),
+						LIBDEPS) == 0)
+		    {
+		      changed |= ar_emul_replace_bfd (after_bfd, Libdeps_bfd,
+				       verbose);
+		    }
+		  else
+		    {
+		      changed |= ar_emul_replace (after_bfd, *files_to_move,
+				       target, verbose);
+		    }
+		  if (changed)
 		    {
 		      /* Snip out this entry from the chain.  */
 		      *current_ptr = (*current_ptr)->archive_next;
-		      changed = TRUE;
 		    }
 
 		  goto next_file;
@@ -1470,9 +1508,16 @@ replace_members (bfd *arch, char **files_to_move, bfd_boolean quick)
       /* Add to the end of the archive.  */
       after_bfd = get_pos_bfd (&arch->archive_next, pos_end, NULL);
 
-      if (ar_emul_append (after_bfd, *files_to_move, target,
-			  verbose, make_thin_archive))
-	changed = TRUE;
+      if (Libdeps && FILENAME_CMP (normalize (*files_to_move, arch), LIBDEPS) == 0)
+        {
+	  changed |= ar_emul_append_bfd (after_bfd, Libdeps_bfd,
+			  verbose, make_thin_archive);
+	}
+      else
+        {
+	  changed |= ar_emul_append (after_bfd, *files_to_move, target,
+			  verbose, make_thin_archive);
+	}
 
     next_file:;
 
diff --git a/binutils/binemul.c b/binutils/binemul.c
index 7c71b5b78c..8a0512ebd9 100644
--- a/binutils/binemul.c
+++ b/binutils/binemul.c
@@ -41,9 +41,24 @@ ar_emul_default_usage (FILE *fp)
 bfd_boolean
 ar_emul_append (bfd **after_bfd, char *file_name, const char *target,
 		bfd_boolean verbose, bfd_boolean flatten)
+{
+  bfd *new_bfd;
+
+  new_bfd = bfd_openr (file_name, target);
+  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
+  if (bin_dummy_emulation.ar_append)
+    return bin_dummy_emulation.ar_append (after_bfd, new_bfd,
+					  verbose, flatten);
+
+  return FALSE;
+}
+
+bfd_boolean
+ar_emul_append_bfd (bfd **after_bfd, bfd *new_bfd,
+		bfd_boolean verbose, bfd_boolean flatten)
 {
   if (bin_dummy_emulation.ar_append)
-    return bin_dummy_emulation.ar_append (after_bfd, file_name, target,
+    return bin_dummy_emulation.ar_append (after_bfd, new_bfd,
 					  verbose, flatten);
 
   return FALSE;
@@ -93,14 +108,9 @@ do_ar_emul_append (bfd **after_bfd, bfd *new_bfd,
 }
 
 bfd_boolean
-ar_emul_default_append (bfd **after_bfd, char *file_name,
-			const char *target, bfd_boolean verbose,
-			bfd_boolean flatten)
+ar_emul_default_append (bfd **after_bfd, bfd *new_bfd,
+			bfd_boolean verbose, bfd_boolean flatten)
 {
-  bfd *new_bfd;
-
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
   return do_ar_emul_append (after_bfd, new_bfd, verbose, flatten, any_ok);
 }
 
@@ -108,23 +118,34 @@ bfd_boolean
 ar_emul_replace (bfd **after_bfd, char *file_name, const char *target,
 		 bfd_boolean verbose)
 {
+  bfd *new_bfd;
+
+  new_bfd = bfd_openr (file_name, target);
+  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
+
   if (bin_dummy_emulation.ar_replace)
-    return bin_dummy_emulation.ar_replace (after_bfd, file_name,
-					   target, verbose);
+    return bin_dummy_emulation.ar_replace (after_bfd, new_bfd,
+					   verbose);
 
   return FALSE;
 }
 
 bfd_boolean
-ar_emul_default_replace (bfd **after_bfd, char *file_name,
-			 const char *target, bfd_boolean verbose)
+ar_emul_replace_bfd (bfd **after_bfd, bfd *new_bfd,
+		 bfd_boolean verbose)
 {
-  bfd *new_bfd;
+  if (bin_dummy_emulation.ar_replace)
+    return bin_dummy_emulation.ar_replace (after_bfd, new_bfd,
+					   verbose);
 
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
+  return FALSE;
+}
 
-  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, file_name);
+bfd_boolean
+ar_emul_default_replace (bfd **after_bfd, bfd *new_bfd,
+			 bfd_boolean verbose)
+{
+  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, bfd_get_filename (new_bfd));
 
   new_bfd->archive_next = *after_bfd;
   *after_bfd = new_bfd;
diff --git a/binutils/binemul.h b/binutils/binemul.h
index d4a14edfeb..16c1f89062 100644
--- a/binutils/binemul.h
+++ b/binutils/binemul.h
@@ -30,15 +30,19 @@ extern void ar_emul_usage (FILE *);
 extern void ar_emul_default_usage (FILE *);
 extern bfd_boolean ar_emul_append (bfd **, char *, const char *,
 				   bfd_boolean, bfd_boolean);
-extern bfd_boolean ar_emul_default_append (bfd **, char *, const char *,
+extern bfd_boolean ar_emul_append_bfd (bfd **, bfd *,
+				   bfd_boolean, bfd_boolean);
+extern bfd_boolean ar_emul_default_append (bfd **, bfd *,
 					   bfd_boolean, bfd_boolean);
 extern bfd_boolean do_ar_emul_append (bfd **, bfd *,
 				      bfd_boolean, bfd_boolean,
 				      bfd_boolean (*)(bfd *));
 extern bfd_boolean ar_emul_replace (bfd **, char *, const char *,
 				    bfd_boolean);
-extern bfd_boolean ar_emul_default_replace (bfd **, char *,
-					    const char *, bfd_boolean);
+extern bfd_boolean ar_emul_replace_bfd (bfd **, bfd *,
+				    bfd_boolean);
+extern bfd_boolean ar_emul_default_replace (bfd **, bfd *,
+					    bfd_boolean);
 extern bfd_boolean ar_emul_parse_arg (char *);
 extern bfd_boolean ar_emul_default_parse_arg (char *);
 
@@ -61,9 +65,8 @@ typedef struct bin_emulation_xfer_struct
 {
   /* Print out the extra options.  */
   void (* ar_usage) (FILE *fp);
-  bfd_boolean (* ar_append) (bfd **, char *, const char *, bfd_boolean,
-			     bfd_boolean);
-  bfd_boolean (* ar_replace) (bfd **, char *, const char *, bfd_boolean);
+  bfd_boolean (* ar_append) (bfd **, bfd *, bfd_boolean, bfd_boolean);
+  bfd_boolean (* ar_replace) (bfd **, bfd *, bfd_boolean);
   bfd_boolean (* ar_parse_arg) (char *);
 }
 bin_emulation_xfer_type;
diff --git a/binutils/emul_aix.c b/binutils/emul_aix.c
index c54e1083e5..726cb3b70c 100644
--- a/binutils/emul_aix.c
+++ b/binutils/emul_aix.c
@@ -67,30 +67,20 @@ check_aix (bfd *try_bfd)
 }
 
 static bfd_boolean
-ar_emul_aix_append (bfd **after_bfd, char *file_name, const char *target,
+ar_emul_aix_append (bfd **after_bfd, bfd *new_bfd,
 		    bfd_boolean verbose, bfd_boolean flatten)
 {
-  bfd *new_bfd;
-
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
-
   return do_ar_emul_append (after_bfd, new_bfd, verbose, flatten, check_aix);
 }
 
 static bfd_boolean
-ar_emul_aix_replace (bfd **after_bfd, char *file_name, const char *target,
+ar_emul_aix_replace (bfd **after_bfd, bfd *new_bfd,
 		     bfd_boolean verbose)
 {
-  bfd *new_bfd;
-
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
-
   if (!check_aix (new_bfd))
     return FALSE;
 
-  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, file_name);
+  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, bfd_get_filename (new_bfd));
 
   new_bfd->archive_next = *after_bfd;
   *after_bfd = new_bfd;
-- 
2.20.1

Re: dependency list for static libraries

[Nick Clifton]

Hi Howard,

> Here's a proposed patch to ar to implement the first half of the solution:
> storing the dependencies into a static library.

Some comments on the patch:

  * You need to add documentation of the new feature to binutils/doc/bin.texi
    and also a note to binutils/NEWS.

  * Whilst single letter options are the convention for ar, it would also be
    good if the GNU convention of providing a more verbose alternative option
    name were used to.  Eg --record-libdeps=<string>.

    Whilst on the subject of options, you should add some error checking for
    the presence of multiple L options.

  * There are several places where you call bfd library functions but do not
    check the return values.  This is a bad idea.  Always be paranoid and
    check to ensure that a function call has succeeded.

  * It is not clear to me why you create a binary bfd for Libdeps_bfd but
    then convert it to a plugin type bfd.  Can you explain what you are
    doing here ?

  * The change to the code to call ar_emul_replace() inside replace_members()
    looks wrong to me.  The current code will try to replace all of the entries
    on the files_to_move list, and will set changed to TRUE if any of the
    replacements succeeds.  The patched code will changed to FALSE if any
    replacement fails, even if earlier ones succeeded.  Plus once one 
    replacement has failed, future successful replacements will not be removed 
    from the archive chain.

  * There are some formatting issues as well. These are minor, but it is 
    helpful to ensure that the codebase uses a uniform style.  Please check
    out the GNU Coding Standards for more information:

      https://www.gnu.org/prep/standards/standards.html

   * It would also be really helpful if you could create a test for the
     binutils testsuite that checks this new functionality.


> It looks like I may be able
> to use the linker plugin facility to handle the ld side of things. But it's
> not clear to me that it won't clash with other plugins. I.e., if one plugin
> claims an archive file, will that prevent other plugins from being able to
> process it?

I am not sure that this has been done before, but I think that you are right -
once a file/archive has been claimed by one plugin, it cannot be claimed by
another.  It may however be possible for your plugin to process an archive
but not claim it.  This is just an idea - I have not tested it - and it might
mean that the ordering of plugins on the command line becomes important...

Cheers
  Nick

Re: dependency list for static libraries

[Howard Chu]

Nick Clifton wrote:
> Hi Howard,
> 
>> Here's a proposed patch to ar to implement the first half of the solution:
>> storing the dependencies into a static library.
> 
> Some comments on the patch:

Thanks for the feedback.
> 
>   * You need to add documentation of the new feature to binutils/doc/bin.texi
>     and also a note to binutils/NEWS.

OK, will add.
> 
>   * Whilst single letter options are the convention for ar, it would also be
>     good if the GNU convention of providing a more verbose alternative option
>     name were used to.  Eg --record-libdeps=<string>.
> 
>     Whilst on the subject of options, you should add some error checking for
>     the presence of multiple L options.

Or maybe just allow multiple L options to be concatenated together?

>   * There are several places where you call bfd library functions but do not
>     check the return values.  This is a bad idea.  Always be paranoid and
>     check to ensure that a function call has succeeded.

Ok.

>   * It is not clear to me why you create a binary bfd for Libdeps_bfd but
>     then convert it to a plugin type bfd.  Can you explain what you are
>     doing here ?

This was a major hassle, I should have commented it. The bfd gets created
with type "plugin", and that refuses the bfd_bwrite() call. (Just fails.)
The write only succeeded if I set it to "binary" type first. But then, trying
to add this bfd to the archive failed unless I changed the type back to "plugin."

All of this was unexpected, none of the docs for make_writable or make_readable
mentioned anything about these incompatibilities.

>   * The change to the code to call ar_emul_replace() inside replace_members()
>     looks wrong to me.  The current code will try to replace all of the entries
>     on the files_to_move list, and will set changed to TRUE if any of the
>     replacements succeeds.  The patched code will changed to FALSE if any
>     replacement fails, even if earlier ones succeeded.

No, that's not correct. The patched code ORs in the result, so it will not
change any previous success into a failure.

>     Plus once one 
>     replacement has failed, future successful replacements will not be removed 
>     from the archive chain.

Not sure that's the case either.

>   * There are some formatting issues as well. These are minor, but it is 
>     helpful to ensure that the codebase uses a uniform style.  Please check
>     out the GNU Coding Standards for more information:

The code used exactly the same indentation as the rest of the source file.
I can check it again, but ... this is not my first time contributing to GNU projects...
> 
>       https://www.gnu.org/prep/standards/standards.html
> 
>    * It would also be really helpful if you could create a test for the
>      binutils testsuite that checks this new functionality.

OK. Any suggestions on what exactly to check?

>> It looks like I may be able
>> to use the linker plugin facility to handle the ld side of things. But it's
>> not clear to me that it won't clash with other plugins. I.e., if one plugin
>> claims an archive file, will that prevent other plugins from being able to
>> process it?
> 
> I am not sure that this has been done before, but I think that you are right -
> once a file/archive has been claimed by one plugin, it cannot be claimed by
> another.  It may however be possible for your plugin to process an archive
> but not claim it.  This is just an idea - I have not tested it - and it might
> mean that the ordering of plugins on the command line becomes important...


> 
> Cheers
>   Nick
> 
> 
> 


-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: dependency list for static libraries

[Nick Clifton]

Hi Howard,

>>   * It is not clear to me why you create a binary bfd for Libdeps_bfd but
>>     then convert it to a plugin type bfd.  Can you explain what you are
>>     doing here ?
> 
> This was a major hassle, I should have commented it. The bfd gets created
> with type "plugin", and that refuses the bfd_bwrite() call. (Just fails.)
> The write only succeeded if I set it to "binary" type first. But then, trying
> to add this bfd to the archive failed unless I changed the type back to "plugin."

OK - please could you add a comment to this effect to enlighten future readers.


> All of this was unexpected, none of the docs for make_writable or make_readable
> mentioned anything about these incompatibilities.

Yes, the "plugin" architecture type is special.  It is not meant to be an ordinary 
bfd type.  And of course you are right - the binutils internal documentation is 
woeful to say the least.

 
>>   * The change to the code to call ar_emul_replace() inside replace_members()
>>     looks wrong to me.  The current code will try to replace all of the entries
>>     on the files_to_move list, and will set changed to TRUE if any of the
>>     replacements succeeds.  The patched code will changed to FALSE if any
>>     replacement fails, even if earlier ones succeeded.
> 
> No, that's not correct. The patched code ORs in the result, so it will not
> change any previous success into a failure.

Doh - OK, I misread this.  But I think that there is still a possible problem.  
Since the result is ORed in, if there is a failed call to ar_emul_replace after 
a successful one, changed will still be TRUE, and the failed element will be 
removed from the chain.  This differs from the old behaviour where the element
would not be removed if ar_emul_replace() fails.  (I am not sure however what
the impact of this change will be).


> OK. Any suggestions on what exactly to check?

Sure.  I would suggest adding a test that checks to see that:

 ar cvL libfoo.a "/foo/bar/" foo.o

(something like that) actually creates a library with a libdeps element
in it.  Take a look at binutils/testsuite/binutils-all/ar.exp where other
tests on ar are run.  Basically you need to create a new proc to the file
and then invoke it at the end of the file.  The new proc would probably
look something like this:

proc test_add_dependencies { } {
    global AR
    global AS
    global srcdir
    global subdir
    global obj

    set testname "ar adding library dependencies"

    if ![binutils_assemble $srcdir/$subdir/bintest.s tmpdir/bintest.${obj}] {
	unresolved $testname
	return
    }

    if [is_remote host] {
	set archive artest.a
	set objfile [remote_download host tmpdir/bintest.${obj}]
	remote_file host delete $archive
    } else {
	set archive tmpdir/artest.a
	set objfile tmpdir/bintest.${obj}
    }

    remote_file build delete tmpdir/artest.a

    set got [binutils_run $AR "-r -c $archive --record-libdeps /foo/bar ${objfile}"]
    if ![string match "" $got] {
	fail $testname
	return
    }

    set got [binutils_run $AR "-t $archive ${objfile}"]
    set expected_output "${objfile}^M__,LIBDEP"
    if ![string match $expected_output $got] {
	fail $testname
	return
    }

    pass $testname
}


Cheers
  Nick

Re: [PATCH] dependency list for static libraries

[Howard Chu]

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

Nick Clifton wrote:
> Hi Howard,
> 
>>>   * It is not clear to me why you create a binary bfd for Libdeps_bfd but
>>>     then convert it to a plugin type bfd.  Can you explain what you are
>>>     doing here ?
>>
>> This was a major hassle, I should have commented it. The bfd gets created
>> with type "plugin", and that refuses the bfd_bwrite() call. (Just fails.)
>> The write only succeeded if I set it to "binary" type first. But then, trying
>> to add this bfd to the archive failed unless I changed the type back to "plugin."
> 
> OK - please could you add a comment to this effect to enlighten future readers.

OK, new patch attached. Added comments, NEWS entry, texi doc, test case.

>>>   * The change to the code to call ar_emul_replace() inside replace_members()
>>>     looks wrong to me.  The current code will try to replace all of the entries
>>>     on the files_to_move list, and will set changed to TRUE if any of the
>>>     replacements succeeds.  The patched code will changed to FALSE if any
>>>     replacement fails, even if earlier ones succeeded.
>>
>> No, that's not correct. The patched code ORs in the result, so it will not
>> change any previous success into a failure.
> 
> Doh - OK, I misread this.  But I think that there is still a possible problem.  
> Since the result is ORed in, if there is a failed call to ar_emul_replace after 
> a successful one, changed will still be TRUE, and the failed element will be 
> removed from the chain.  This differs from the old behaviour where the element
> would not be removed if ar_emul_replace() fails.  (I am not sure however what
> the impact of this change will be).

OK, fixed this to preserve the original behavior for failed elements.

>> OK. Any suggestions on what exactly to check?
> 
> Sure.  I would suggest adding a test that checks to see that:
> 
>  ar cvL libfoo.a "/foo/bar/" foo.o
> 
> (something like that) actually creates a library with a libdeps element
> in it.  Take a look at binutils/testsuite/binutils-all/ar.exp where other
> tests on ar are run.  Basically you need to create a new proc to the file
> and then invoke it at the end of the file.  The new proc would probably
> look something like this:

Thanks. Had to tweak 2 lines but otherwise this worked.

> proc test_add_dependencies { } {

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-ar-support-for-dependencies.patch --]
[-- Type: text/x-patch; name="0001-Add-ar-support-for-dependencies.patch", Size: 17901 bytes --]

From a3eec185a507339cda733f55dfcc1ca092e619f4 Mon Sep 17 00:00:00 2001
From: Howard Chu <hyc@symas.com>
Date: Thu, 3 Sep 2020 17:34:16 +0100
Subject: [PATCH] Add ar support for dependencies

Record -L/-l dependencies of a static library in a __.LIBDEP entry.
Options should be passed literally as a single argument, e.g.
 ar rL "-L/usr/local/lib -lfoo -lbar" libnew.a obj1.o obj2.o
---
 binutils/NEWS                          |  5 ++
 binutils/ar.c                          | 91 +++++++++++++++++++++++---
 binutils/binemul.c                     | 53 ++++++++++-----
 binutils/binemul.h                     | 15 +++--
 binutils/doc/binutils.texi             | 19 +++++-
 binutils/emul_aix.c                    | 16 +----
 binutils/testsuite/binutils-all/ar.exp | 41 ++++++++++++
 7 files changed, 194 insertions(+), 46 deletions(-)

diff --git a/binutils/NEWS b/binutils/NEWS
index c0dc73d7d8..92a855f5d3 100644
--- a/binutils/NEWS
+++ b/binutils/NEWS
@@ -1,5 +1,10 @@
 -*- text -*-
 
+* The ar tool now has a L modifier (or --record-libdeps) option for
+  specifying dependencies of a static library. The arguments of this
+  option will be stored verbatim in the __.LIBDEP member of the archive,
+  which the linker may read at link time.
+
 * Readelf now accepts the -C command line option to enable the demangling of
   symbol names.  In addition the --demangle=<style>, --no-demangle,
   --recurse-limit and --no-recurse-limit options are also now availale.
diff --git a/binutils/ar.c b/binutils/ar.c
index 85b342a650..7128bb41e6 100644
--- a/binutils/ar.c
+++ b/binutils/ar.c
@@ -138,6 +138,11 @@ static bfd_boolean full_pathname = FALSE;
 /* Whether to create a "thin" archive (symbol index only -- no files).  */
 static bfd_boolean make_thin_archive = FALSE;
 
+#define LIBDEPS	"__.LIBDEP"
+/* Dependencies to store in __.LIBDEP for linker to use */
+char *Libdeps;
+static bfd * Libdeps_bfd;
+
 static int show_version = 0;
 
 static int show_help = 0;
@@ -166,6 +171,7 @@ static struct option long_options[] =
   {"target", required_argument, NULL, OPTION_TARGET},
   {"version", no_argument, &show_version, 1},
   {"output", required_argument, NULL, OPTION_OUTPUT},
+  {"record-libdeps", required_argument, NULL, 'L'},
   {NULL, no_argument, NULL, 0}
 };
 
@@ -279,13 +285,13 @@ usage (int help)
 #if BFD_SUPPORTS_PLUGINS
   /* xgettext:c-format */
   const char *command_line
-    = _("Usage: %s [emulation options] [-]{dmpqrstx}[abcDfilMNoOPsSTuvV]"
+    = _("Usage: %s [emulation options] [-]{dmpqrstx}[abcDfilLMNoOPsSTuvV]"
 	" [--plugin <name>] [member-name] [count] archive-file file...\n");
 
 #else
   /* xgettext:c-format */
   const char *command_line
-    = _("Usage: %s [emulation options] [-]{dmpqrstx}[abcDfilMNoOPsSTuvV]"
+    = _("Usage: %s [emulation options] [-]{dmpqrstx}[abcDfilLMNoOPsSTuvV]"
 	" [member-name] [count] archive-file file...\n");
 #endif
   s = help ? stdout : stderr;
@@ -329,6 +335,7 @@ usage (int help)
   fprintf (s, _(" generic modifiers:\n"));
   fprintf (s, _("  [c]          - do not warn if the library had to be created\n"));
   fprintf (s, _("  [s]          - create an archive index (cf. ranlib)\n"));
+  fprintf (s, _("  [L LIBDEPS]  - specify dependencies of this library\n"));
   fprintf (s, _("  [S]          - do not build a symbol table\n"));
   fprintf (s, _("  [T]          - make a thin archive\n"));
   fprintf (s, _("  [v]          - be verbose\n"));
@@ -336,6 +343,7 @@ usage (int help)
   fprintf (s, _("  @<file>      - read options from <file>\n"));
   fprintf (s, _("  --target=BFDNAME - specify the target object format as BFDNAME\n"));
   fprintf (s, _("  --output=DIRNAME - specify the output directory for extraction operations\n"));
+  fprintf (s, _("  --record-libdeps=LIBDEPS - specify dependencies of this library\n"));
 #if BFD_SUPPORTS_PLUGINS
   fprintf (s, _(" optional:\n"));
   fprintf (s, _("  --plugin <p> - load the specified plugin\n"));
@@ -487,7 +495,7 @@ decode_options (int argc, char **argv)
       argv = new_argv;
     }
 
-  while ((c = getopt_long (argc, argv, "hdmpqrtxlcoOVsSuvabiMNfPTDU",
+  while ((c = getopt_long (argc, argv, "hdmpqrtxlL:coOVsSuvabiMNfPTDU",
 			   long_options, NULL)) != EOF)
     {
       switch (c)
@@ -536,6 +544,11 @@ decode_options (int argc, char **argv)
           break;
         case 'l':
           break;
+        case 'L':
+	  if (Libdeps != NULL)
+            fatal (_("Libdeps specified more than once"));
+          Libdeps = optarg;
+          break;
         case 'c':
           silent_create = 1;
           break;
@@ -847,6 +860,50 @@ main (int argc, char **argv)
       if (operation == extract && bfd_is_thin_archive (arch))
 	fatal (_("`x' cannot be used on thin archives."));
 
+      if (Libdeps)
+        {
+          char **new_files;
+          bfd_size_type reclen = strlen(Libdeps)+1;
+
+          /* Create a bfd to contain the dependencies.
+	     It inherits its type from arch, but we must
+	     set the type to "binary" otherwise bfd_bwrite()
+	     will fail. After writing, we must set the type
+	     back to "plugin" otherwise adding it to the
+	     archive will fail. */
+          Libdeps_bfd = bfd_create(LIBDEPS, arch);
+	  if (Libdeps_bfd == NULL)
+	    fatal (_("Cannot create Libdeps record."));
+
+          if (bfd_find_target("binary", Libdeps_bfd) == NULL)
+	    fatal (_("Cannot set Libdeps record type to binary."));
+
+          if (!bfd_set_format(Libdeps_bfd, bfd_object))
+	    fatal (_("Cannot set Libdeps object format."));
+
+          if (!bfd_make_writable(Libdeps_bfd))
+	    fatal (_("Cannot make Libdeps object writable."));
+
+	  if (bfd_bwrite(Libdeps, reclen, Libdeps_bfd) != reclen)
+	    fatal (_("Cannot write Libdeps record."));
+
+          if (!bfd_make_readable(Libdeps_bfd))
+	    fatal (_("Cannot make Libdeps object readable."));
+
+          if (bfd_find_target("plugin", Libdeps_bfd) == NULL)
+	    fatal (_("Cannot reset Libdeps record type."));
+
+	  /* Append our Libdeps record to the list of files
+	     being operated on. */
+          new_files = xmalloc((file_count+2) * sizeof(char *));
+          for (i=0; i<file_count; i++)
+            new_files[i] = files[i];
+          new_files[i++] = LIBDEPS;
+          file_count = i;
+          files = new_files;
+          files[i] = NULL;
+        }
+
       switch (operation)
 	{
 	case print_table:
@@ -1432,6 +1489,7 @@ replace_members (bfd *arch, char **files_to_move, bfd_boolean quick)
 				normalize (bfd_get_filename (current), arch)) == 0
 		  && current->arelt_data != NULL)
 		{
+		  bfd_boolean replaced;
 		  if (newer_only)
 		    {
 		      struct stat fsbuf, asbuf;
@@ -1453,8 +1511,18 @@ replace_members (bfd *arch, char **files_to_move, bfd_boolean quick)
 
 		  after_bfd = get_pos_bfd (&arch->archive_next, pos_after,
 					   bfd_get_filename (current));
-		  if (ar_emul_replace (after_bfd, *files_to_move,
-				       target, verbose))
+		  if (Libdeps && FILENAME_CMP (normalize (*files_to_move, arch),
+						LIBDEPS) == 0)
+		    {
+		      replaced = ar_emul_replace_bfd (after_bfd, Libdeps_bfd,
+				       verbose);
+		    }
+		  else
+		    {
+		      replaced = ar_emul_replace (after_bfd, *files_to_move,
+				       target, verbose);
+		    }
+		  if (replaced)
 		    {
 		      /* Snip out this entry from the chain.  */
 		      *current_ptr = (*current_ptr)->archive_next;
@@ -1470,9 +1538,16 @@ replace_members (bfd *arch, char **files_to_move, bfd_boolean quick)
       /* Add to the end of the archive.  */
       after_bfd = get_pos_bfd (&arch->archive_next, pos_end, NULL);
 
-      if (ar_emul_append (after_bfd, *files_to_move, target,
-			  verbose, make_thin_archive))
-	changed = TRUE;
+      if (Libdeps && FILENAME_CMP (normalize (*files_to_move, arch), LIBDEPS) == 0)
+        {
+	  changed |= ar_emul_append_bfd (after_bfd, Libdeps_bfd,
+			  verbose, make_thin_archive);
+	}
+      else
+        {
+	  changed |= ar_emul_append (after_bfd, *files_to_move, target,
+			  verbose, make_thin_archive);
+	}
 
     next_file:;
 
diff --git a/binutils/binemul.c b/binutils/binemul.c
index 7c71b5b78c..8a0512ebd9 100644
--- a/binutils/binemul.c
+++ b/binutils/binemul.c
@@ -41,9 +41,24 @@ ar_emul_default_usage (FILE *fp)
 bfd_boolean
 ar_emul_append (bfd **after_bfd, char *file_name, const char *target,
 		bfd_boolean verbose, bfd_boolean flatten)
+{
+  bfd *new_bfd;
+
+  new_bfd = bfd_openr (file_name, target);
+  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
+  if (bin_dummy_emulation.ar_append)
+    return bin_dummy_emulation.ar_append (after_bfd, new_bfd,
+					  verbose, flatten);
+
+  return FALSE;
+}
+
+bfd_boolean
+ar_emul_append_bfd (bfd **after_bfd, bfd *new_bfd,
+		bfd_boolean verbose, bfd_boolean flatten)
 {
   if (bin_dummy_emulation.ar_append)
-    return bin_dummy_emulation.ar_append (after_bfd, file_name, target,
+    return bin_dummy_emulation.ar_append (after_bfd, new_bfd,
 					  verbose, flatten);
 
   return FALSE;
@@ -93,14 +108,9 @@ do_ar_emul_append (bfd **after_bfd, bfd *new_bfd,
 }
 
 bfd_boolean
-ar_emul_default_append (bfd **after_bfd, char *file_name,
-			const char *target, bfd_boolean verbose,
-			bfd_boolean flatten)
+ar_emul_default_append (bfd **after_bfd, bfd *new_bfd,
+			bfd_boolean verbose, bfd_boolean flatten)
 {
-  bfd *new_bfd;
-
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
   return do_ar_emul_append (after_bfd, new_bfd, verbose, flatten, any_ok);
 }
 
@@ -108,23 +118,34 @@ bfd_boolean
 ar_emul_replace (bfd **after_bfd, char *file_name, const char *target,
 		 bfd_boolean verbose)
 {
+  bfd *new_bfd;
+
+  new_bfd = bfd_openr (file_name, target);
+  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
+
   if (bin_dummy_emulation.ar_replace)
-    return bin_dummy_emulation.ar_replace (after_bfd, file_name,
-					   target, verbose);
+    return bin_dummy_emulation.ar_replace (after_bfd, new_bfd,
+					   verbose);
 
   return FALSE;
 }
 
 bfd_boolean
-ar_emul_default_replace (bfd **after_bfd, char *file_name,
-			 const char *target, bfd_boolean verbose)
+ar_emul_replace_bfd (bfd **after_bfd, bfd *new_bfd,
+		 bfd_boolean verbose)
 {
-  bfd *new_bfd;
+  if (bin_dummy_emulation.ar_replace)
+    return bin_dummy_emulation.ar_replace (after_bfd, new_bfd,
+					   verbose);
 
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
+  return FALSE;
+}
 
-  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, file_name);
+bfd_boolean
+ar_emul_default_replace (bfd **after_bfd, bfd *new_bfd,
+			 bfd_boolean verbose)
+{
+  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, bfd_get_filename (new_bfd));
 
   new_bfd->archive_next = *after_bfd;
   *after_bfd = new_bfd;
diff --git a/binutils/binemul.h b/binutils/binemul.h
index d4a14edfeb..16c1f89062 100644
--- a/binutils/binemul.h
+++ b/binutils/binemul.h
@@ -30,15 +30,19 @@ extern void ar_emul_usage (FILE *);
 extern void ar_emul_default_usage (FILE *);
 extern bfd_boolean ar_emul_append (bfd **, char *, const char *,
 				   bfd_boolean, bfd_boolean);
-extern bfd_boolean ar_emul_default_append (bfd **, char *, const char *,
+extern bfd_boolean ar_emul_append_bfd (bfd **, bfd *,
+				   bfd_boolean, bfd_boolean);
+extern bfd_boolean ar_emul_default_append (bfd **, bfd *,
 					   bfd_boolean, bfd_boolean);
 extern bfd_boolean do_ar_emul_append (bfd **, bfd *,
 				      bfd_boolean, bfd_boolean,
 				      bfd_boolean (*)(bfd *));
 extern bfd_boolean ar_emul_replace (bfd **, char *, const char *,
 				    bfd_boolean);
-extern bfd_boolean ar_emul_default_replace (bfd **, char *,
-					    const char *, bfd_boolean);
+extern bfd_boolean ar_emul_replace_bfd (bfd **, bfd *,
+				    bfd_boolean);
+extern bfd_boolean ar_emul_default_replace (bfd **, bfd *,
+					    bfd_boolean);
 extern bfd_boolean ar_emul_parse_arg (char *);
 extern bfd_boolean ar_emul_default_parse_arg (char *);
 
@@ -61,9 +65,8 @@ typedef struct bin_emulation_xfer_struct
 {
   /* Print out the extra options.  */
   void (* ar_usage) (FILE *fp);
-  bfd_boolean (* ar_append) (bfd **, char *, const char *, bfd_boolean,
-			     bfd_boolean);
-  bfd_boolean (* ar_replace) (bfd **, char *, const char *, bfd_boolean);
+  bfd_boolean (* ar_append) (bfd **, bfd *, bfd_boolean, bfd_boolean);
+  bfd_boolean (* ar_replace) (bfd **, bfd *, bfd_boolean);
   bfd_boolean (* ar_parse_arg) (char *);
 }
 bin_emulation_xfer_type;
diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
index 35a5a67041..70d97565b5 100644
--- a/binutils/doc/binutils.texi
+++ b/binutils/doc/binutils.texi
@@ -170,7 +170,7 @@ in the section entitled ``GNU Free Documentation License''.
 @c man title ar create, modify, and extract from archives
 
 @smallexample
-ar [-]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@option{--output} @var{dirname}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
+ar [-]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@option{--output} @var{dirname}] [@option{--record-libdeps} @var{libdeps}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
 ar -M [ <mri-script ]
 @end smallexample
 
@@ -196,7 +196,9 @@ characters (typical of formats related to coff).
 @cindex libraries
 @command{ar} is considered a binary utility because archives of this sort
 are most often used as @dfn{libraries} holding commonly needed
-subroutines.
+subroutines.  Since libraries often will depend on other libraries,
+@command{ar} can also record the dependencies of a library when the
+@option{--record-libdeps} option is specified.
 
 @cindex symbol index
 @command{ar} creates an index to the symbols defined in relocatable
@@ -254,7 +256,7 @@ program.
 
 @smallexample
 @c man begin SYNOPSIS ar
-ar [@option{-X32_64}] [@option{-}]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@option{--output} @var{dirname}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
+ar [@option{-X32_64}] [@option{-}]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@option{--output} @var{dirname}] [@option{--record-libdeps} @var{libdeps}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
 @c man end
 @end smallexample
 
@@ -452,6 +454,13 @@ This modifier is accepted but not used.
 @c whaffor ar l modifier??? presumably compat; with
 @c what???---doc@@cygnus.com, 25jan91
 
+@item L
+Specify dependencies of this library.  The dependencies must immediately
+follow this option character, must use the same syntax as the linker
+command line, and must be specified within a single argument.  I.e., if
+multiple items are needed, they must be quoted to form a single command
+line argument.  For example @samp{L "-L/usr/local/lib -lmydep1 -lmydep2"}
+
 @item N
 Uses the @var{count} parameter.  This is used if there are multiple
 entries in the archive with the same name.  Extract or delete instance
@@ -592,6 +601,10 @@ Note - although the presence of this option does imply a @option{x}
 extraction operation that option must still be included on the command
 line.
 
+@item --record-libdeps @var{libdeps}
+The @option{--record-libdeps} option is identical to the @option{L} modifier,
+just handled in long form.
+
 @end table
 @c man end
 
diff --git a/binutils/emul_aix.c b/binutils/emul_aix.c
index c54e1083e5..726cb3b70c 100644
--- a/binutils/emul_aix.c
+++ b/binutils/emul_aix.c
@@ -67,30 +67,20 @@ check_aix (bfd *try_bfd)
 }
 
 static bfd_boolean
-ar_emul_aix_append (bfd **after_bfd, char *file_name, const char *target,
+ar_emul_aix_append (bfd **after_bfd, bfd *new_bfd,
 		    bfd_boolean verbose, bfd_boolean flatten)
 {
-  bfd *new_bfd;
-
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
-
   return do_ar_emul_append (after_bfd, new_bfd, verbose, flatten, check_aix);
 }
 
 static bfd_boolean
-ar_emul_aix_replace (bfd **after_bfd, char *file_name, const char *target,
+ar_emul_aix_replace (bfd **after_bfd, bfd *new_bfd,
 		     bfd_boolean verbose)
 {
-  bfd *new_bfd;
-
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
-
   if (!check_aix (new_bfd))
     return FALSE;
 
-  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, file_name);
+  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, bfd_get_filename (new_bfd));
 
   new_bfd->archive_next = *after_bfd;
   *after_bfd = new_bfd;
diff --git a/binutils/testsuite/binutils-all/ar.exp b/binutils/testsuite/binutils-all/ar.exp
index 5a9d27c6d0..273a0a3909 100644
--- a/binutils/testsuite/binutils-all/ar.exp
+++ b/binutils/testsuite/binutils-all/ar.exp
@@ -715,6 +715,46 @@ proc many_files { } {
     pass $testname
 }
 
+proc test_add_dependencies { } {
+    global AR
+    global AS
+    global srcdir
+    global subdir
+    global obj
+
+    set testname "ar adding library dependencies"
+
+    if ![binutils_assemble $srcdir/$subdir/bintest.s tmpdir/bintest.${obj}] {
+	unresolved $testname
+	return
+    }
+
+    if [is_remote host] {
+	set archive artest.a
+	set objfile [remote_download host tmpdir/bintest.${obj}]
+	remote_file host delete $archive
+    } else {
+	set archive tmpdir/artest.a
+	set objfile tmpdir/bintest.${obj}
+    }
+
+    remote_file build delete tmpdir/artest.a
+
+    set got [binutils_run $AR "-r -c $archive --record-libdeps /foo/bar ${objfile}"]
+    if ![string match "" $got] {
+	fail $testname
+	return
+    }
+
+    set got [binutils_run $AR "-t $archive"]
+    if ![string match "*bintest.${obj}\r__.LIBDEP*" $got] {
+	fail $testname
+	return
+    }
+
+    pass $testname
+}
+
 # Run the tests.
 
 # Only run the bfdtest checks if the programs exist.  Since these
@@ -743,6 +783,7 @@ move_an_element
 empty_archive
 extract_an_element
 many_files
+test_add_dependencies
 
 if { [is_elf_format] && [supports_gnu_unique] } {
     unique_symbol
-- 
2.20.1

Re: [PATCH] dependency list for static libraries

[Fangrui Song]

Did the thread start at
https://sourceware.org/legacy-ml/binutils/2017-09/msg00158.html ?

If all dependencies are specified on the command line but there are
"backward references", we can step back and think about loosing the
requirement like some other binary formats:
https://sourceware.org/pipermail/binutils/2020-September/113194.html "Memoryless" archive processing of ld

If some archives want system libraries (e.g. -lm -lpthread), MSVC style
#pragma comment(lib, "pthread")
may be convenient. clang supports the syntax for ELF and the syntax has
been used in several places in libc++. LLD supports .deplibs (
http://lists.llvm.org/pipermail/llvm-dev/2019-March/131004.html
http://llvm.1065342.n5.nabble.com/llvm-dev-RFC-ELF-Autolinking-td126998.html )

On 2020-09-22, Howard Chu wrote:
>Nick Clifton wrote:
>> Hi Howard,
>>
>>>>   * It is not clear to me why you create a binary bfd for Libdeps_bfd but
>>>>     then convert it to a plugin type bfd.  Can you explain what you are
>>>>     doing here ?
>>>
>>> This was a major hassle, I should have commented it. The bfd gets created
>>> with type "plugin", and that refuses the bfd_bwrite() call. (Just fails.)
>>> The write only succeeded if I set it to "binary" type first. But then, trying
>>> to add this bfd to the archive failed unless I changed the type back to "plugin."
>>
>> OK - please could you add a comment to this effect to enlighten future readers.
>
>OK, new patch attached. Added comments, NEWS entry, texi doc, test case.
>
>>>>   * The change to the code to call ar_emul_replace() inside replace_members()
>>>>     looks wrong to me.  The current code will try to replace all of the entries
>>>>     on the files_to_move list, and will set changed to TRUE if any of the
>>>>     replacements succeeds.  The patched code will changed to FALSE if any
>>>>     replacement fails, even if earlier ones succeeded.
>>>
>>> No, that's not correct. The patched code ORs in the result, so it will not
>>> change any previous success into a failure.
>>
>> Doh - OK, I misread this.  But I think that there is still a possible problem.
>> Since the result is ORed in, if there is a failed call to ar_emul_replace after
>> a successful one, changed will still be TRUE, and the failed element will be
>> removed from the chain.  This differs from the old behaviour where the element
>> would not be removed if ar_emul_replace() fails.  (I am not sure however what
>> the impact of this change will be).
>
>OK, fixed this to preserve the original behavior for failed elements.
>
>>> OK. Any suggestions on what exactly to check?
>>
>> Sure.  I would suggest adding a test that checks to see that:
>>
>>  ar cvL libfoo.a "/foo/bar/" foo.o
>>
>> (something like that) actually creates a library with a libdeps element
>> in it.  Take a look at binutils/testsuite/binutils-all/ar.exp where other
>> tests on ar are run.  Basically you need to create a new proc to the file
>> and then invoke it at the end of the file.  The new proc would probably
>> look something like this:
>
>Thanks. Had to tweak 2 lines but otherwise this worked.
>
>> proc test_add_dependencies { } {

Re: [PATCH] dependency list for static libraries

[Howard Chu]

Fangrui Song wrote:
> Did the thread start at
> https://sourceware.org/legacy-ml/binutils/2017-09/msg00158.html ?

Yes.
> 
> If all dependencies are specified on the command line but there are
> "backward references", we can step back and think about loosing the
> requirement like some other binary formats:
> https://sourceware.org/pipermail/binutils/2020-September/113194.html "Memoryless" archive processing of ld

Seems unrelated.

> If some archives want system libraries (e.g. -lm -lpthread), MSVC style
> #pragma comment(lib, "pthread")
> may be convenient. clang supports the syntax for ELF and the syntax has
> been used in several places in libc++. LLD supports .deplibs (
> http://lists.llvm.org/pipermail/llvm-dev/2019-March/131004.html
> http://llvm.1065342.n5.nabble.com/llvm-dev-RFC-ELF-Autolinking-td126998.html )

This seems misguided and wasteful, IMO. You'll have redundant info embedded in
each of the object files that are built for a library, but you only need these
dependencies to be specified once - at the library level itself.

This problem is already solved for shared libraries. They have their dependencies
listed in their dynamic section, and it's specified once at library build time.
That's the correct approach. My aim here is to make static libraries behave the
same as shared libraries, in this respect.

Embedding dependent library info in source files is just wrong, particularly when
you write code that's cross-platform. It makes sense that Microsoft would latch
on to an idea like this, since in their worldview they are the sole provider of
useful libraries for their systems. But it doesn't make sense for portable
cross-platform code.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] dependency list for static libraries

[Howard Chu]

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

Howard Chu wrote:
> Nick Clifton wrote:
>> Hi Howard,
>>
>>>>   * It is not clear to me why you create a binary bfd for Libdeps_bfd but
>>>>     then convert it to a plugin type bfd.  Can you explain what you are
>>>>     doing here ?
>>>
>>> This was a major hassle, I should have commented it. The bfd gets created
>>> with type "plugin", and that refuses the bfd_bwrite() call. (Just fails.)
>>> The write only succeeded if I set it to "binary" type first. But then, trying
>>> to add this bfd to the archive failed unless I changed the type back to "plugin."
>>
>> OK - please could you add a comment to this effect to enlighten future readers.
> 
> OK, new patch attached. Added comments, NEWS entry, texi doc, test case.

Sorry for the noise, this attached is the same as previous but with fixes
to whitespace / indentation.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-ar-support-for-dependencies.patch --]
[-- Type: text/x-patch; name="0001-Add-ar-support-for-dependencies.patch", Size: 17792 bytes --]

From e691851bb3cfa021613f71dea381e33b167bf362 Mon Sep 17 00:00:00 2001
From: Howard Chu <hyc@symas.com>
Date: Thu, 3 Sep 2020 17:34:16 +0100
Subject: [PATCH] Add ar support for dependencies

Record -L/-l dependencies of a static library in a __.LIBDEP entry.
Options should be passed literally as a single argument, e.g.
 ar rL "-L/usr/local/lib -lfoo -lbar" libnew.a obj1.o obj2.o
---
 binutils/NEWS                          |  5 ++
 binutils/ar.c                          | 91 +++++++++++++++++++++++---
 binutils/binemul.c                     | 53 ++++++++++-----
 binutils/binemul.h                     | 15 +++--
 binutils/doc/binutils.texi             | 19 +++++-
 binutils/emul_aix.c                    | 16 +----
 binutils/testsuite/binutils-all/ar.exp | 41 ++++++++++++
 7 files changed, 194 insertions(+), 46 deletions(-)

diff --git a/binutils/NEWS b/binutils/NEWS
index c0dc73d7d8..92a855f5d3 100644
--- a/binutils/NEWS
+++ b/binutils/NEWS
@@ -1,5 +1,10 @@
 -*- text -*-
 
+* The ar tool now has a L modifier (or --record-libdeps) option for
+  specifying dependencies of a static library. The arguments of this
+  option will be stored verbatim in the __.LIBDEP member of the archive,
+  which the linker may read at link time.
+
 * Readelf now accepts the -C command line option to enable the demangling of
   symbol names.  In addition the --demangle=<style>, --no-demangle,
   --recurse-limit and --no-recurse-limit options are also now availale.
diff --git a/binutils/ar.c b/binutils/ar.c
index 85b342a650..c937d3268f 100644
--- a/binutils/ar.c
+++ b/binutils/ar.c
@@ -138,6 +138,11 @@ static bfd_boolean full_pathname = FALSE;
 /* Whether to create a "thin" archive (symbol index only -- no files).  */
 static bfd_boolean make_thin_archive = FALSE;
 
+#define LIBDEPS	"__.LIBDEP"
+/* Dependencies to store in __.LIBDEP for linker to use */
+char *Libdeps;
+static bfd * Libdeps_bfd;
+
 static int show_version = 0;
 
 static int show_help = 0;
@@ -166,6 +171,7 @@ static struct option long_options[] =
   {"target", required_argument, NULL, OPTION_TARGET},
   {"version", no_argument, &show_version, 1},
   {"output", required_argument, NULL, OPTION_OUTPUT},
+  {"record-libdeps", required_argument, NULL, 'L'},
   {NULL, no_argument, NULL, 0}
 };
 
@@ -279,13 +285,13 @@ usage (int help)
 #if BFD_SUPPORTS_PLUGINS
   /* xgettext:c-format */
   const char *command_line
-    = _("Usage: %s [emulation options] [-]{dmpqrstx}[abcDfilMNoOPsSTuvV]"
+    = _("Usage: %s [emulation options] [-]{dmpqrstx}[abcDfilLMNoOPsSTuvV]"
 	" [--plugin <name>] [member-name] [count] archive-file file...\n");
 
 #else
   /* xgettext:c-format */
   const char *command_line
-    = _("Usage: %s [emulation options] [-]{dmpqrstx}[abcDfilMNoOPsSTuvV]"
+    = _("Usage: %s [emulation options] [-]{dmpqrstx}[abcDfilLMNoOPsSTuvV]"
 	" [member-name] [count] archive-file file...\n");
 #endif
   s = help ? stdout : stderr;
@@ -329,6 +335,7 @@ usage (int help)
   fprintf (s, _(" generic modifiers:\n"));
   fprintf (s, _("  [c]          - do not warn if the library had to be created\n"));
   fprintf (s, _("  [s]          - create an archive index (cf. ranlib)\n"));
+  fprintf (s, _("  [L LIBDEPS]  - specify dependencies of this library\n"));
   fprintf (s, _("  [S]          - do not build a symbol table\n"));
   fprintf (s, _("  [T]          - make a thin archive\n"));
   fprintf (s, _("  [v]          - be verbose\n"));
@@ -336,6 +343,7 @@ usage (int help)
   fprintf (s, _("  @<file>      - read options from <file>\n"));
   fprintf (s, _("  --target=BFDNAME - specify the target object format as BFDNAME\n"));
   fprintf (s, _("  --output=DIRNAME - specify the output directory for extraction operations\n"));
+  fprintf (s, _("  --record-libdeps=LIBDEPS - specify dependencies of this library\n"));
 #if BFD_SUPPORTS_PLUGINS
   fprintf (s, _(" optional:\n"));
   fprintf (s, _("  --plugin <p> - load the specified plugin\n"));
@@ -487,7 +495,7 @@ decode_options (int argc, char **argv)
       argv = new_argv;
     }
 
-  while ((c = getopt_long (argc, argv, "hdmpqrtxlcoOVsSuvabiMNfPTDU",
+  while ((c = getopt_long (argc, argv, "hdmpqrtxlL:coOVsSuvabiMNfPTDU",
 			   long_options, NULL)) != EOF)
     {
       switch (c)
@@ -536,6 +544,11 @@ decode_options (int argc, char **argv)
           break;
         case 'l':
           break;
+        case 'L':
+          if (Libdeps != NULL)
+            fatal (_("Libdeps specified more than once"));
+          Libdeps = optarg;
+          break;
         case 'c':
           silent_create = 1;
           break;
@@ -847,6 +860,50 @@ main (int argc, char **argv)
       if (operation == extract && bfd_is_thin_archive (arch))
 	fatal (_("`x' cannot be used on thin archives."));
 
+      if (Libdeps)
+	{
+	  char **new_files;
+	  bfd_size_type reclen = strlen(Libdeps)+1;
+
+	  /* Create a bfd to contain the dependencies.
+	     It inherits its type from arch, but we must
+	     set the type to "binary" otherwise bfd_bwrite()
+	     will fail. After writing, we must set the type
+	     back to "plugin" otherwise adding it to the
+	     archive will fail. */
+	  Libdeps_bfd = bfd_create(LIBDEPS, arch);
+	  if (Libdeps_bfd == NULL)
+	    fatal (_("Cannot create Libdeps record."));
+
+	  if (bfd_find_target("binary", Libdeps_bfd) == NULL)
+	    fatal (_("Cannot set Libdeps record type to binary."));
+
+	  if (!bfd_set_format(Libdeps_bfd, bfd_object))
+	    fatal (_("Cannot set Libdeps object format."));
+
+	  if (!bfd_make_writable(Libdeps_bfd))
+	    fatal (_("Cannot make Libdeps object writable."));
+
+	  if (bfd_bwrite(Libdeps, reclen, Libdeps_bfd) != reclen)
+	    fatal (_("Cannot write Libdeps record."));
+
+	  if (!bfd_make_readable(Libdeps_bfd))
+	    fatal (_("Cannot make Libdeps object readable."));
+
+	  if (bfd_find_target("plugin", Libdeps_bfd) == NULL)
+	    fatal (_("Cannot reset Libdeps record type."));
+
+	  /* Append our Libdeps record to the list of files
+	     being operated on. */
+	  new_files = xmalloc((file_count+2) * sizeof(char *));
+	  for (i=0; i<file_count; i++)
+	    new_files[i] = files[i];
+	  new_files[i++] = LIBDEPS;
+	  file_count = i;
+	  files = new_files;
+	  files[i] = NULL;
+	}
+
       switch (operation)
 	{
 	case print_table:
@@ -1432,6 +1489,7 @@ replace_members (bfd *arch, char **files_to_move, bfd_boolean quick)
 				normalize (bfd_get_filename (current), arch)) == 0
 		  && current->arelt_data != NULL)
 		{
+		  bfd_boolean replaced;
 		  if (newer_only)
 		    {
 		      struct stat fsbuf, asbuf;
@@ -1453,8 +1511,18 @@ replace_members (bfd *arch, char **files_to_move, bfd_boolean quick)
 
 		  after_bfd = get_pos_bfd (&arch->archive_next, pos_after,
 					   bfd_get_filename (current));
-		  if (ar_emul_replace (after_bfd, *files_to_move,
-				       target, verbose))
+		  if (Libdeps && FILENAME_CMP (normalize (*files_to_move, arch),
+					       LIBDEPS) == 0)
+		    {
+		      replaced = ar_emul_replace_bfd (after_bfd, Libdeps_bfd,
+						      verbose);
+		    }
+		  else
+		    {
+		      replaced = ar_emul_replace (after_bfd, *files_to_move,
+						  target, verbose);
+		    }
+		  if (replaced)
 		    {
 		      /* Snip out this entry from the chain.  */
 		      *current_ptr = (*current_ptr)->archive_next;
@@ -1470,9 +1538,16 @@ replace_members (bfd *arch, char **files_to_move, bfd_boolean quick)
       /* Add to the end of the archive.  */
       after_bfd = get_pos_bfd (&arch->archive_next, pos_end, NULL);
 
-      if (ar_emul_append (after_bfd, *files_to_move, target,
-			  verbose, make_thin_archive))
-	changed = TRUE;
+      if (Libdeps && FILENAME_CMP (normalize (*files_to_move, arch), LIBDEPS) == 0)
+        {
+	  changed |= ar_emul_append_bfd (after_bfd, Libdeps_bfd,
+					 verbose, make_thin_archive);
+	}
+      else
+        {
+	  changed |= ar_emul_append (after_bfd, *files_to_move, target,
+				     verbose, make_thin_archive);
+	}
 
     next_file:;
 
diff --git a/binutils/binemul.c b/binutils/binemul.c
index 7c71b5b78c..8a0512ebd9 100644
--- a/binutils/binemul.c
+++ b/binutils/binemul.c
@@ -41,9 +41,24 @@ ar_emul_default_usage (FILE *fp)
 bfd_boolean
 ar_emul_append (bfd **after_bfd, char *file_name, const char *target,
 		bfd_boolean verbose, bfd_boolean flatten)
+{
+  bfd *new_bfd;
+
+  new_bfd = bfd_openr (file_name, target);
+  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
+  if (bin_dummy_emulation.ar_append)
+    return bin_dummy_emulation.ar_append (after_bfd, new_bfd,
+					  verbose, flatten);
+
+  return FALSE;
+}
+
+bfd_boolean
+ar_emul_append_bfd (bfd **after_bfd, bfd *new_bfd,
+		bfd_boolean verbose, bfd_boolean flatten)
 {
   if (bin_dummy_emulation.ar_append)
-    return bin_dummy_emulation.ar_append (after_bfd, file_name, target,
+    return bin_dummy_emulation.ar_append (after_bfd, new_bfd,
 					  verbose, flatten);
 
   return FALSE;
@@ -93,14 +108,9 @@ do_ar_emul_append (bfd **after_bfd, bfd *new_bfd,
 }
 
 bfd_boolean
-ar_emul_default_append (bfd **after_bfd, char *file_name,
-			const char *target, bfd_boolean verbose,
-			bfd_boolean flatten)
+ar_emul_default_append (bfd **after_bfd, bfd *new_bfd,
+			bfd_boolean verbose, bfd_boolean flatten)
 {
-  bfd *new_bfd;
-
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
   return do_ar_emul_append (after_bfd, new_bfd, verbose, flatten, any_ok);
 }
 
@@ -108,23 +118,34 @@ bfd_boolean
 ar_emul_replace (bfd **after_bfd, char *file_name, const char *target,
 		 bfd_boolean verbose)
 {
+  bfd *new_bfd;
+
+  new_bfd = bfd_openr (file_name, target);
+  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
+
   if (bin_dummy_emulation.ar_replace)
-    return bin_dummy_emulation.ar_replace (after_bfd, file_name,
-					   target, verbose);
+    return bin_dummy_emulation.ar_replace (after_bfd, new_bfd,
+					   verbose);
 
   return FALSE;
 }
 
 bfd_boolean
-ar_emul_default_replace (bfd **after_bfd, char *file_name,
-			 const char *target, bfd_boolean verbose)
+ar_emul_replace_bfd (bfd **after_bfd, bfd *new_bfd,
+		 bfd_boolean verbose)
 {
-  bfd *new_bfd;
+  if (bin_dummy_emulation.ar_replace)
+    return bin_dummy_emulation.ar_replace (after_bfd, new_bfd,
+					   verbose);
 
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
+  return FALSE;
+}
 
-  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, file_name);
+bfd_boolean
+ar_emul_default_replace (bfd **after_bfd, bfd *new_bfd,
+			 bfd_boolean verbose)
+{
+  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, bfd_get_filename (new_bfd));
 
   new_bfd->archive_next = *after_bfd;
   *after_bfd = new_bfd;
diff --git a/binutils/binemul.h b/binutils/binemul.h
index d4a14edfeb..57b2e20b7a 100644
--- a/binutils/binemul.h
+++ b/binutils/binemul.h
@@ -30,15 +30,19 @@ extern void ar_emul_usage (FILE *);
 extern void ar_emul_default_usage (FILE *);
 extern bfd_boolean ar_emul_append (bfd **, char *, const char *,
 				   bfd_boolean, bfd_boolean);
-extern bfd_boolean ar_emul_default_append (bfd **, char *, const char *,
+extern bfd_boolean ar_emul_append_bfd (bfd **, bfd *,
+				       bfd_boolean, bfd_boolean);
+extern bfd_boolean ar_emul_default_append (bfd **, bfd *,
 					   bfd_boolean, bfd_boolean);
 extern bfd_boolean do_ar_emul_append (bfd **, bfd *,
 				      bfd_boolean, bfd_boolean,
 				      bfd_boolean (*)(bfd *));
 extern bfd_boolean ar_emul_replace (bfd **, char *, const char *,
 				    bfd_boolean);
-extern bfd_boolean ar_emul_default_replace (bfd **, char *,
-					    const char *, bfd_boolean);
+extern bfd_boolean ar_emul_replace_bfd (bfd **, bfd *,
+					bfd_boolean);
+extern bfd_boolean ar_emul_default_replace (bfd **, bfd *,
+					    bfd_boolean);
 extern bfd_boolean ar_emul_parse_arg (char *);
 extern bfd_boolean ar_emul_default_parse_arg (char *);
 
@@ -61,9 +65,8 @@ typedef struct bin_emulation_xfer_struct
 {
   /* Print out the extra options.  */
   void (* ar_usage) (FILE *fp);
-  bfd_boolean (* ar_append) (bfd **, char *, const char *, bfd_boolean,
-			     bfd_boolean);
-  bfd_boolean (* ar_replace) (bfd **, char *, const char *, bfd_boolean);
+  bfd_boolean (* ar_append) (bfd **, bfd *, bfd_boolean, bfd_boolean);
+  bfd_boolean (* ar_replace) (bfd **, bfd *, bfd_boolean);
   bfd_boolean (* ar_parse_arg) (char *);
 }
 bin_emulation_xfer_type;
diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
index 35a5a67041..70d97565b5 100644
--- a/binutils/doc/binutils.texi
+++ b/binutils/doc/binutils.texi
@@ -170,7 +170,7 @@ in the section entitled ``GNU Free Documentation License''.
 @c man title ar create, modify, and extract from archives
 
 @smallexample
-ar [-]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@option{--output} @var{dirname}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
+ar [-]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@option{--output} @var{dirname}] [@option{--record-libdeps} @var{libdeps}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
 ar -M [ <mri-script ]
 @end smallexample
 
@@ -196,7 +196,9 @@ characters (typical of formats related to coff).
 @cindex libraries
 @command{ar} is considered a binary utility because archives of this sort
 are most often used as @dfn{libraries} holding commonly needed
-subroutines.
+subroutines.  Since libraries often will depend on other libraries,
+@command{ar} can also record the dependencies of a library when the
+@option{--record-libdeps} option is specified.
 
 @cindex symbol index
 @command{ar} creates an index to the symbols defined in relocatable
@@ -254,7 +256,7 @@ program.
 
 @smallexample
 @c man begin SYNOPSIS ar
-ar [@option{-X32_64}] [@option{-}]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@option{--output} @var{dirname}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
+ar [@option{-X32_64}] [@option{-}]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@option{--output} @var{dirname}] [@option{--record-libdeps} @var{libdeps}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
 @c man end
 @end smallexample
 
@@ -452,6 +454,13 @@ This modifier is accepted but not used.
 @c whaffor ar l modifier??? presumably compat; with
 @c what???---doc@@cygnus.com, 25jan91
 
+@item L
+Specify dependencies of this library.  The dependencies must immediately
+follow this option character, must use the same syntax as the linker
+command line, and must be specified within a single argument.  I.e., if
+multiple items are needed, they must be quoted to form a single command
+line argument.  For example @samp{L "-L/usr/local/lib -lmydep1 -lmydep2"}
+
 @item N
 Uses the @var{count} parameter.  This is used if there are multiple
 entries in the archive with the same name.  Extract or delete instance
@@ -592,6 +601,10 @@ Note - although the presence of this option does imply a @option{x}
 extraction operation that option must still be included on the command
 line.
 
+@item --record-libdeps @var{libdeps}
+The @option{--record-libdeps} option is identical to the @option{L} modifier,
+just handled in long form.
+
 @end table
 @c man end
 
diff --git a/binutils/emul_aix.c b/binutils/emul_aix.c
index c54e1083e5..726cb3b70c 100644
--- a/binutils/emul_aix.c
+++ b/binutils/emul_aix.c
@@ -67,30 +67,20 @@ check_aix (bfd *try_bfd)
 }
 
 static bfd_boolean
-ar_emul_aix_append (bfd **after_bfd, char *file_name, const char *target,
+ar_emul_aix_append (bfd **after_bfd, bfd *new_bfd,
 		    bfd_boolean verbose, bfd_boolean flatten)
 {
-  bfd *new_bfd;
-
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
-
   return do_ar_emul_append (after_bfd, new_bfd, verbose, flatten, check_aix);
 }
 
 static bfd_boolean
-ar_emul_aix_replace (bfd **after_bfd, char *file_name, const char *target,
+ar_emul_aix_replace (bfd **after_bfd, bfd *new_bfd,
 		     bfd_boolean verbose)
 {
-  bfd *new_bfd;
-
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
-
   if (!check_aix (new_bfd))
     return FALSE;
 
-  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, file_name);
+  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, bfd_get_filename (new_bfd));
 
   new_bfd->archive_next = *after_bfd;
   *after_bfd = new_bfd;
diff --git a/binutils/testsuite/binutils-all/ar.exp b/binutils/testsuite/binutils-all/ar.exp
index 5a9d27c6d0..273a0a3909 100644
--- a/binutils/testsuite/binutils-all/ar.exp
+++ b/binutils/testsuite/binutils-all/ar.exp
@@ -715,6 +715,46 @@ proc many_files { } {
     pass $testname
 }
 
+proc test_add_dependencies { } {
+    global AR
+    global AS
+    global srcdir
+    global subdir
+    global obj
+
+    set testname "ar adding library dependencies"
+
+    if ![binutils_assemble $srcdir/$subdir/bintest.s tmpdir/bintest.${obj}] {
+	unresolved $testname
+	return
+    }
+
+    if [is_remote host] {
+	set archive artest.a
+	set objfile [remote_download host tmpdir/bintest.${obj}]
+	remote_file host delete $archive
+    } else {
+	set archive tmpdir/artest.a
+	set objfile tmpdir/bintest.${obj}
+    }
+
+    remote_file build delete tmpdir/artest.a
+
+    set got [binutils_run $AR "-r -c $archive --record-libdeps /foo/bar ${objfile}"]
+    if ![string match "" $got] {
+	fail $testname
+	return
+    }
+
+    set got [binutils_run $AR "-t $archive"]
+    if ![string match "*bintest.${obj}\r__.LIBDEP*" $got] {
+	fail $testname
+	return
+    }
+
+    pass $testname
+}
+
 # Run the tests.
 
 # Only run the bfdtest checks if the programs exist.  Since these
@@ -743,6 +783,7 @@ move_an_element
 empty_archive
 extract_an_element
 many_files
+test_add_dependencies
 
 if { [is_elf_format] && [supports_gnu_unique] } {
     unique_symbol
-- 
2.20.1

Re: [PATCH] dependency list for static libraries

[Nick Clifton]

Hi Howard,

> Sorry for the noise, this attached is the same as previous but with fixes
> to whitespace / indentation.

Thanks - the patch looks good now, although there are still a couple of minor
issues - and one major issue:

  * The new test fails for the alpha-vms target.  This is not serious however
    as almost all of the ar tests fail for this target.  One day I will track
    down what is going wrong and either fix it, or arrange to skip these tests
    for alpha-vms.

  * There are a couple of minor formatting issues.  Specifically: comments should
    end in a period followed by two spaces before the closing marker. /* Like this.  */
    Plus function calls should have a space between the function name and the 
    opening parenthesis of the argument list.  like (this)

The major issue is that I would really like for this extension to be supported
by the LLVM community as well.  It would be a shame to add it to the binutils
only to have a different solution implemented there.  I might have misread the
emails but I believe that Fangrui still has some misgivings about this approach.
Is that correct ?  If so, then I would very much like to see them resolved 
before we commit the patch.

Cheers
  Nick

Re: [PATCH] dependency list for static libraries

[Howard Chu]

Nick Clifton wrote:
> Hi Howard,
> 
>> Sorry for the noise, this attached is the same as previous but with fixes
>> to whitespace / indentation.
> 
> Thanks - the patch looks good now, although there are still a couple of minor
> issues - and one major issue:
> 
>   * The new test fails for the alpha-vms target.  This is not serious however
>     as almost all of the ar tests fail for this target.  One day I will track
>     down what is going wrong and either fix it, or arrange to skip these tests
>     for alpha-vms.
> 
>   * There are a couple of minor formatting issues.  Specifically: comments should
>     end in a period followed by two spaces before the closing marker. /* Like this.  */
>     Plus function calls should have a space between the function name and the 
>     opening parenthesis of the argument list.  like (this)

I can send an updated patch after the major issue is addressed.

> The major issue is that I would really like for this extension to be supported
> by the LLVM community as well.  It would be a shame to add it to the binutils
> only to have a different solution implemented there.  I might have misread the
> emails but I believe that Fangrui still has some misgivings about this approach.
> Is that correct ?  If so, then I would very much like to see them resolved 
> before we commit the patch.

It still seems pretty obvious that handling dependencies once at the library
level is more efficient than sticking a bunch of free-form notes in every single
object file. And it should also be obvious that choice of library is a buildtime
decision, not a decision made solely at the time the source code is written.
(And for shared libraries, it's a runtime decision - even further removed from
the time of code being written.)

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] dependency list for static libraries

[Fangrui Song]

Thanks to Nick for mentioning me :)

On 2020-09-23, Howard Chu wrote:
>Nick Clifton wrote:
>> Hi Howard,
>>
>>> Sorry for the noise, this attached is the same as previous but with fixes
>>> to whitespace / indentation.
>>
>> Thanks - the patch looks good now, although there are still a couple of minor
>> issues - and one major issue:
>>
>>   * The new test fails for the alpha-vms target.  This is not serious however
>>     as almost all of the ar tests fail for this target.  One day I will track
>>     down what is going wrong and either fix it, or arrange to skip these tests
>>     for alpha-vms.
>>
>>   * There are a couple of minor formatting issues.  Specifically: comments should
>>     end in a period followed by two spaces before the closing marker. /* Like this.  */
>>     Plus function calls should have a space between the function name and the
>>     opening parenthesis of the argument list.  like (this)
>
>I can send an updated patch after the major issue is addressed.
>
>> The major issue is that I would really like for this extension to be supported
>> by the LLVM community as well.  It would be a shame to add it to the binutils
>> only to have a different solution implemented there.  I might have misread the
>> emails but I believe that Fangrui still has some misgivings about this approach.
>> Is that correct ?  If so, then I would very much like to see them resolved
>> before we commit the patch.
>
>It still seems pretty obvious that handling dependencies once at the library
>level is more efficient than sticking a bunch of free-form notes in every single
>object file. And it should also be obvious that choice of library is a buildtime
>decision, not a decision made solely at the time the source code is written.
>(And for shared libraries, it's a runtime decision - even further removed from
>the time of code being written.)

 From my experience (I have investigated hundreds of link issues),
archive link errors are usually caused by the following two problems:

* There is a backward reference between two archives. This is related to a.out/ELF style linker behaviors
(https://sourceware.org/pipermail/binutils/2020-September/113194.html ).
   + A dependency exists but is incorrectly omitted. The linker actually captures a layering problem.
   + A dependency is intentionally omitted. If the linker allows backward references, this will not be a problem.

   I have recently thought a lot on the topic and written http://lld.llvm.org/ELF/warn_backrefs.html

* Some archives are incorrectly omitted: the executable uses A, but A's
   dependency B is not on the command line.  Now I hope Howard can clarify on this
   topic:) What do you want to achieve with the dependency recorded in the archive?
   I assume that you want the linker to smartly link B. This is indeed the #pragma
   comment(lib, ...) and .deplibs feature clang/LLD support.

   The proposal does not mention the ld part, which seems important. How
   does ld handle __.LIBDEP ?

   + The executable also references B and you don't want to write its dependency on B.
     This is the traditional --copy-dt-needed-entries behavior (that the
     designers of gold explicitly do not want to support). The linker traverses all
     dependencies of shared objects recursively. This is against the "include what
     you use" philosophy.  https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/WhyIWYU.md
     It imposes a lot of extra work for the linker. It makes the build brittle - if
     A removes its dependency on B, you need to find and fix all the transitive users
     of A. I hope this proposal is not about the bullet point.
   + The executable does not references B. I am still unsure why you want to move the dependency information from the
     object file to the archive.
     - First, I want to mention that in some cases archives are not needed. For
       some use cases people propose thin archives.  For some thin archive use cases,
       --start-lib --end-lib surrounded object files are probably more useful.  gold
       pioneered the idea but GNU ld still does not support the feature
       https://sourceware.org/bugzilla/show_bug.cgi?id=24600
     - Moving the dependency to the archive can actually increases the work for
       the linker.  Say the two members of A are a0.o and a1.o. If a0.o depends on B
       but a0.o is not selected, then B does not need to be linked. Having the
       dependency in the archive will cause B to be linked.
     - How do you ensure the dependency information does not become stale? i.e.
       How do you maintain the dependency information when members are added to/removed from
       the archive? The information is more difficult to maintain than the index, which only contains
       the definitions. You may argue that the archive is finalized after it is created - then
       we go back to square one, --start-lib may be more suitable.

---

Last,

>  fprintf (s, _("  [L LIBDEPS]  - specify dependencies of this library\n"));

In llvm-ar, 'L' is an extension used with 'q' to add all members of an archive to the current archive.
It is similar to ADDLIB (https://sourceware.org/binutils/docs/binutils/ar-scripts.html )
Hope the two tools don't have conflicting operations.

Re: [PATCH] dependency list for static libraries

[Howard Chu]

Fangrui Song wrote:
> Thanks to Nick for mentioning me :)
> 
> On 2020-09-23, Howard Chu wrote:
>> Nick Clifton wrote:

>> I can send an updated patch after the major issue is addressed.

>>> The major issue is that I would really like for this extension to be supported
>>> by the LLVM community as well.  It would be a shame to add it to the binutils
>>> only to have a different solution implemented there.  I might have misread the
>>> emails but I believe that Fangrui still has some misgivings about this approach.
>>> Is that correct ?  If so, then I would very much like to see them resolved
>>> before we commit the patch.
>>
>> It still seems pretty obvious that handling dependencies once at the library
>> level is more efficient than sticking a bunch of free-form notes in every single
>> object file. And it should also be obvious that choice of library is a buildtime
>> decision, not a decision made solely at the time the source code is written.
>> (And for shared libraries, it's a runtime decision - even further removed from
>> the time of code being written.)
> 
> From my experience (I have investigated hundreds of link issues),
> archive link errors are usually caused by the following two problems:
> 
> * There is a backward reference between two archives. This is related to a.out/ELF style linker behaviors
> (https://sourceware.org/pipermail/binutils/2020-September/113194.html ).
>   + A dependency exists but is incorrectly omitted. The linker actually captures a layering problem.
>   + A dependency is intentionally omitted. If the linker allows backward references, this will not be a problem.
> 
>   I have recently thought a lot on the topic and written http://lld.llvm.org/ELF/warn_backrefs.html

That is not a problem I encounter frequently, and is not in the scope of what
I'm concerned with.

> * Some archives are incorrectly omitted: the executable uses A, but A's
>   dependency B is not on the command line.  Now I hope Howard can clarify on this
>   topic:) What do you want to achieve with the dependency recorded in the archive?
>   I assume that you want the linker to smartly link B.

Yes.

>   This is indeed the #pragma
>   comment(lib, ...) and .deplibs feature clang/LLD support.

Perhaps, but I remain skeptical.

>   The proposal does not mention the ld part, which seems important. How
>   does ld handle __.LIBDEP ?

ld effectively inserts the flags from __.LIBDEP into its command line immediately
following its archive.

>   + The executable does not references B. I am still unsure why you want to move the dependency information from the
>     object file to the archive.

The most common case is we reference a library we know about, and it has dependencies
we don't know about. An example I currently have in mind is libzmq, which may or may
not be built against libsodium, and may or may not have other library dependencies
as well. The "traditional" solution to this for the past couple decades has been to
use pkg-config or libtool .la files to propagate this information but those are both
ugly hacks that require excessive tooling to support. The information clearly belongs
to the archive file itself.

>     - First, I want to mention that in some cases archives are not needed.

Irrelevant. I'm talking about software that is shipped in library form.

>     - Moving the dependency to the archive can actually increases the work for
>       the linker.  Say the two members of A are a0.o and a1.o. If a0.o depends on B
>       but a0.o is not selected, then B does not need to be linked. Having the
>       dependency in the archive will cause B to be linked.
>     - How do you ensure the dependency information does not become stale? i.e.
>       How do you maintain the dependency information when members are added to/removed from
>       the archive? The information is more difficult to maintain than the index, which only contains
>       the definitions. You may argue that the archive is finalized after it is created - then
>       we go back to square one, --start-lib may be more suitable.

How do you maintain the dependency information of a shared library? This is a
problem developers already deal with, not a new problem.
> 
> ---
> 
> Last,
> 
>>  fprintf (s, _("  [L LIBDEPS]  - specify dependencies of this library\n"));
> 
> In llvm-ar, 'L' is an extension used with 'q' to add all members of an archive to the current archive.
> It is similar to ADDLIB (https://sourceware.org/binutils/docs/binutils/ar-scripts.html )
> Hope the two tools don't have conflicting operations.

We could just use 'l' instead, which has historically been a no-op. The specific choice
of letter option here is not significant.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] dependency list for static libraries

[Howard Chu]

Howard Chu wrote:
>> * Some archives are incorrectly omitted: the executable uses A, but A's
>>   dependency B is not on the command line.  Now I hope Howard can clarify on this
>>   topic:) What do you want to achieve with the dependency recorded in the archive?
>>   I assume that you want the linker to smartly link B.
> Yes.
> 
>>   This is indeed the #pragma
>>   comment(lib, ...) and .deplibs feature clang/LLD support.
> Perhaps, but I remain skeptical.
> 
In particular, the feature you're proposing only works with ELF object files,
not with a.out files or any of the myriad non-ELF object file formats. It is
attacking the problem at the wrong level.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] dependency list for static libraries

[Howard Chu]

Howard Chu wrote:
> Fangrui Song wrote:
>>   + The executable does not references B. I am still unsure why you want to move the dependency information from the
>>     object file to the archive.
> 
> The most common case is we reference a library we know about, and it has dependencies
> we don't know about. An example I currently have in mind is libzmq, which may or may
> not be built against libsodium, and may or may not have other library dependencies
> as well. The "traditional" solution to this for the past couple decades has been to
> use pkg-config or libtool .la files to propagate this information but those are both
> ugly hacks that require excessive tooling to support. The information clearly belongs
> to the archive file itself.

The libzmq/libsodium example is particularly illustrative. The symbols libzmq references
may be entirely resolved by its bundled TweetNaCl, or may be satisfied by libsodium. The
choice of which to use is made at build time and either one can be substituted in without
making any alterations to source code or other build scripts, or recompiling any sources.
As such, embedding the name of a dependency into libzmq's object files is not a solution
for this situation. The knowledge only exists at build time, chosen by whoever is packaging
the software. It would be ridiculous to require the library to be recompiled to change
this embedded dependency, when no such step was needed before.

There's another argument of perspective - there's a multi-decade expectation that object
files are passive, with no side effects. If I reference foo.o in a link command, all I
get is foo.o, nothing else. There's also an expectation that if I reference a library in
a link command, I may get arbitrarily many other components. That's a fundamental difference
between individual object files and objects collected into a library. It would be a mistake
to violate these norms.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] dependency list for static libraries

[Howard Chu]

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

Nick Clifton wrote:
> Hi Howard,
> 
>> Sorry for the noise, this attached is the same as previous but with fixes
>> to whitespace / indentation.
> 
> Thanks - the patch looks good now, although there are still a couple of minor
> issues - and one major issue:
> 
>   * The new test fails for the alpha-vms target.  This is not serious however
>     as almost all of the ar tests fail for this target.  One day I will track
>     down what is going wrong and either fix it, or arrange to skip these tests
>     for alpha-vms.
> 
>   * There are a couple of minor formatting issues.  Specifically: comments should
>     end in a period followed by two spaces before the closing marker. /* Like this.  */
>     Plus function calls should have a space between the function name and the 
>     opening parenthesis of the argument list.  like (this)

Whitespace fixes done, attached.
> 
> The major issue is that I would really like for this extension to be supported
> by the LLVM community as well.  It would be a shame to add it to the binutils
> only to have a different solution implemented there.  I might have misread the
> emails but I believe that Fangrui still has some misgivings about this approach.
> Is that correct ?  If so, then I would very much like to see them resolved 
> before we commit the patch.

I haven't seen any further response from that side. Still seems obvious that a solution
that revolves around plaintext is easier to maintain than one that requires access to
the original source code+build environment and a recompile in order to modify/update.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-ar-support-for-dependencies.patch --]
[-- Type: text/x-patch; name="0001-Add-ar-support-for-dependencies.patch", Size: 17173 bytes --]

From 9232ff64d1b2b9fa8031c8bd6785296569e8c021 Mon Sep 17 00:00:00 2001
From: Howard Chu <hyc@symas.com>
Date: Thu, 3 Sep 2020 17:34:16 +0100
Subject: [PATCH] Add ar support for dependencies

Record -L/-l dependencies of a static library in a __.LIBDEP entry.
Options should be passed literally as a single argument, e.g.
 ar rL "-L/usr/local/lib -lfoo -lbar" libnew.a obj1.o obj2.o
---
 binutils/NEWS                          |  5 ++
 binutils/ar.c                          | 85 ++++++++++++++++++++++++--
 binutils/binemul.c                     | 53 +++++++++++-----
 binutils/binemul.h                     | 15 +++--
 binutils/doc/binutils.texi             | 19 ++++--
 binutils/emul_aix.c                    | 16 +----
 binutils/testsuite/binutils-all/ar.exp | 41 +++++++++++++
 7 files changed, 189 insertions(+), 45 deletions(-)

diff --git a/binutils/NEWS b/binutils/NEWS
index 35e4e303e1..02a19ea09b 100644
--- a/binutils/NEWS
+++ b/binutils/NEWS
@@ -1,5 +1,10 @@
 -*- text -*-
 
+* The ar tool's previously unused l modifier is now used for specifying
+  dependencies of a static library. The arguments of this option
+  (or --record-libdeps long form option) will be stored verbatim in the
+  __.LIBDEP member of the archive, which the linker may read at link time.
+
 * Readelf can now display the contents of LTO symbol table sections when asked
   to do so via the --lto-syms command line option.
 
diff --git a/binutils/ar.c b/binutils/ar.c
index 85b342a650..60fc51efcf 100644
--- a/binutils/ar.c
+++ b/binutils/ar.c
@@ -138,6 +138,11 @@ static bfd_boolean full_pathname = FALSE;
 /* Whether to create a "thin" archive (symbol index only -- no files).  */
 static bfd_boolean make_thin_archive = FALSE;
 
+#define LIBDEPS	"__.LIBDEP"
+/* Dependencies to store in __.LIBDEP for linker to use.  */
+char *Libdeps;
+static bfd * Libdeps_bfd;
+
 static int show_version = 0;
 
 static int show_help = 0;
@@ -166,6 +171,7 @@ static struct option long_options[] =
   {"target", required_argument, NULL, OPTION_TARGET},
   {"version", no_argument, &show_version, 1},
   {"output", required_argument, NULL, OPTION_OUTPUT},
+  {"record-libdeps", required_argument, NULL, 'l'},
   {NULL, no_argument, NULL, 0}
 };
 
@@ -329,6 +335,7 @@ usage (int help)
   fprintf (s, _(" generic modifiers:\n"));
   fprintf (s, _("  [c]          - do not warn if the library had to be created\n"));
   fprintf (s, _("  [s]          - create an archive index (cf. ranlib)\n"));
+  fprintf (s, _("  [l LIBDEPS]  - specify dependencies of this library\n"));
   fprintf (s, _("  [S]          - do not build a symbol table\n"));
   fprintf (s, _("  [T]          - make a thin archive\n"));
   fprintf (s, _("  [v]          - be verbose\n"));
@@ -336,6 +343,7 @@ usage (int help)
   fprintf (s, _("  @<file>      - read options from <file>\n"));
   fprintf (s, _("  --target=BFDNAME - specify the target object format as BFDNAME\n"));
   fprintf (s, _("  --output=DIRNAME - specify the output directory for extraction operations\n"));
+  fprintf (s, _("  --record-libdeps=LIBDEPS - specify dependencies of this library\n"));
 #if BFD_SUPPORTS_PLUGINS
   fprintf (s, _(" optional:\n"));
   fprintf (s, _("  --plugin <p> - load the specified plugin\n"));
@@ -487,7 +495,7 @@ decode_options (int argc, char **argv)
       argv = new_argv;
     }
 
-  while ((c = getopt_long (argc, argv, "hdmpqrtxlcoOVsSuvabiMNfPTDU",
+  while ((c = getopt_long (argc, argv, "hdmpqrtxl:coOVsSuvabiMNfPTDU",
 			   long_options, NULL)) != EOF)
     {
       switch (c)
@@ -535,6 +543,9 @@ decode_options (int argc, char **argv)
           operation = extract;
           break;
         case 'l':
+          if (Libdeps != NULL)
+            fatal (_("Libdeps specified more than once"));
+          Libdeps = optarg;
           break;
         case 'c':
           silent_create = 1;
@@ -847,6 +858,50 @@ main (int argc, char **argv)
       if (operation == extract && bfd_is_thin_archive (arch))
 	fatal (_("`x' cannot be used on thin archives."));
 
+      if (Libdeps)
+	{
+	  char **new_files;
+	  bfd_size_type reclen = strlen (Libdeps)+1;
+
+	  /* Create a bfd to contain the dependencies.
+	     It inherits its type from arch, but we must
+	     set the type to "binary" otherwise bfd_bwrite()
+	     will fail. After writing, we must set the type
+	     back to "plugin" otherwise adding it to the
+	     archive will fail.  */
+	  Libdeps_bfd = bfd_create (LIBDEPS, arch);
+	  if (Libdeps_bfd == NULL)
+	    fatal (_("Cannot create Libdeps record."));
+
+	  if (bfd_find_target ("binary", Libdeps_bfd) == NULL)
+	    fatal (_("Cannot set Libdeps record type to binary."));
+
+	  if (! bfd_set_format (Libdeps_bfd, bfd_object))
+	    fatal (_("Cannot set Libdeps object format."));
+
+	  if (! bfd_make_writable (Libdeps_bfd))
+	    fatal (_("Cannot make Libdeps object writable."));
+
+	  if (bfd_bwrite (Libdeps, reclen, Libdeps_bfd) != reclen)
+	    fatal (_("Cannot write Libdeps record."));
+
+	  if (! bfd_make_readable (Libdeps_bfd))
+	    fatal (_("Cannot make Libdeps object readable."));
+
+	  if (bfd_find_target ("plugin", Libdeps_bfd) == NULL)
+	    fatal (_("Cannot reset Libdeps record type."));
+
+	  /* Append our Libdeps record to the list of files
+	     being operated on.  */
+	  new_files = xmalloc ((file_count+2) * sizeof(char *));
+	  for (i=0; i<file_count; i++)
+	    new_files[i] = files[i];
+	  new_files[i++] = LIBDEPS;
+	  file_count = i;
+	  files = new_files;
+	  files[i] = NULL;
+	}
+
       switch (operation)
 	{
 	case print_table:
@@ -1432,6 +1487,7 @@ replace_members (bfd *arch, char **files_to_move, bfd_boolean quick)
 				normalize (bfd_get_filename (current), arch)) == 0
 		  && current->arelt_data != NULL)
 		{
+		  bfd_boolean replaced;
 		  if (newer_only)
 		    {
 		      struct stat fsbuf, asbuf;
@@ -1453,8 +1509,18 @@ replace_members (bfd *arch, char **files_to_move, bfd_boolean quick)
 
 		  after_bfd = get_pos_bfd (&arch->archive_next, pos_after,
 					   bfd_get_filename (current));
-		  if (ar_emul_replace (after_bfd, *files_to_move,
-				       target, verbose))
+		  if (Libdeps && FILENAME_CMP (normalize (*files_to_move, arch),
+					       LIBDEPS) == 0)
+		    {
+		      replaced = ar_emul_replace_bfd (after_bfd, Libdeps_bfd,
+						      verbose);
+		    }
+		  else
+		    {
+		      replaced = ar_emul_replace (after_bfd, *files_to_move,
+						  target, verbose);
+		    }
+		  if (replaced)
 		    {
 		      /* Snip out this entry from the chain.  */
 		      *current_ptr = (*current_ptr)->archive_next;
@@ -1470,9 +1536,16 @@ replace_members (bfd *arch, char **files_to_move, bfd_boolean quick)
       /* Add to the end of the archive.  */
       after_bfd = get_pos_bfd (&arch->archive_next, pos_end, NULL);
 
-      if (ar_emul_append (after_bfd, *files_to_move, target,
-			  verbose, make_thin_archive))
-	changed = TRUE;
+      if (Libdeps && FILENAME_CMP (normalize (*files_to_move, arch), LIBDEPS) == 0)
+        {
+	  changed |= ar_emul_append_bfd (after_bfd, Libdeps_bfd,
+					 verbose, make_thin_archive);
+	}
+      else
+        {
+	  changed |= ar_emul_append (after_bfd, *files_to_move, target,
+				     verbose, make_thin_archive);
+	}
 
     next_file:;
 
diff --git a/binutils/binemul.c b/binutils/binemul.c
index 7c71b5b78c..8a0512ebd9 100644
--- a/binutils/binemul.c
+++ b/binutils/binemul.c
@@ -41,9 +41,24 @@ ar_emul_default_usage (FILE *fp)
 bfd_boolean
 ar_emul_append (bfd **after_bfd, char *file_name, const char *target,
 		bfd_boolean verbose, bfd_boolean flatten)
+{
+  bfd *new_bfd;
+
+  new_bfd = bfd_openr (file_name, target);
+  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
+  if (bin_dummy_emulation.ar_append)
+    return bin_dummy_emulation.ar_append (after_bfd, new_bfd,
+					  verbose, flatten);
+
+  return FALSE;
+}
+
+bfd_boolean
+ar_emul_append_bfd (bfd **after_bfd, bfd *new_bfd,
+		bfd_boolean verbose, bfd_boolean flatten)
 {
   if (bin_dummy_emulation.ar_append)
-    return bin_dummy_emulation.ar_append (after_bfd, file_name, target,
+    return bin_dummy_emulation.ar_append (after_bfd, new_bfd,
 					  verbose, flatten);
 
   return FALSE;
@@ -93,14 +108,9 @@ do_ar_emul_append (bfd **after_bfd, bfd *new_bfd,
 }
 
 bfd_boolean
-ar_emul_default_append (bfd **after_bfd, char *file_name,
-			const char *target, bfd_boolean verbose,
-			bfd_boolean flatten)
+ar_emul_default_append (bfd **after_bfd, bfd *new_bfd,
+			bfd_boolean verbose, bfd_boolean flatten)
 {
-  bfd *new_bfd;
-
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
   return do_ar_emul_append (after_bfd, new_bfd, verbose, flatten, any_ok);
 }
 
@@ -108,23 +118,34 @@ bfd_boolean
 ar_emul_replace (bfd **after_bfd, char *file_name, const char *target,
 		 bfd_boolean verbose)
 {
+  bfd *new_bfd;
+
+  new_bfd = bfd_openr (file_name, target);
+  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
+
   if (bin_dummy_emulation.ar_replace)
-    return bin_dummy_emulation.ar_replace (after_bfd, file_name,
-					   target, verbose);
+    return bin_dummy_emulation.ar_replace (after_bfd, new_bfd,
+					   verbose);
 
   return FALSE;
 }
 
 bfd_boolean
-ar_emul_default_replace (bfd **after_bfd, char *file_name,
-			 const char *target, bfd_boolean verbose)
+ar_emul_replace_bfd (bfd **after_bfd, bfd *new_bfd,
+		 bfd_boolean verbose)
 {
-  bfd *new_bfd;
+  if (bin_dummy_emulation.ar_replace)
+    return bin_dummy_emulation.ar_replace (after_bfd, new_bfd,
+					   verbose);
 
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
+  return FALSE;
+}
 
-  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, file_name);
+bfd_boolean
+ar_emul_default_replace (bfd **after_bfd, bfd *new_bfd,
+			 bfd_boolean verbose)
+{
+  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, bfd_get_filename (new_bfd));
 
   new_bfd->archive_next = *after_bfd;
   *after_bfd = new_bfd;
diff --git a/binutils/binemul.h b/binutils/binemul.h
index d4a14edfeb..57b2e20b7a 100644
--- a/binutils/binemul.h
+++ b/binutils/binemul.h
@@ -30,15 +30,19 @@ extern void ar_emul_usage (FILE *);
 extern void ar_emul_default_usage (FILE *);
 extern bfd_boolean ar_emul_append (bfd **, char *, const char *,
 				   bfd_boolean, bfd_boolean);
-extern bfd_boolean ar_emul_default_append (bfd **, char *, const char *,
+extern bfd_boolean ar_emul_append_bfd (bfd **, bfd *,
+				       bfd_boolean, bfd_boolean);
+extern bfd_boolean ar_emul_default_append (bfd **, bfd *,
 					   bfd_boolean, bfd_boolean);
 extern bfd_boolean do_ar_emul_append (bfd **, bfd *,
 				      bfd_boolean, bfd_boolean,
 				      bfd_boolean (*)(bfd *));
 extern bfd_boolean ar_emul_replace (bfd **, char *, const char *,
 				    bfd_boolean);
-extern bfd_boolean ar_emul_default_replace (bfd **, char *,
-					    const char *, bfd_boolean);
+extern bfd_boolean ar_emul_replace_bfd (bfd **, bfd *,
+					bfd_boolean);
+extern bfd_boolean ar_emul_default_replace (bfd **, bfd *,
+					    bfd_boolean);
 extern bfd_boolean ar_emul_parse_arg (char *);
 extern bfd_boolean ar_emul_default_parse_arg (char *);
 
@@ -61,9 +65,8 @@ typedef struct bin_emulation_xfer_struct
 {
   /* Print out the extra options.  */
   void (* ar_usage) (FILE *fp);
-  bfd_boolean (* ar_append) (bfd **, char *, const char *, bfd_boolean,
-			     bfd_boolean);
-  bfd_boolean (* ar_replace) (bfd **, char *, const char *, bfd_boolean);
+  bfd_boolean (* ar_append) (bfd **, bfd *, bfd_boolean, bfd_boolean);
+  bfd_boolean (* ar_replace) (bfd **, bfd *, bfd_boolean);
   bfd_boolean (* ar_parse_arg) (char *);
 }
 bin_emulation_xfer_type;
diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
index 6203fde887..32a5acc3b9 100644
--- a/binutils/doc/binutils.texi
+++ b/binutils/doc/binutils.texi
@@ -170,7 +170,7 @@ in the section entitled ``GNU Free Documentation License''.
 @c man title ar create, modify, and extract from archives
 
 @smallexample
-ar [-]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@option{--output} @var{dirname}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
+ar [-]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@option{--output} @var{dirname}] [@option{--record-libdeps} @var{libdeps}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
 ar -M [ <mri-script ]
 @end smallexample
 
@@ -196,7 +196,9 @@ characters (typical of formats related to coff).
 @cindex libraries
 @command{ar} is considered a binary utility because archives of this sort
 are most often used as @dfn{libraries} holding commonly needed
-subroutines.
+subroutines.  Since libraries often will depend on other libraries,
+@command{ar} can also record the dependencies of a library when the
+@option{--record-libdeps} option is specified.
 
 @cindex symbol index
 @command{ar} creates an index to the symbols defined in relocatable
@@ -254,7 +256,7 @@ program.
 
 @smallexample
 @c man begin SYNOPSIS ar
-ar [@option{-X32_64}] [@option{-}]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@option{--output} @var{dirname}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
+ar [@option{-X32_64}] [@option{-}]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@option{--output} @var{dirname}] [@option{--record-libdeps} @var{libdeps}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
 @c man end
 @end smallexample
 
@@ -448,9 +450,14 @@ member must be present as the @var{relpos} argument, before the
 @var{archive} specification.  (same as @samp{b}).
 
 @item l
-This modifier is accepted but not used.
+@c This modifier was accepted but not used.
 @c whaffor ar l modifier??? presumably compat; with
 @c what???---doc@@cygnus.com, 25jan91
+Specify dependencies of this library.  The dependencies must immediately
+follow this option character, must use the same syntax as the linker
+command line, and must be specified within a single argument.  I.e., if
+multiple items are needed, they must be quoted to form a single command
+line argument.  For example @samp{L "-L/usr/local/lib -lmydep1 -lmydep2"}
 
 @item N
 Uses the @var{count} parameter.  This is used if there are multiple
@@ -592,6 +599,10 @@ Note - although the presence of this option does imply a @option{x}
 extraction operation that option must still be included on the command
 line.
 
+@item --record-libdeps @var{libdeps}
+The @option{--record-libdeps} option is identical to the @option{L} modifier,
+just handled in long form.
+
 @end table
 @c man end
 
diff --git a/binutils/emul_aix.c b/binutils/emul_aix.c
index c54e1083e5..726cb3b70c 100644
--- a/binutils/emul_aix.c
+++ b/binutils/emul_aix.c
@@ -67,30 +67,20 @@ check_aix (bfd *try_bfd)
 }
 
 static bfd_boolean
-ar_emul_aix_append (bfd **after_bfd, char *file_name, const char *target,
+ar_emul_aix_append (bfd **after_bfd, bfd *new_bfd,
 		    bfd_boolean verbose, bfd_boolean flatten)
 {
-  bfd *new_bfd;
-
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
-
   return do_ar_emul_append (after_bfd, new_bfd, verbose, flatten, check_aix);
 }
 
 static bfd_boolean
-ar_emul_aix_replace (bfd **after_bfd, char *file_name, const char *target,
+ar_emul_aix_replace (bfd **after_bfd, bfd *new_bfd,
 		     bfd_boolean verbose)
 {
-  bfd *new_bfd;
-
-  new_bfd = bfd_openr (file_name, target);
-  AR_EMUL_ELEMENT_CHECK (new_bfd, file_name);
-
   if (!check_aix (new_bfd))
     return FALSE;
 
-  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, file_name);
+  AR_EMUL_REPLACE_PRINT_VERBOSE (verbose, bfd_get_filename (new_bfd));
 
   new_bfd->archive_next = *after_bfd;
   *after_bfd = new_bfd;
diff --git a/binutils/testsuite/binutils-all/ar.exp b/binutils/testsuite/binutils-all/ar.exp
index 5a9d27c6d0..273a0a3909 100644
--- a/binutils/testsuite/binutils-all/ar.exp
+++ b/binutils/testsuite/binutils-all/ar.exp
@@ -715,6 +715,46 @@ proc many_files { } {
     pass $testname
 }
 
+proc test_add_dependencies { } {
+    global AR
+    global AS
+    global srcdir
+    global subdir
+    global obj
+
+    set testname "ar adding library dependencies"
+
+    if ![binutils_assemble $srcdir/$subdir/bintest.s tmpdir/bintest.${obj}] {
+	unresolved $testname
+	return
+    }
+
+    if [is_remote host] {
+	set archive artest.a
+	set objfile [remote_download host tmpdir/bintest.${obj}]
+	remote_file host delete $archive
+    } else {
+	set archive tmpdir/artest.a
+	set objfile tmpdir/bintest.${obj}
+    }
+
+    remote_file build delete tmpdir/artest.a
+
+    set got [binutils_run $AR "-r -c $archive --record-libdeps /foo/bar ${objfile}"]
+    if ![string match "" $got] {
+	fail $testname
+	return
+    }
+
+    set got [binutils_run $AR "-t $archive"]
+    if ![string match "*bintest.${obj}\r__.LIBDEP*" $got] {
+	fail $testname
+	return
+    }
+
+    pass $testname
+}
+
 # Run the tests.
 
 # Only run the bfdtest checks if the programs exist.  Since these
@@ -743,6 +783,7 @@ move_an_element
 empty_archive
 extract_an_element
 many_files
+test_add_dependencies
 
 if { [is_elf_format] && [supports_gnu_unique] } {
     unique_symbol
-- 
2.20.1

Re: [PATCH] dependency list for static libraries

[Nick Clifton]

Hi Howard,

> Whitespace fixes done, attached.

Sorry for the delay in reviewing this patch.  This revised form
looks good to me, so I have gone ahead and applied it to the sources.

Cheers
  Nick

Re: [PATCH] dependency list for static libraries

[Howard Chu]

Nick Clifton wrote:
> Hi Howard,
> 
>> Whitespace fixes done, attached.
> 
> Sorry for the delay in reviewing this patch.  This revised form
> looks good to me, so I have gone ahead and applied it to the sources.

Thanks. Digging into the linker plugin code now.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] dependency list for static libraries

[Alan Modra]

On Tue, Nov 03, 2020 at 03:31:49PM +0000, Howard Chu wrote:
> Nick Clifton wrote:
> > Hi Howard,
> > 
> >> Whitespace fixes done, attached.
> > 
> > Sorry for the delay in reviewing this patch.  This revised form
> > looks good to me, so I have gone ahead and applied it to the sources.

I now see these failures.

alpha-dec-vms  +FAIL: ar adding library dependencies
hppa-hp-hpux10  +FAIL: ar adding library dependencies
i386-bsd  +FAIL: ar adding library dependencies
i386-msdos  +FAIL: ar adding library dependencies
ia64-vms  +FAIL: ar adding library dependencies
ns32k-netbsd  +FAIL: ar adding library dependencies
ns32k-pc532-mach  +FAIL: ar adding library dependencies
pdp11-dec-aout  +FAIL: ar adding library dependencies
powerpc-aix5.1  +FAIL: ar adding library dependencies
powerpc-aix5.2  +FAIL: ar adding library dependencies
rs6000-aix4.3.3  +FAIL: ar adding library dependencies
rs6000-aix5.1  +FAIL: ar adding library dependencies
rs6000-aix5.2  +FAIL: ar adding library dependencies

I think these are all targets that default to --disable-plugins.  The
proper fix is to condition all of the added ar.c code on
BFD_SUPPORTS_PLUGINS, including help text and option processing.  And
then of course not run the new tests if --record-libdeps is not a
known ar option.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] dependency list for static libraries

[Howard Chu]

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

Alan Modra wrote:
> On Tue, Nov 03, 2020 at 03:31:49PM +0000, Howard Chu wrote:
>> Nick Clifton wrote:
>>> Hi Howard,
>>>
>>>> Whitespace fixes done, attached.
>>>
>>> Sorry for the delay in reviewing this patch.  This revised form
>>> looks good to me, so I have gone ahead and applied it to the sources.
> 
> I now see these failures.
> 
> alpha-dec-vms  +FAIL: ar adding library dependencies
> hppa-hp-hpux10  +FAIL: ar adding library dependencies
> i386-bsd  +FAIL: ar adding library dependencies
> i386-msdos  +FAIL: ar adding library dependencies
> ia64-vms  +FAIL: ar adding library dependencies
> ns32k-netbsd  +FAIL: ar adding library dependencies
> ns32k-pc532-mach  +FAIL: ar adding library dependencies
> pdp11-dec-aout  +FAIL: ar adding library dependencies
> powerpc-aix5.1  +FAIL: ar adding library dependencies
> powerpc-aix5.2  +FAIL: ar adding library dependencies
> rs6000-aix4.3.3  +FAIL: ar adding library dependencies
> rs6000-aix5.1  +FAIL: ar adding library dependencies
> rs6000-aix5.2  +FAIL: ar adding library dependencies
> 
> I think these are all targets that default to --disable-plugins.  The
> proper fix is to condition all of the added ar.c code on
> BFD_SUPPORTS_PLUGINS, including help text and option processing.  And
> then of course not run the new tests if --record-libdeps is not a
> known ar option.
> 
Thanks for catching that. This feature doesn't actually require plugin
support, that was a mistake in the previous patch. Fixed here.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

[-- Attachment #2: 0001-ar-fix-libdeps-to-avoid-unnecessary-dependency-on-pl.patch --]
[-- Type: text/x-patch, Size: 1820 bytes --]

From 4d2982a8c62b09a16ba1e9c4913c262f11557017 Mon Sep 17 00:00:00 2001
From: Howard Chu <hyc@symas.com>
Date: Sun, 8 Nov 2020 15:04:00 +0000
Subject: [PATCH] ar: fix libdeps to avoid unnecessary dependency on plugin
 support

Also move __.LIBDEP entry to head of archive instead of tail.
---
 binutils/ar.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/binutils/ar.c b/binutils/ar.c
index 56c758cc1a..d886c3b720 100644
--- a/binutils/ar.c
+++ b/binutils/ar.c
@@ -866,7 +866,7 @@ main (int argc, char **argv)
 	  /* Create a bfd to contain the dependencies.
 	     It inherits its type from arch, but we must set the type to
 	     "binary" otherwise bfd_bwrite() will fail.  After writing, we
-	     must set the type back to "plugin" otherwise adding it to the
+	     must set the type back to default otherwise adding it to the
 	     archive will fail.  */
 	  libdeps_bfd = bfd_create (LIBDEPS, arch);
 	  if (libdeps_bfd == NULL)
@@ -887,18 +887,18 @@ main (int argc, char **argv)
 	  if (! bfd_make_readable (libdeps_bfd))
 	    fatal (_("Cannot make libdeps object readable."));
 
-	  if (bfd_find_target ("plugin", libdeps_bfd) == NULL)
+	  if (bfd_find_target (plugin_target, libdeps_bfd) == NULL)
 	    fatal (_("Cannot reset libdeps record type."));
 
-	  /* Append our libdeps record to the list of files
+	  /* Prepend our libdeps record to the list of files
 	     being operated on.  */
 	  new_files = xmalloc ((file_count + 2) * sizeof (char *));
+	  new_files[0] = LIBDEPS;
 	  for (i = 0; i < file_count; i++)
-	    new_files[i] = files[i];
-	  new_files[i++] = LIBDEPS;
-	  file_count = i;
+	    new_files[i+1] = files[i];
+	  file_count = i+1;
 	  files = new_files;
-	  files[i] = NULL;
+	  files[file_count] = NULL;
 	}
 
       switch (operation)
-- 
2.20.1

Re: [PATCH] dependency list for static libraries

[Alan Modra]

On Sun, Nov 08, 2020 at 03:07:28PM +0000, Howard Chu wrote:
> --- a/binutils/ar.c
> +++ b/binutils/ar.c
> @@ -866,7 +866,7 @@ main (int argc, char **argv)
>  	  /* Create a bfd to contain the dependencies.
>  	     It inherits its type from arch, but we must set the type to
>  	     "binary" otherwise bfd_bwrite() will fail.  After writing, we
> -	     must set the type back to "plugin" otherwise adding it to the
> +	     must set the type back to default otherwise adding it to the
>  	     archive will fail.  */
>  	  libdeps_bfd = bfd_create (LIBDEPS, arch);
>  	  if (libdeps_bfd == NULL)
> @@ -887,18 +887,18 @@ main (int argc, char **argv)
>  	  if (! bfd_make_readable (libdeps_bfd))
>  	    fatal (_("Cannot make libdeps object readable."));
>  
> -	  if (bfd_find_target ("plugin", libdeps_bfd) == NULL)
> +	  if (bfd_find_target (plugin_target, libdeps_bfd) == NULL)
>  	    fatal (_("Cannot reset libdeps record type."));

Thanks, I'm going to commit the above changes after running tests.
I'm not going to commit or OK the unrelated change to prepend the
libdeps record.  I think that is a bad idea, because BFD treats the
first object in an archive specially.  In effect, the first object
selects the archive target.  See archive.c:910 and following.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] dependency list for static libraries

[Howard Chu]

Alan Modra wrote:
> Thanks, I'm going to commit the above changes after running tests.
> I'm not going to commit or OK the unrelated change to prepend the
> libdeps record.  I think that is a bad idea, because BFD treats the
> first object in an archive specially.  In effect, the first object
> selects the archive target.  See archive.c:910 and following.

Ok, how about a patch to make it 2nd. Just trying to avoid having to
search all the way to the end, for archives with a particularly large
number of members.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] dependency list for static libraries

[Alan Modra]

On Tue, Nov 10, 2020 at 02:44:13AM +0000, Howard Chu wrote:
> Alan Modra wrote:
> > Thanks, I'm going to commit the above changes after running tests.
> > I'm not going to commit or OK the unrelated change to prepend the
> > libdeps record.  I think that is a bad idea, because BFD treats the
> > first object in an archive specially.  In effect, the first object
> > selects the archive target.  See archive.c:910 and following.
> 
> Ok, how about a patch to make it 2nd. Just trying to avoid having to
> search all the way to the end, for archives with a particularly large
> number of members.

That might be OK.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] dependency list for static libraries

[Howard Chu]

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

Alan Modra wrote:
> On Tue, Nov 10, 2020 at 02:44:13AM +0000, Howard Chu wrote:
>> Ok, how about a patch to make it 2nd. Just trying to avoid having to
>> search all the way to the end, for archives with a particularly large
>> number of members.
> 
> That might be OK.
> 


-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

[-- Attachment #2: 0001-Insert-libdeps-record-as-2nd-member-of-archive-inste.patch --]
[-- Type: text/x-patch, Size: 1327 bytes --]

From 868bf42666b74ccfa0e87bb4560801a63f71db83 Mon Sep 17 00:00:00 2001
From: Howard Chu <hyc@symas.com>
Date: Wed, 11 Nov 2020 14:54:25 +0000
Subject: [PATCH] Insert libdeps record as 2nd member of archive instead of
 last

---
 binutils/ar.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/binutils/ar.c b/binutils/ar.c
index 537b139e98..8bde751168 100644
--- a/binutils/ar.c
+++ b/binutils/ar.c
@@ -890,13 +890,16 @@ main (int argc, char **argv)
 	  if (bfd_find_target (plugin_target, libdeps_bfd) == NULL)
 	    fatal (_("Cannot reset libdeps record type."));
 
-	  /* Append our libdeps record to the list of files
-	     being operated on.  */
+	  /* Insert our libdeps record in 2nd slot of the list of files
+	     being operated on.  We shouldn't use 1st slot, but we want
+		 to avoid having to search all the way to the end of an
+		 archive with a large number of members at link time.  */
 	  new_files = xmalloc ((file_count + 2) * sizeof (char *));
-	  for (i = 0; i < file_count; i++)
-	    new_files[i] = files[i];
-	  new_files[i++] = LIBDEPS;
-	  file_count = i;
+	  new_files[0] = files[0];
+	  new_files[1] = LIBDEPS;
+	  for (i = 1; i < file_count; i++)
+	    new_files[i+1] = files[i];
+	  file_count = ++i;
 	  files = new_files;
 	  files[i] = NULL;
 	}
-- 
2.20.1

Re: [PATCH] dependency list for static libraries

[Howard Chu]

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

Howard Chu wrote:
> Alan Modra wrote:
>> On Tue, Nov 10, 2020 at 02:44:13AM +0000, Howard Chu wrote:
>>> Ok, how about a patch to make it 2nd. Just trying to avoid having to
>>> search all the way to the end, for archives with a particularly large
>>> number of members.
>>
>> That might be OK.

Hm, previous one had some errant tabs in there, fixed here.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

[-- Attachment #2: 0001-Insert-libdeps-record-as-2nd-member-of-archive-inste.patch --]
[-- Type: text/x-patch, Size: 1333 bytes --]

From 12f23b487218a5750d2ab384828f344eb5cee3c9 Mon Sep 17 00:00:00 2001
From: Howard Chu <hyc@symas.com>
Date: Wed, 11 Nov 2020 14:54:25 +0000
Subject: [PATCH] Insert libdeps record as 2nd member of archive instead of
 last

---
 binutils/ar.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/binutils/ar.c b/binutils/ar.c
index 537b139e98..fbfc6e5c08 100644
--- a/binutils/ar.c
+++ b/binutils/ar.c
@@ -890,13 +890,16 @@ main (int argc, char **argv)
 	  if (bfd_find_target (plugin_target, libdeps_bfd) == NULL)
 	    fatal (_("Cannot reset libdeps record type."));
 
-	  /* Append our libdeps record to the list of files
-	     being operated on.  */
+	  /* Insert our libdeps record in 2nd slot of the list of files
+	     being operated on.  We shouldn't use 1st slot, but we want
+	     to avoid having to search all the way to the end of an
+	     archive with a large number of members at link time.  */
 	  new_files = xmalloc ((file_count + 2) * sizeof (char *));
-	  for (i = 0; i < file_count; i++)
-	    new_files[i] = files[i];
-	  new_files[i++] = LIBDEPS;
-	  file_count = i;
+	  new_files[0] = files[0];
+	  new_files[1] = LIBDEPS;
+	  for (i = 1; i < file_count; i++)
+	    new_files[i+1] = files[i];
+	  file_count = ++i;
 	  files = new_files;
 	  files[i] = NULL;
 	}
-- 
2.20.1

Re: [PATCH] dependency list for static libraries

[Nick Clifton]

Hi Howard,

>>>> Ok, how about a patch to make it 2nd. Just trying to avoid having to
>>>> search all the way to the end, for archives with a particularly large
>>>> number of members.

Approved and applied.

Cheers
   Nick

[PATCH] dependency list for static libraries

[Howard Chu]

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

Nick Clifton wrote:
> Hi Howard,
> 
>> Whitespace fixes done, attached.
> 
> Sorry for the delay in reviewing this patch.  This revised form
> looks good to me, so I have gone ahead and applied it to the sources.

A tentative patch for the corresponding linker plugin is attached. One big question
I still have is how to get this plugin to be used by default?

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

[-- Attachment #2: 0001-Add-libdep-linker-plugin.patch --]
[-- Type: text/x-patch, Size: 8371 bytes --]

From 0bebac309c1e5772a22a4715b2367f565236f183 Mon Sep 17 00:00:00 2001
From: Howard Chu <hyc@symas.com>
Date: Wed, 4 Nov 2020 00:24:56 +0000
Subject: [PATCH] Add libdep linker plugin

Needs ld to be linked with -E to export its symbols; the plugin
uses the libbfd that ld was linked with.
---
 ld/Makefile.am     |   5 +
 ld/libdep_plugin.c | 284 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 289 insertions(+)
 create mode 100644 ld/libdep_plugin.c

diff --git a/ld/Makefile.am b/ld/Makefile.am
index 41db0c6016..1f5b73e0ce 100644
--- a/ld/Makefile.am
+++ b/ld/Makefile.am
@@ -947,6 +947,7 @@ ld_new_SOURCES = ldgram.y ldlex-wrapper.c lexsup.c ldlang.c mri.c ldctor.c ldmai
 ld_new_DEPENDENCIES = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) \
 		      $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL_DEP)
 ld_new_LDADD = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL) $(ZLIB)
+ld_new_LDFLAGS = -Wl,-E
 
 # Dependency tracking for the generated emulation files.
 EXTRA_ld_new_SOURCES += $(ALL_EMULATION_SOURCES) $(ALL_64_EMULATION_SOURCES)
@@ -998,6 +999,10 @@ libldtestplug4_la_SOURCES = testplug4.c
 libldtestplug4_la_CFLAGS= -g -O2
 libldtestplug4_la_LDFLAGS = -no-undefined -rpath /nowhere
 
+lib_LTLIBRARIES = libdep_plugin.la
+libdep_plugin_la_SOURCES = libdep_plugin.c
+libdep_plugin_la_LDFLAGS = -no-undefined -rpath /nowhere
+
 # DOCUMENTATION TARGETS
 # Manual configuration file; not usually attached to normal configuration,
 # because almost all configs use "gen" version of manual.
diff --git a/ld/libdep_plugin.c b/ld/libdep_plugin.c
new file mode 100644
index 0000000000..2cf00b2b25
--- /dev/null
+++ b/ld/libdep_plugin.c
@@ -0,0 +1,284 @@
+/* libdeps plugin for the GNU linker.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+
+   This file is part of the GNU Binutils.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+   MA 02110-1301, USA.  */
+
+#include "sysdep.h"
+#include "bfd.h"
+#if BFD_SUPPORTS_PLUGINS
+#include "plugin-api.h"
+
+#include <ctype.h> /* For isspace.  */
+
+/* Helper for calling plugin api message function.  */
+#define TV_MESSAGE if (tv_message) (*tv_message)
+
+/* Function pointers to cache hooks passed at onload time.  */
+static ld_plugin_register_claim_file tv_register_claim_file = 0;
+static ld_plugin_register_cleanup tv_register_cleanup = 0;
+static ld_plugin_message tv_message = 0;
+static ld_plugin_add_input_library tv_add_input_library = 0;
+static ld_plugin_set_extra_library_path tv_set_extra_library_path = 0;
+
+/* Handle/record information received in a transfer vector entry.  */
+static enum ld_plugin_status
+parse_tv_tag (struct ld_plugin_tv *tv)
+{
+#define SETVAR(x) x = tv->tv_u.x
+  switch (tv->tv_tag)
+    {
+      case LDPT_REGISTER_CLAIM_FILE_HOOK:
+	SETVAR(tv_register_claim_file);
+	break;
+      case LDPT_REGISTER_CLEANUP_HOOK:
+	SETVAR(tv_register_cleanup);
+	break;
+      case LDPT_MESSAGE:
+	SETVAR(tv_message);
+	break;
+      case LDPT_ADD_INPUT_LIBRARY:
+	SETVAR(tv_add_input_library);
+	break;
+      case LDPT_SET_EXTRA_LIBRARY_PATH:
+	SETVAR(tv_set_extra_library_path);
+	break;
+      default:
+	break;
+    }
+#undef SETVAR
+  return LDPS_OK;
+}
+
+/* Turn a string into an argvec.  */
+static char **
+str2vec (char *in)
+{
+  char **res;
+  char *s, *first, *end;
+  char *sq, *dq;
+  int i, total;
+
+  end = in + strlen(in);
+  s = in;
+  while (isspace (*s)) s++;
+  first = s;
+
+  i = 1;
+  while (s = strchr (s, ' ')) {
+    s++;
+    i++;
+  }
+  res = (char **)malloc ((i+1) * sizeof (char *));
+  if (!res)
+    return res;
+
+  i = 0;
+  sq = NULL;
+  dq = NULL;
+  res[0] = first;
+  for (s = first; *s; s++) {
+    if (*s == '\\') {
+      memmove (s, s+1, end-s-1);
+      end--;
+    }
+    if (isspace (*s)) {
+      if (sq || dq)
+        continue;
+      *s++ = '\0';
+      while(isspace (*s)) s++;
+      if (*s)
+        res[++i] = s;
+    }
+    if (*s == '\'') {
+      if (!dq) {
+        if (sq) {
+	  memmove (sq, sq+1, s-sq-1);
+	  memmove (s-2, s+1, end-s-1);
+	  end -= 2;
+	  s--;
+	  sq = NULL;
+	} else {
+	  sq = s;
+	}
+      }
+    }
+    if (*s == '"') {
+      if (!sq) {
+        if (dq) {
+	  memmove (dq, dq+1, s-dq-1);
+	  memmove (s-2, s+1, end-s-1);
+	  end -= 2;
+	  s--;
+	  dq = NULL;
+	} else {
+	  dq = s;
+	}
+      }
+    }
+  }
+  res[++i] = NULL;
+  return res;
+}
+
+static char *prevfile;
+#define LIBDEPS "__.LIBDEP"
+
+/* Standard plugin API registerable hook.  */
+static enum ld_plugin_status
+onclaim_file (const struct ld_plugin_input_file *file, int *claimed)
+{
+  bfd *arch, *next, *prev;
+  bfd_size_type len;
+  char *line = NULL, **vec;
+  enum ld_plugin_status rv;
+
+  /* If we've already seen this file, ignore it.  */
+  if (prevfile && !strcmp (file->name, prevfile))
+    return LDPS_OK;
+
+  /* If it's not an archive member, ignore it.  */
+  if (!file->offset)
+    return LDPS_OK;
+
+  if (prevfile)
+    free (prevfile);
+
+  prevfile = strdup (file->name);
+  if (!prevfile)
+    return LDPS_ERR;
+
+  /* This hook only gets called on actual object files.
+   * We have to examine the archive ourselves, to find
+   * our LIBDEPS member.  */
+  arch = bfd_openr (file->name,  "plugin");
+  if (!arch)
+    return LDPS_ERR;
+
+  bfd_check_format (arch, bfd_archive);
+
+  prev = NULL;
+  while ((next = bfd_openr_next_archived_file (arch, prev)))
+    {
+      prev = next;
+      if (!strcmp (bfd_get_filename (next), LIBDEPS))
+        break;
+    }
+  if (!next)
+    {
+      bfd_close (arch);
+      return LDPS_OK;
+    }
+  {
+    struct stat st;
+    bfd_stat_arch_elt (next, &st);
+    len = st.st_size;
+  }
+  line = malloc (len);
+  if (!line)
+    {
+      rv = LDPS_ERR;
+      goto quit;
+    }
+
+  if (bfd_bread (line, len, next) != len)
+    {
+      rv = LDPS_ERR;
+      goto quit;
+    }
+
+  rv = LDPS_OK;
+  vec = str2vec (line);
+  if (vec) {
+    int i;
+    for (i=0; vec[i]; i++) {
+      if (vec[i][0] != '-')
+        continue;
+      rv = LDPS_ERR;
+      if (vec[i][1] == 'l') {
+        rv = tv_add_input_library (vec[i]+2);
+      } else if (vec[i][1] == 'L') {
+        rv = tv_set_extra_library_path (vec[i]+2);
+      }
+      if (rv != LDPS_OK)
+        break;
+    }
+    free (vec);
+  }
+  /* Inform the user/testsuite.  */
+  TV_MESSAGE (LDPL_INFO, "processed deps for library %s: %s",
+	      file->name, line);
+  fflush (NULL);
+
+quit:
+  if (line)
+    free (line);
+  bfd_close (arch);
+
+  return rv;
+}
+
+/* Standard plugin API registerable hook.  */
+static enum ld_plugin_status
+oncleanup (void)
+{
+  if (prevfile) {
+    free (prevfile);
+    prevfile = NULL;
+  }
+  return LDPS_OK;
+}
+
+/* Standard plugin API entry point.  */
+enum ld_plugin_status
+onload (struct ld_plugin_tv *tv)
+{
+  enum ld_plugin_status rv;
+
+  /* This plugin requires a valid tv array.  */
+  if (!tv)
+    return LDPS_ERR;
+
+  /* First entry should always be LDPT_MESSAGE, letting us get
+     hold of it easily so we can send output straight away.  */
+  if (tv[0].tv_tag == LDPT_MESSAGE)
+    tv_message = tv[0].tv_u.tv_message;
+
+  do
+    if ((rv = parse_tv_tag (tv)) != LDPS_OK)
+      return rv;
+  while ((tv++)->tv_tag != LDPT_NULL);
+
+  /* Register hooks.  */
+  if (!tv_register_claim_file)
+    {
+      TV_MESSAGE (LDPL_FATAL, "No register_claim_file hook");
+      fflush (NULL);
+      return LDPS_ERR;
+    }
+  (*tv_register_claim_file) (onclaim_file);
+  if (!tv_register_cleanup)
+    {
+      TV_MESSAGE (LDPL_FATAL, "No register_cleanup hook");
+      fflush (NULL);
+      return LDPS_ERR;
+    }
+  (*tv_register_cleanup) (oncleanup);
+  fflush (NULL);
+  return LDPS_OK;
+}
+#endif /* BFD_SUPPORTS_PLUGINS */
-- 
2.20.1

Re: [PATCH] dependency list for static libraries

[Nick Clifton]

Hi Howard,

> One big question
> I still have is how to get this plugin to be used by default?

I do not think that there is a way to guarantee this.  After all
plugins, by design, are intended to be optional.

Obviously if a build system is involved - one that is adding the
--record-libdeps option when creating libraries - then it will be
able to add the necessary linker command line option to invoke
the plugin as well.

Cheers
  Nick

Re: [PATCH] dependency list for static libraries

[Howard Chu]

Nick Clifton wrote:
> Hi Howard,
> 
>> One big question
>> I still have is how to get this plugin to be used by default?
> 
> I do not think that there is a way to guarantee this.  After all
> plugins, by design, are intended to be optional.
> 
> Obviously if a build system is involved - one that is adding the
> --record-libdeps option when creating libraries - then it will be
> able to add the necessary linker command line option to invoke
> the plugin as well.

I suppose that means I'll still be using shell scripts to wrap these then.
Was hoping to be able to do away with them.


On a slightly related note - are linker plugins supposed to be compatible
between ld and gold? I get a SEGV using gold:

Reading symbols from ./ld-new...
(gdb) r
Starting program: /media/hyc/software/binutils.o/gold/ld-new -plugin ../ld/.libs/libdep_plugin.so -o ssljunk ../ld/ssljunk.o -L/usr/local/lib -lssl
-L/usr/lib/x86_64-linux-gnu -lpthread -lc -ldl
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
gold::Workqueue::add_to_queue (this=0x0, queue=0x8, t=0x555555bb3200, front=false)
    at ../../binutils-gdb/gold/workqueue.cc:154
154	  Hold_lock hl(this->lock_);
(gdb) bt
#0  gold::Workqueue::add_to_queue (this=0x0, queue=0x8, t=0x555555bb3200, front=false)
    at ../../binutils-gdb/gold/workqueue.cc:154
#1  0x00005555559b70ab in gold::Plugin_manager::add_input_file (this=0x555555b8c350, pathname=<optimized out>,
    is_lib=<optimized out>) at /usr/include/c++/9/bits/char_traits.h:300
#2  0x00007ffff74b2d93 in onclaim_file (claimed=<optimized out>, file=<optimized out>)
    at ../../binutils-gdb/ld/libdep_plugin.c:217
#3  onclaim_file (file=<optimized out>, claimed=<optimized out>) at ../../binutils-gdb/ld/libdep_plugin.c:145
#4  0x00005555559b6b69 in gold::Plugin::claim_file (this=<optimized out>, plugin_input_file=0x555555b8c3a8)
    at ../../binutils-gdb/gold/plugin.cc:390
#5  gold::Plugin_manager::claim_file (this=this@entry=0x555555b8c350, input_file=0x555555c2b9e0, offset=441644,
    filesize=7880, elf_object=<optimized out>, elf_object@entry=0x555555c399e0)
    at ../../binutils-gdb/gold/plugin.cc:766
#6  0x00005555558c216f in gold::Archive::get_elf_object_for_member (this=0x555555b83c80, off=441584,
    punconfigured=<optimized out>) at ../../binutils-gdb/gold/options.h:1646
#7  0x00005555558c2c3e in gold::Archive::include_member (this=0x555555b83c80, symtab=0x7fffffff4760,
    layout=0x7fffffff4a20, input_objects=0x7fffffff4520, off=441584, mapfile=<optimized out>, sym=0x555555c176f0,
    why=0x7fffffff4150 "") at ../../binutils-gdb/gold/archive.cc:1030
#8  0x00005555558c3485 in gold::Archive::add_symbols (this=0x555555b83c80, symtab=0x7fffffff4760,
    layout=0x7fffffff4a20, input_objects=0x7fffffff4520, mapfile=0x0) at /usr/include/c++/9/bits/basic_string.h:2300
#9  0x00005555558c3641 in gold::Add_archive_symbols::run (this=0x555555c357e0, workqueue=0x7fffffff4460)
    at ../../binutils-gdb/gold/archive.cc:1142
#10 0x0000555555a0ea58 in gold::Workqueue::find_and_run_task (this=0x7fffffff4460, thread_number=0)
    at ../../binutils-gdb/gold/token.h:290
#11 0x0000555555a0ecaa in gold::Workqueue::process (this=this@entry=0x7fffffff4460,
    thread_number=thread_number@entry=0) at ../../binutils-gdb/gold/workqueue.cc:495
#12 0x00005555556c6e85 in main (argc=<optimized out>, argv=<optimized out>) at ../../binutils-gdb/gold/main.cc:252
(gdb) frame 2
#2  0x00007ffff74b2d93 in onclaim_file (claimed=<optimized out>, file=<optimized out>)
    at ../../binutils-gdb/ld/libdep_plugin.c:217
217	        rv = tv_add_input_library (vec[i]+2);
(gdb)

While using ld yields:

violino:/media/hyc/software/binutils.o/ld> ./ld-new -plugin ../ld/.libs/libdep_plugin.so -o ssljunk ../ld/ssljunk.o -L/usr/local/lib -lssl
-L/usr/lib/x86_64-linux-gnu -lpthread -lc -ldl
processed deps for library /usr/local/lib/libssl.a: -lcrypto
./ld-new: warning: cannot find entry symbol _start; defaulting to 0000000000404000
./ld-new: /usr/local/lib/libcrypto.a(init.o): in function `ossl_init_register_atexit_ossl_':
init.c:(.text+0x1c0): undefined reference to `atexit'
./ld-new: /usr/local/lib/libcrypto.a(threads_pthread.o): in function `fork_once_func':
threads_pthread.c:(.text+0x1a): undefined reference to `pthread_atfork'

(Testing with __.LIBDEP in libssl.a containing just "-lcrypto")

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] dependency list for static libraries

[Nick Clifton]

Hi Howard,

> On a slightly related note - are linker plugins supposed to be compatible
> between ld and gold? 

Yes...

> I get a SEGV using gold:

Sorry I am not a gold expert.  Or C++ in general for that matter.

I would say that the bug must be at least partially in gold itself,
since it should never segfault.

One thing I did note however:

> 0  gold::Workqueue::add_to_queue (this=0x0, queue=0x8, t=0x555555bb3200, front=false)
>    at ../../binutils-gdb/gold/workqueue.cc:154

It would appear that the "this" pointer is NULL.
Presumably this means that the function/method is not being called from inside a class ?

Cheers
   Nick

Re: [PATCH] dependency list for static libraries

[Howard Chu]

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

This linker plugin works with both ld and gold.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

[-- Attachment #2: 0001-Add-libdep-linker-plugin.patch --]
[-- Type: text/x-patch, Size: 9461 bytes --]

From c36984014f5a5b1779456a4a57c9610e89c9ea24 Mon Sep 17 00:00:00 2001
From: Howard Chu <hyc@symas.com>
Date: Fri, 13 Nov 2020 14:07:56 +0000
Subject: [PATCH] Add libdep linker plugin

---
 ld/Makefile.am     |   4 +
 ld/libdep_plugin.c | 366 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 370 insertions(+)
 create mode 100644 ld/libdep_plugin.c

diff --git a/ld/Makefile.am b/ld/Makefile.am
index c9f85e5292..4b68b21495 100644
--- a/ld/Makefile.am
+++ b/ld/Makefile.am
@@ -998,6 +998,10 @@ libldtestplug4_la_SOURCES = testplug4.c
 libldtestplug4_la_CFLAGS= -g -O2
 libldtestplug4_la_LDFLAGS = -no-undefined -rpath /nowhere
 
+lib_LTLIBRARIES = libdep_plugin.la
+libdep_plugin_la_SOURCES = libdep_plugin.c
+libdep_plugin_la_LDFLAGS = -no-undefined -rpath /nowhere
+
 # DOCUMENTATION TARGETS
 # Manual configuration file; not usually attached to normal configuration,
 # because almost all configs use "gen" version of manual.
diff --git a/ld/libdep_plugin.c b/ld/libdep_plugin.c
new file mode 100644
index 0000000000..2a7fdc4d0b
--- /dev/null
+++ b/ld/libdep_plugin.c
@@ -0,0 +1,366 @@
+/* libdeps plugin for the GNU linker.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+
+   This file is part of the GNU Binutils.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+   MA 02110-1301, USA.  */
+
+#include "sysdep.h"
+#include "bfd.h"
+#if BFD_SUPPORTS_PLUGINS
+#include "plugin-api.h"
+
+#include <ctype.h> /* For isspace.  */
+
+extern enum ld_plugin_status onload (struct ld_plugin_tv *tv);
+
+/* Helper for calling plugin api message function.  */
+#define TV_MESSAGE if (tv_message) (*tv_message)
+
+/* Function pointers to cache hooks passed at onload time.  */
+static ld_plugin_register_claim_file tv_register_claim_file = 0;
+static ld_plugin_register_all_symbols_read tv_register_all_symbols_read = 0;
+static ld_plugin_register_cleanup tv_register_cleanup = 0;
+static ld_plugin_message tv_message = 0;
+static ld_plugin_add_input_library tv_add_input_library = 0;
+static ld_plugin_set_extra_library_path tv_set_extra_library_path = 0;
+
+/* Handle/record information received in a transfer vector entry.  */
+static enum ld_plugin_status
+parse_tv_tag (struct ld_plugin_tv *tv)
+{
+#define SETVAR(x) x = tv->tv_u.x
+  switch (tv->tv_tag)
+    {
+      case LDPT_REGISTER_CLAIM_FILE_HOOK:
+	SETVAR(tv_register_claim_file);
+	break;
+      case LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK:
+	SETVAR(tv_register_all_symbols_read);
+	break;
+      case LDPT_REGISTER_CLEANUP_HOOK:
+	SETVAR(tv_register_cleanup);
+	break;
+      case LDPT_MESSAGE:
+	SETVAR(tv_message);
+	break;
+      case LDPT_ADD_INPUT_LIBRARY:
+	SETVAR(tv_add_input_library);
+	break;
+      case LDPT_SET_EXTRA_LIBRARY_PATH:
+	SETVAR(tv_set_extra_library_path);
+	break;
+      default:
+	break;
+    }
+#undef SETVAR
+  return LDPS_OK;
+}
+
+/* Defs for archive parsing.  */
+#define ARMAGSIZE	8
+typedef struct arhdr
+{
+  char ar_name[16];
+  char ar_date[12];
+  char ar_uid[6];
+  char ar_gid[6];
+  char ar_mode[8];
+  char ar_size[10];
+  char ar_fmag[2];
+} arhdr;
+
+typedef struct linerec
+{
+  struct linerec *next;
+  char line[];
+} linerec;
+
+#define LIBDEPS "__.LIBDEP/ "
+
+static linerec *line_head, **line_tail = &line_head;
+
+static enum ld_plugin_status
+get_libdeps (int fd)
+{
+  arhdr ah;
+  int len;
+  unsigned long mlen;
+  linerec *lr;
+  enum ld_plugin_status rc = LDPS_NO_SYMS;
+
+  lseek (fd, ARMAGSIZE, SEEK_SET);
+  for (;;)
+    {
+      len = read (fd, (void *) &ah, sizeof (ah));
+      if (len != sizeof (ah))
+	break;
+      mlen = strtoul (ah.ar_size, NULL, 10);
+      if (!mlen || strncmp (ah.ar_name, LIBDEPS, sizeof (LIBDEPS)-1))
+	{
+	  lseek (fd, mlen, SEEK_CUR);
+	  continue;
+	}
+      lr = malloc (sizeof (linerec) + mlen);
+      if (!lr)
+	return LDPS_ERR;
+      lr->next = NULL;
+      len = read (fd, lr->line, mlen);
+      lr->line[mlen-1] = '\0';
+      *line_tail = lr;
+      line_tail = &lr->next;
+      rc = LDPS_OK;
+      break;
+    }
+  return rc;
+}
+
+/* Turn a string into an argvec.  */
+static char **
+str2vec (char *in)
+{
+  char **res;
+  char *s, *first, *end;
+  char *sq, *dq;
+  int i;
+
+  end = in + strlen (in);
+  s = in;
+  while (isspace (*s)) s++;
+  first = s;
+
+  i = 1;
+  while ((s = strchr (s, ' ')))
+    {
+      s++;
+      i++;
+    }
+  res = (char **)malloc ((i+1) * sizeof (char *));
+  if (!res)
+    return res;
+
+  i = 0;
+  sq = NULL;
+  dq = NULL;
+  res[0] = first;
+  for (s = first; *s; s++)
+    {
+      if (*s == '\\')
+	{
+	  memmove (s, s+1, end-s-1);
+	  end--;
+	}
+      if (isspace (*s))
+	{
+	  if (sq || dq)
+	    continue;
+	  *s++ = '\0';
+	  while (isspace (*s)) s++;
+	  if (*s)
+	    res[++i] = s;
+	}
+      if (*s == '\'' && !dq)
+	{
+	  if (sq)
+	    {
+	      memmove (sq, sq+1, s-sq-1);
+	      memmove (s-2, s+1, end-s-1);
+	      end -= 2;
+	      s--;
+	      sq = NULL;
+	    }
+	  else
+	    {
+	      sq = s;
+	    }
+	}
+      if (*s == '"' && !sq)
+	{
+	  if (dq)
+	    {
+	      memmove (dq, dq+1, s-dq-1);
+	      memmove (s-2, s+1, end-s-1);
+	      end -= 2;
+	      s--;
+	      dq = NULL;
+	    }
+	  else
+	    {
+	      dq = s;
+	    }
+	}
+    }
+  res[++i] = NULL;
+  return res;
+}
+
+static char *prevfile;
+
+/* Standard plugin API registerable hook.  */
+static enum ld_plugin_status
+onclaim_file (const struct ld_plugin_input_file *file, int *claimed)
+{
+  enum ld_plugin_status rv;
+
+  *claimed = 0;
+
+  /* If we've already seen this file, ignore it.  */
+  if (prevfile && !strcmp (file->name, prevfile))
+    return LDPS_OK;
+
+  /* If it's not an archive member, ignore it.  */
+  if (!file->offset)
+    return LDPS_OK;
+
+  if (prevfile)
+    free (prevfile);
+
+  prevfile = strdup (file->name);
+  if (!prevfile)
+    return LDPS_ERR;
+
+  /* This hook only gets called on actual object files.
+   * We have to examine the archive ourselves, to find
+   * our LIBDEPS member.  */
+  rv = get_libdeps (file->fd);
+  if (rv == LDPS_ERR)
+    return rv;
+
+  if (rv == LDPS_OK)
+    {
+      linerec *lr = (linerec *)line_tail;
+      /* Inform the user/testsuite.  */
+      TV_MESSAGE (LDPL_INFO, "got deps for library %s: %s",
+		  file->name, lr->line);
+      fflush (NULL);
+    }
+
+  return LDPS_OK;
+}
+
+/* Standard plugin API registerable hook.  */
+static enum ld_plugin_status
+onall_symbols_read (void)
+{
+  linerec *lr;
+  char **vec;
+  enum ld_plugin_status rv = LDPS_OK;
+
+  while ((lr = line_head))
+    {
+      line_head = lr->next;
+      vec = str2vec (lr->line);
+      if (vec)
+	{
+	  int i;
+	  for (i = 0; vec[i]; i++)
+	    {
+	      if (vec[i][0] != '-')
+		{
+		  TV_MESSAGE (LDPL_WARNING, "ignoring libdep argument %s",
+			      vec[i]);
+		  fflush (NULL);
+		  continue;
+		}
+	      if (vec[i][1] == 'l')
+		rv = tv_add_input_library (vec[i]+2);
+	      else if (vec[i][1] == 'L')
+		rv = tv_set_extra_library_path (vec[i]+2);
+	      else
+		{
+		  TV_MESSAGE (LDPL_WARNING, "ignoring libdep argument %s",
+			      vec[i]);
+		  fflush (NULL);
+		}
+	      if (rv != LDPS_OK)
+		break;
+	    }
+	  free (vec);
+	}
+      free (lr);
+    }
+  line_tail = NULL;
+  return rv;
+}
+
+/* Standard plugin API registerable hook.  */
+static enum ld_plugin_status
+oncleanup (void)
+{
+  if (prevfile)
+    {
+      free (prevfile);
+      prevfile = NULL;
+    }
+  if (line_head)
+    {
+      linerec *lr;
+      while ((lr = line_head))
+	{
+	  line_head = lr->next;
+	  free (lr);
+	}
+      line_tail = NULL;
+    }
+  return LDPS_OK;
+}
+
+/* Standard plugin API entry point.  */
+enum ld_plugin_status
+onload (struct ld_plugin_tv *tv)
+{
+  enum ld_plugin_status rv;
+
+  /* This plugin requires a valid tv array.  */
+  if (!tv)
+    return LDPS_ERR;
+
+  /* First entry should always be LDPT_MESSAGE, letting us get
+     hold of it easily so we can send output straight away.  */
+  if (tv[0].tv_tag == LDPT_MESSAGE)
+    tv_message = tv[0].tv_u.tv_message;
+
+  do
+    if ((rv = parse_tv_tag (tv)) != LDPS_OK)
+      return rv;
+  while ((tv++)->tv_tag != LDPT_NULL);
+
+  /* Register hooks.  */
+  if (!tv_register_claim_file)
+    {
+      TV_MESSAGE (LDPL_FATAL, "No register_claim_file hook");
+      fflush (NULL);
+      return LDPS_ERR;
+    }
+  (*tv_register_claim_file) (onclaim_file);
+  if (!tv_register_all_symbols_read)
+    {
+      TV_MESSAGE (LDPL_FATAL, "No register_all_symbols_read hook");
+      fflush (NULL);
+      return LDPS_ERR;
+    }
+  (*tv_register_all_symbols_read) (onall_symbols_read);
+  if (!tv_register_cleanup)
+    {
+      TV_MESSAGE (LDPL_FATAL, "No register_cleanup hook");
+      fflush (NULL);
+      return LDPS_ERR;
+    }
+  (*tv_register_cleanup) (oncleanup);
+  fflush (NULL);
+  return LDPS_OK;
+}
+#endif /* BFD_SUPPORTS_PLUGINS */
-- 
2.20.1

Re: [PATCH] dependency list for static libraries

[Howard Chu]

Howard Chu wrote:
> This linker plugin works with both ld and gold.
> 
Ping: anything else needed for this, docs, etc.?

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] dependency list for static libraries

[Nick Clifton]

Hi Howard,

>> This linker plugin works with both ld and gold.

> Ping: anything else needed for this, docs, etc.?

Sorry, sorry - my mind has been a mile away.

So - the code in the patch looks good.  I am running regression tests
now but I am not expecting to encounter any problems.  There are some
things missing however, the first of which is documentation as you
guessed:

   * A entry in ld/NEWS describing the new plugin.

   * Documentation in ld/ld.texi describing how to use the plugin,
     preferably with examples.

   * One or more new tests in linker's testsuite to check the
     functionality of the plugin.  Obviously these would have to be
     native only tests, but there are places in the testsuite
     where this is checked.  (Eg ld/testsuite/ld-bootstrap/bootstrap.exp)


One other thing - is the plugin installed into the correct location
if "make install" is run ?  (Ie:  ${libdir}/bfd-plugins).  I have not
actually checked this yet, so maybe it works, but I would have guessed
that some extra work would need to be done on ld/Makefile.am to manage
the installation.

Cheers
   Nick

Re: [PATCH] dependency list for static libraries

[Howard Chu]

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

Nick Clifton wrote:
> Hi Howard,
> 
>>> This linker plugin works with both ld and gold.
> 
>> Ping: anything else needed for this, docs, etc.?
> 
> Sorry, sorry - my mind has been a mile away.
> 
> So - the code in the patch looks good.  I am running regression tests
> now but I am not expecting to encounter any problems.  There are some
> things missing however, the first of which is documentation as you
> guessed:

I'm still working on writing up some tests, but the rest is here to
get a head start on reviewing.
> 
>   * A entry in ld/NEWS describing the new plugin.

Done.

>   * Documentation in ld/ld.texi describing how to use the plugin,
>     preferably with examples.

I took a stab at this, open to suggestions.

>   * One or more new tests in linker's testsuite to check the
>     functionality of the plugin.  Obviously these would have to be
>     native only tests, but there are places in the testsuite
>     where this is checked.  (Eg ld/testsuite/ld-bootstrap/bootstrap.exp)

Working on this.

> One other thing - is the plugin installed into the correct location
> if "make install" is run ?  (Ie:  ${libdir}/bfd-plugins).  I have not
> actually checked this yet, so maybe it works, but I would have guessed
> that some extra work would need to be done on ld/Makefile.am to manage
> the installation.

I've tweaked the Makefile.am to install into $(libdir)/bfd-plugins.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

[-- Attachment #2: 0001-Add-libdep-linker-plugin.patch --]
[-- Type: text/x-patch, Size: 13100 bytes --]

From 5e5dccb81d36d6198093d0991b669c0e9dc5a626 Mon Sep 17 00:00:00 2001
From: Howard Chu <hyc@symas.com>
Date: Fri, 13 Nov 2020 14:07:56 +0000
Subject: [PATCH] Add libdep linker plugin

---
 ld/Makefile.am     |   5 +
 ld/NEWS            |   3 +
 ld/ld.texi         |  53 +++++++
 ld/libdep_plugin.c | 366 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 427 insertions(+)
 create mode 100644 ld/libdep_plugin.c

diff --git a/ld/Makefile.am b/ld/Makefile.am
index c9f85e5292..2f59ee6169 100644
--- a/ld/Makefile.am
+++ b/ld/Makefile.am
@@ -998,6 +998,11 @@ libldtestplug4_la_SOURCES = testplug4.c
 libldtestplug4_la_CFLAGS= -g -O2
 libldtestplug4_la_LDFLAGS = -no-undefined -rpath /nowhere
 
+bfdplugindir = $(libdir)/bfd-plugins
+bfdplugin_LTLIBRARIES = libdep.la
+libdep_la_SOURCES = libdep_plugin.c
+libdep_la_LDFLAGS = -no-undefined -rpath /nowhere
+
 # DOCUMENTATION TARGETS
 # Manual configuration file; not usually attached to normal configuration,
 # because almost all configs use "gen" version of manual.
diff --git a/ld/NEWS b/ld/NEWS
index 607030bddd..1c3f9c156e 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -1,5 +1,8 @@
 -*- text -*-
 
+* Add libdep plugin, for linking dependencies of static libraries that
+  were recorded by ar in the __.LIBDEP archive member.
+
 * Add --error-handling-script=<NAME> command line option to allow a helper
   script to be invoked when an undefined symbol or a missing library is
   encountered.  This option can be suppressed via the configure time
diff --git a/ld/ld.texi b/ld/ld.texi
index 8e3c7da145..51c51a3ec1 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -126,6 +126,7 @@ in the section entitled ``GNU Free Documentation License''.
 * Overview::                    Overview
 * Invocation::                  Invocation
 * Scripts::                     Linker Scripts
+* Plugins::                     Linker Plugins
 @ifset GENERIC
 * Machine Dependent::           Machine Dependent Features
 @end ifset
@@ -6954,6 +6955,58 @@ Any input files read because of an implicit linker script will be read
 at the position in the command line where the implicit linker script was
 read.  This can affect archive searching.
 
+@node Plugins
+@chapter Linker Plugins
+
+@cindex plugins
+@cindex linker plugins
+The linker can use dynamically loaded plugins to modify its behavior.
+For example, the link-time optimization feature that some compilers
+support is implemented with a linker plugin.
+
+Currently there is only one plugin shipped by default, but more may
+be added here later.
+
+@menu
+* libdep Plugin::	Static Library Dependencies Plugin
+@end menu
+
+@node libdep Plugin
+@section Static Library Dependencies Plugin
+@cindex static library dependencies
+Originally, static libraries were contained in an archive file consisting
+just of a collection of relocatable object files.  Later they evolved to
+optionally include a symbol table, to assist in finding the needed objects
+within a library.  There their evolution ended, and dynamic libraries
+rose to ascendance.
+
+One useful feature of dynamic libraries was that, more than just collecting
+multiple objects into a single file, they also included a list of their
+dependencies, such that one could specify just the name of a single dynamic
+library at link time, and all of its dependencies would be implicitly
+referenced as well.  But static libraries lacked this feature, so if a
+link invocation was switched from using dynamic libraries to static
+libraries, the link command would usually fail unless it was rewritten to
+explicitly list the dependencies of the static library.
+
+The GNU @command{ar} utility now supports a @option{--record-libdeps} option
+to embed dependency lists into static libraries as well, and the @file{libdep}
+plugin may be used to read this dependency information at link time.  The
+dependency information is stored as a single string, carrying @option{-l}
+and @option{-L} arguments as they would normally appear in a linker
+command line.  As such, the information can be written with any text
+utility and stored into any archive, even if GNU @command{ar} is not
+being used to create the archive.  The information is stored in an
+archive member named @samp{__.LIBDEP}.
+
+For example, given a library @file{libssl.a} that depends on another
+library @file{libcrypto.a} which may be found in @file{/usr/local/lib},
+the @samp{__.LIBDEP} member of @file{libssl.a} would contain
+
+@smallexample
+-L/usr/local/lib -lcrypto
+@end smallexample
+
 @ifset GENERIC
 @node Machine Dependent
 @chapter Machine Dependent Features
diff --git a/ld/libdep_plugin.c b/ld/libdep_plugin.c
new file mode 100644
index 0000000000..2a7fdc4d0b
--- /dev/null
+++ b/ld/libdep_plugin.c
@@ -0,0 +1,366 @@
+/* libdeps plugin for the GNU linker.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+
+   This file is part of the GNU Binutils.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+   MA 02110-1301, USA.  */
+
+#include "sysdep.h"
+#include "bfd.h"
+#if BFD_SUPPORTS_PLUGINS
+#include "plugin-api.h"
+
+#include <ctype.h> /* For isspace.  */
+
+extern enum ld_plugin_status onload (struct ld_plugin_tv *tv);
+
+/* Helper for calling plugin api message function.  */
+#define TV_MESSAGE if (tv_message) (*tv_message)
+
+/* Function pointers to cache hooks passed at onload time.  */
+static ld_plugin_register_claim_file tv_register_claim_file = 0;
+static ld_plugin_register_all_symbols_read tv_register_all_symbols_read = 0;
+static ld_plugin_register_cleanup tv_register_cleanup = 0;
+static ld_plugin_message tv_message = 0;
+static ld_plugin_add_input_library tv_add_input_library = 0;
+static ld_plugin_set_extra_library_path tv_set_extra_library_path = 0;
+
+/* Handle/record information received in a transfer vector entry.  */
+static enum ld_plugin_status
+parse_tv_tag (struct ld_plugin_tv *tv)
+{
+#define SETVAR(x) x = tv->tv_u.x
+  switch (tv->tv_tag)
+    {
+      case LDPT_REGISTER_CLAIM_FILE_HOOK:
+	SETVAR(tv_register_claim_file);
+	break;
+      case LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK:
+	SETVAR(tv_register_all_symbols_read);
+	break;
+      case LDPT_REGISTER_CLEANUP_HOOK:
+	SETVAR(tv_register_cleanup);
+	break;
+      case LDPT_MESSAGE:
+	SETVAR(tv_message);
+	break;
+      case LDPT_ADD_INPUT_LIBRARY:
+	SETVAR(tv_add_input_library);
+	break;
+      case LDPT_SET_EXTRA_LIBRARY_PATH:
+	SETVAR(tv_set_extra_library_path);
+	break;
+      default:
+	break;
+    }
+#undef SETVAR
+  return LDPS_OK;
+}
+
+/* Defs for archive parsing.  */
+#define ARMAGSIZE	8
+typedef struct arhdr
+{
+  char ar_name[16];
+  char ar_date[12];
+  char ar_uid[6];
+  char ar_gid[6];
+  char ar_mode[8];
+  char ar_size[10];
+  char ar_fmag[2];
+} arhdr;
+
+typedef struct linerec
+{
+  struct linerec *next;
+  char line[];
+} linerec;
+
+#define LIBDEPS "__.LIBDEP/ "
+
+static linerec *line_head, **line_tail = &line_head;
+
+static enum ld_plugin_status
+get_libdeps (int fd)
+{
+  arhdr ah;
+  int len;
+  unsigned long mlen;
+  linerec *lr;
+  enum ld_plugin_status rc = LDPS_NO_SYMS;
+
+  lseek (fd, ARMAGSIZE, SEEK_SET);
+  for (;;)
+    {
+      len = read (fd, (void *) &ah, sizeof (ah));
+      if (len != sizeof (ah))
+	break;
+      mlen = strtoul (ah.ar_size, NULL, 10);
+      if (!mlen || strncmp (ah.ar_name, LIBDEPS, sizeof (LIBDEPS)-1))
+	{
+	  lseek (fd, mlen, SEEK_CUR);
+	  continue;
+	}
+      lr = malloc (sizeof (linerec) + mlen);
+      if (!lr)
+	return LDPS_ERR;
+      lr->next = NULL;
+      len = read (fd, lr->line, mlen);
+      lr->line[mlen-1] = '\0';
+      *line_tail = lr;
+      line_tail = &lr->next;
+      rc = LDPS_OK;
+      break;
+    }
+  return rc;
+}
+
+/* Turn a string into an argvec.  */
+static char **
+str2vec (char *in)
+{
+  char **res;
+  char *s, *first, *end;
+  char *sq, *dq;
+  int i;
+
+  end = in + strlen (in);
+  s = in;
+  while (isspace (*s)) s++;
+  first = s;
+
+  i = 1;
+  while ((s = strchr (s, ' ')))
+    {
+      s++;
+      i++;
+    }
+  res = (char **)malloc ((i+1) * sizeof (char *));
+  if (!res)
+    return res;
+
+  i = 0;
+  sq = NULL;
+  dq = NULL;
+  res[0] = first;
+  for (s = first; *s; s++)
+    {
+      if (*s == '\\')
+	{
+	  memmove (s, s+1, end-s-1);
+	  end--;
+	}
+      if (isspace (*s))
+	{
+	  if (sq || dq)
+	    continue;
+	  *s++ = '\0';
+	  while (isspace (*s)) s++;
+	  if (*s)
+	    res[++i] = s;
+	}
+      if (*s == '\'' && !dq)
+	{
+	  if (sq)
+	    {
+	      memmove (sq, sq+1, s-sq-1);
+	      memmove (s-2, s+1, end-s-1);
+	      end -= 2;
+	      s--;
+	      sq = NULL;
+	    }
+	  else
+	    {
+	      sq = s;
+	    }
+	}
+      if (*s == '"' && !sq)
+	{
+	  if (dq)
+	    {
+	      memmove (dq, dq+1, s-dq-1);
+	      memmove (s-2, s+1, end-s-1);
+	      end -= 2;
+	      s--;
+	      dq = NULL;
+	    }
+	  else
+	    {
+	      dq = s;
+	    }
+	}
+    }
+  res[++i] = NULL;
+  return res;
+}
+
+static char *prevfile;
+
+/* Standard plugin API registerable hook.  */
+static enum ld_plugin_status
+onclaim_file (const struct ld_plugin_input_file *file, int *claimed)
+{
+  enum ld_plugin_status rv;
+
+  *claimed = 0;
+
+  /* If we've already seen this file, ignore it.  */
+  if (prevfile && !strcmp (file->name, prevfile))
+    return LDPS_OK;
+
+  /* If it's not an archive member, ignore it.  */
+  if (!file->offset)
+    return LDPS_OK;
+
+  if (prevfile)
+    free (prevfile);
+
+  prevfile = strdup (file->name);
+  if (!prevfile)
+    return LDPS_ERR;
+
+  /* This hook only gets called on actual object files.
+   * We have to examine the archive ourselves, to find
+   * our LIBDEPS member.  */
+  rv = get_libdeps (file->fd);
+  if (rv == LDPS_ERR)
+    return rv;
+
+  if (rv == LDPS_OK)
+    {
+      linerec *lr = (linerec *)line_tail;
+      /* Inform the user/testsuite.  */
+      TV_MESSAGE (LDPL_INFO, "got deps for library %s: %s",
+		  file->name, lr->line);
+      fflush (NULL);
+    }
+
+  return LDPS_OK;
+}
+
+/* Standard plugin API registerable hook.  */
+static enum ld_plugin_status
+onall_symbols_read (void)
+{
+  linerec *lr;
+  char **vec;
+  enum ld_plugin_status rv = LDPS_OK;
+
+  while ((lr = line_head))
+    {
+      line_head = lr->next;
+      vec = str2vec (lr->line);
+      if (vec)
+	{
+	  int i;
+	  for (i = 0; vec[i]; i++)
+	    {
+	      if (vec[i][0] != '-')
+		{
+		  TV_MESSAGE (LDPL_WARNING, "ignoring libdep argument %s",
+			      vec[i]);
+		  fflush (NULL);
+		  continue;
+		}
+	      if (vec[i][1] == 'l')
+		rv = tv_add_input_library (vec[i]+2);
+	      else if (vec[i][1] == 'L')
+		rv = tv_set_extra_library_path (vec[i]+2);
+	      else
+		{
+		  TV_MESSAGE (LDPL_WARNING, "ignoring libdep argument %s",
+			      vec[i]);
+		  fflush (NULL);
+		}
+	      if (rv != LDPS_OK)
+		break;
+	    }
+	  free (vec);
+	}
+      free (lr);
+    }
+  line_tail = NULL;
+  return rv;
+}
+
+/* Standard plugin API registerable hook.  */
+static enum ld_plugin_status
+oncleanup (void)
+{
+  if (prevfile)
+    {
+      free (prevfile);
+      prevfile = NULL;
+    }
+  if (line_head)
+    {
+      linerec *lr;
+      while ((lr = line_head))
+	{
+	  line_head = lr->next;
+	  free (lr);
+	}
+      line_tail = NULL;
+    }
+  return LDPS_OK;
+}
+
+/* Standard plugin API entry point.  */
+enum ld_plugin_status
+onload (struct ld_plugin_tv *tv)
+{
+  enum ld_plugin_status rv;
+
+  /* This plugin requires a valid tv array.  */
+  if (!tv)
+    return LDPS_ERR;
+
+  /* First entry should always be LDPT_MESSAGE, letting us get
+     hold of it easily so we can send output straight away.  */
+  if (tv[0].tv_tag == LDPT_MESSAGE)
+    tv_message = tv[0].tv_u.tv_message;
+
+  do
+    if ((rv = parse_tv_tag (tv)) != LDPS_OK)
+      return rv;
+  while ((tv++)->tv_tag != LDPT_NULL);
+
+  /* Register hooks.  */
+  if (!tv_register_claim_file)
+    {
+      TV_MESSAGE (LDPL_FATAL, "No register_claim_file hook");
+      fflush (NULL);
+      return LDPS_ERR;
+    }
+  (*tv_register_claim_file) (onclaim_file);
+  if (!tv_register_all_symbols_read)
+    {
+      TV_MESSAGE (LDPL_FATAL, "No register_all_symbols_read hook");
+      fflush (NULL);
+      return LDPS_ERR;
+    }
+  (*tv_register_all_symbols_read) (onall_symbols_read);
+  if (!tv_register_cleanup)
+    {
+      TV_MESSAGE (LDPL_FATAL, "No register_cleanup hook");
+      fflush (NULL);
+      return LDPS_ERR;
+    }
+  (*tv_register_cleanup) (oncleanup);
+  fflush (NULL);
+  return LDPS_OK;
+}
+#endif /* BFD_SUPPORTS_PLUGINS */
-- 
2.20.1

Re: [PATCH] dependency list for static libraries

[Nick Clifton]

Hi Hoawrd,

   Thanks for your patience with me for reviewing this patch.

   I have now gone ahead and applied the patch.  We can add
   some tests later but I wanted to make sure that it was in
   before the holiday break as I am planning on starting the
   2.36 release once I get back in January.

Cheers
   Nick

Re: [PATCH] dependency list for static libraries

[Jim Wilson]

On Mon, Dec 14, 2020 at 6:28 AM Nick Clifton via Binutils <
binutils@sourceware.org> wrote:

>    I have now gone ahead and applied the patch.  We can add
>    some tests later but I wanted to make sure that it was in
>    before the holiday break as I am planning on starting the
>    2.36 release once I get back in January.
>

This patch beaks nm.  You have to do an install to see the problem.  The
new libdep plugin is installed into the bfd-plugins dir, so bfd tries to
load it and gets an error.

gamma17:2093$ build-install/bin/riscv64-unknown-linux-gnu-nm tmp.o
bfd plugin: No register_all_symbols_read hook
bfd plugin: No register_all_symbols_read hook
bfd plugin: No register_all_symbols_read hook
0000000000000000 T main
gamma17:2094$

This in turn breaks glibc builds.

configure:6658: checking for __builtin_trap with no external dependencies
configure:6675: riscv64-unknown-linux-gnu-gcc  -c   -mcmodel=medlow -g -O2
  conftest.c >&5
configure:6675: $? = 0
configure:6680: error: confusing output from riscv64-unknown-linux-gnu-nm -u

Jim

Re: [PATCH] dependency list for static libraries

[Jeff Law]

On 12/15/20 9:17 AM, Jim Wilson wrote:
> On Mon, Dec 14, 2020 at 6:28 AM Nick Clifton via Binutils <
> binutils@sourceware.org> wrote:
>
>>    I have now gone ahead and applied the patch.  We can add
>>    some tests later but I wanted to make sure that it was in
>>    before the holiday break as I am planning on starting the
>>    2.36 release once I get back in January.
>>
> This patch beaks nm.  You have to do an install to see the problem.  The
> new libdep plugin is installed into the bfd-plugins dir, so bfd tries to
> load it and gets an error.
>
> gamma17:2093$ build-install/bin/riscv64-unknown-linux-gnu-nm tmp.o
> bfd plugin: No register_all_symbols_read hook
> bfd plugin: No register_all_symbols_read hook
> bfd plugin: No register_all_symbols_read hook
> 0000000000000000 T main
> gamma17:2094$
>
> This in turn breaks glibc builds.
>
> configure:6658: checking for __builtin_trap with no external dependencies
> configure:6675: riscv64-unknown-linux-gnu-gcc  -c   -mcmodel=medlow -g -O2
>   conftest.c >&5
> configure:6675: $? = 0
> configure:6680: error: confusing output from riscv64-unknown-linux-gnu-nm -u
Yea, it's breaking every glibc build in a similar manner.

jeff

Re: [PATCH] dependency list for static libraries

[Nick Clifton]

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

Hi Jim, Hi Jeff,

   Would one or both of you mind trying out the attached patch ?

   I think that it should work, although I am not sure if it ought
   to be extended so that the all_symbols_read and cleanup plugin
   functions are actually called.

Cheers
   Nick

[-- Attachment #2: plugin.patch --]
[-- Type: text/x-patch, Size: 2115 bytes --]

diff --git a/bfd/plugin.c b/bfd/plugin.c
index bed98520da..3a52554141 100644
--- a/bfd/plugin.c
+++ b/bfd/plugin.c
@@ -127,10 +127,10 @@ message (int level ATTRIBUTE_UNUSED,
 struct plugin_list_entry
 {
   /* These must be initialized for each IR object with LTO wrapper.  */
-  ld_plugin_claim_file_handler claim_file;
-  ld_plugin_all_symbols_read_handler all_symbols_read;
-  ld_plugin_all_symbols_read_handler cleanup_handler;
-  bfd_boolean has_symbol_type;
+  ld_plugin_claim_file_handler        claim_file;
+  ld_plugin_all_symbols_read_handler  all_symbols_read_handler;
+  ld_plugin_cleanup_handler           cleanup_handler;
+  bfd_boolean                         has_symbol_type;
 
   struct plugin_list_entry *next;
 
@@ -158,6 +158,23 @@ register_claim_file (ld_plugin_claim_file_handler handler)
   return LDPS_OK;
 }
 
+static enum ld_plugin_status
+register_all_symbols_read (ld_plugin_all_symbols_read_handler handler)
+{
+  BFD_ASSERT (current_plugin != NULL);
+  current_plugin->all_symbols_read_handler = handler;
+  return LDPS_OK;
+}
+
+/* Register a cleanup handler.  */
+static enum ld_plugin_status
+register_cleanup (ld_plugin_cleanup_handler handler)
+{
+  BFD_ASSERT (current_plugin != NULL);
+  current_plugin->cleanup_handler = handler;
+  return LDPS_OK;
+}
+
 static enum ld_plugin_status
 add_symbols (void * handle,
 	     int nsyms,
@@ -256,7 +273,7 @@ try_load_plugin (const char *                pname,
 		 bfd_boolean                 build_list_p)
 {
   void *plugin_handle;
-  struct ld_plugin_tv tv[5];
+  struct ld_plugin_tv tv[7];
   int i;
   ld_plugin_onload onload;
   enum ld_plugin_status status;
@@ -329,6 +346,14 @@ try_load_plugin (const char *                pname,
   tv[i].tv_tag = LDPT_ADD_SYMBOLS_V2;
   tv[i].tv_u.tv_add_symbols = add_symbols_v2;
 
+  ++i;
+  tv[i].tv_tag = LDPT_REGISTER_ALL_SYMBOLS_READ_HOOK;
+  tv[i].tv_u.tv_register_all_symbols_read = register_all_symbols_read;
+
+  ++i;
+  tv[i].tv_tag = LDPT_REGISTER_CLEANUP_HOOK;
+  tv[i].tv_u.tv_register_cleanup = register_cleanup;
+
   ++i;
   tv[i].tv_tag = LDPT_NULL;
   tv[i].tv_u.tv_val = 0;

Re: [PATCH] dependency list for static libraries

[Jeff Law]

On 12/15/20 9:50 AM, Nick Clifton wrote:
> Hi Jim, Hi Jeff,
>
>   Would one or both of you mind trying out the attached patch ?
>
>   I think that it should work, although I am not sure if it ought
>   to be extended so that the all_symbols_read and cleanup plugin
>   functions are actually called.
It certainly helps -- it gets us back to the section attribute problems
between gcc & glibc causing problems.  So progress ;-)

jeff

Re: [PATCH] dependency list for static libraries

[Jim Wilson]

On Tue, Dec 15, 2020 at 11:12 AM Jeff Law <law@redhat.com> wrote:

> On 12/15/20 9:50 AM, Nick Clifton wrote:
> > Hi Jim, Hi Jeff,
> >   Would one or both of you mind trying out the attached patch ?
> >
> >   I think that it should work, although I am not sure if it ought
> >   to be extended so that the all_symbols_read and cleanup plugin
> >   functions are actually called.
> It certainly helps -- it gets us back to the section attribute problems
> between gcc & glibc causing problems.  So progress ;-)
>

It works for me also.  I have a workaround for the gcc/glibc section
problem so i was able to complete the toolchain build, including the glibc
build.

Jim

Re: [PATCH] dependency list for static libraries

[Cary Coutant]

>    Would one or both of you mind trying out the attached patch ?
>
>    I think that it should work, although I am not sure if it ought
>    to be extended so that the all_symbols_read and cleanup plugin
>    functions are actually called.

Why not just make the new plugin disable itself if the host tool
doesn't provide the all_symbols_read entry point?

-cary

Re: [PATCH] dependency list for static libraries

[Howard Chu]

Cary Coutant via Binutils wrote:
>>    Would one or both of you mind trying out the attached patch ?
>>
>>    I think that it should work, although I am not sure if it ought
>>    to be extended so that the all_symbols_read and cleanup plugin
>>    functions are actually called.
> 
> Why not just make the new plugin disable itself if the host tool
> doesn't provide the all_symbols_read entry point?

Yeah, that'd be fine too. Just noop/silently exit if entry points are missing.
I can write that up if folks agree it's better.
> 
> -cary
> 


-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] dependency list for static libraries

[Nick Clifton]

Hi Howard,

>> Why not just make the new plugin disable itself if the host tool
>> doesn't provide the all_symbols_read entry point?
> 
> Yeah, that'd be fine too. Just noop/silently exit if entry points are missing.
> I can write that up if folks agree it's better.

Please do.

It turns out that my proposed fix breaks the LTO plugin, so it is a no-go. :-(

Cheers
   Nick

[PATCH] ld: Call plugin hooks only if they are available

[H.J. Lu]

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

On Wed, Dec 16, 2020 at 3:16 AM Nick Clifton via Binutils
<binutils@sourceware.org> wrote:
>
> Hi Howard,
>
> >> Why not just make the new plugin disable itself if the host tool
> >> doesn't provide the all_symbols_read entry point?
> >
> > Yeah, that'd be fine too. Just noop/silently exit if entry points are missing.
> > I can write that up if folks agree it's better.
>
> Please do.
>
> It turns out that my proposed fix breaks the LTO plugin, so it is a no-go. :-(
>

I am going to check this in if there are no objections today.

-- 
H.J.

[-- Attachment #2: 0001-ld-Call-plugin-hooks-only-if-they-are-available.patch --]
[-- Type: text/x-patch, Size: 2989 bytes --]

From 32f73ea9955bc4ddaa83fd5477e76196dc4cdb96 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 16 Dec 2020 06:22:46 -0800
Subject: [PATCH] ld: Call plugin hooks only if they are available

Call tv_register_all_symbols_read and tv_register_cleanup only if they
are set.

	PR ld/27081
	* libdep_plugin.c (onload): Call tv_register_all_symbols_read
	and tv_register_cleanup only if they are set.
	* testsuite/config/default.exp (dep_plug_opt): New.
	* testsuite/ld-elf/elf.exp: Pass $dep_plug_opt to nm.
	* testsuite/ld-elf/pr26391.fd: New file.
---
 ld/libdep_plugin.c              | 18 ++++--------------
 ld/testsuite/config/default.exp |  8 ++++++++
 ld/testsuite/ld-elf/elf.exp     |  5 ++++-
 ld/testsuite/ld-elf/pr26391.fd  |  4 ++++
 4 files changed, 20 insertions(+), 15 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/pr26391.fd

diff --git a/ld/libdep_plugin.c b/ld/libdep_plugin.c
index 2a7fdc4d0b..37ad7c7f97 100644
--- a/ld/libdep_plugin.c
+++ b/ld/libdep_plugin.c
@@ -346,20 +346,10 @@ onload (struct ld_plugin_tv *tv)
       return LDPS_ERR;
     }
   (*tv_register_claim_file) (onclaim_file);
-  if (!tv_register_all_symbols_read)
-    {
-      TV_MESSAGE (LDPL_FATAL, "No register_all_symbols_read hook");
-      fflush (NULL);
-      return LDPS_ERR;
-    }
-  (*tv_register_all_symbols_read) (onall_symbols_read);
-  if (!tv_register_cleanup)
-    {
-      TV_MESSAGE (LDPL_FATAL, "No register_cleanup hook");
-      fflush (NULL);
-      return LDPS_ERR;
-    }
-  (*tv_register_cleanup) (oncleanup);
+  if (tv_register_all_symbols_read)
+    (*tv_register_all_symbols_read) (onall_symbols_read);
+  if (tv_register_cleanup)
+    (*tv_register_cleanup) (oncleanup);
   fflush (NULL);
   return LDPS_OK;
 }
diff --git a/ld/testsuite/config/default.exp b/ld/testsuite/config/default.exp
index f711fb792e..a8ad1fdb6f 100644
--- a/ld/testsuite/config/default.exp
+++ b/ld/testsuite/config/default.exp
@@ -443,3 +443,11 @@ if { [check_compiler_available] } {
 	}
     }
 }
+
+if {[file exists .libs/libdep.so]} {
+  set dep_plug_opt "--plugin .libs/libdep.so"
+} elseif {[file exists .libs/libdep.dll]} {
+  set dep_plug_opt "--plugin .libs/libdep.dll"
+} else {
+    set dep_plug_opt ""
+}
diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
index e08a6f5feb..a58b17b512 100644
--- a/ld/testsuite/ld-elf/elf.exp
+++ b/ld/testsuite/ld-elf/elf.exp
@@ -415,7 +415,10 @@ run_ld_link_tests [list \
 	"" \
 	"" \
 	{pr26391a.c pr26391b.c pr26391c.c pr26391d.c} \
-	{{nm "" pr26391.nd}} \
+	[list \
+	    [list "nm" "$dep_plug_opt" "pr26391.nd"] \
+	    [list "nm" "$dep_plug_opt" "pr26391.fd"] \
+	] \
 	"pr26391-5.o" \
 	"-fno-function-sections" \
     ] \
diff --git a/ld/testsuite/ld-elf/pr26391.fd b/ld/testsuite/ld-elf/pr26391.fd
new file mode 100644
index 0000000000..1921cd6c70
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr26391.fd
@@ -0,0 +1,4 @@
+#failif
+#...
+bfd plugin: No register_all_symbols_read hook
+#...
-- 
2.29.2

Re: [PATCH] ld: Call plugin hooks only if they are available

[Howard Chu]

H.J. Lu wrote:
> On Wed, Dec 16, 2020 at 3:16 AM Nick Clifton via Binutils
> <binutils@sourceware.org> wrote:
>>
>> Hi Howard,
>>
>>>> Why not just make the new plugin disable itself if the host tool
>>>> doesn't provide the all_symbols_read entry point?
>>>
>>> Yeah, that'd be fine too. Just noop/silently exit if entry points are missing.
>>> I can write that up if folks agree it's better.
>>
>> Please do.
>>
>> It turns out that my proposed fix breaks the LTO plugin, so it is a no-go. :-(
>>
> 
> I am going to check this in if there are no objections today.
> 
diff --git a/ld/libdep_plugin.c b/ld/libdep_plugin.c
index 2a7fdc4d0b..37ad7c7f97 100644
--- a/ld/libdep_plugin.c
+++ b/ld/libdep_plugin.c
@@ -346,20 +346,10 @@ onload (struct ld_plugin_tv *tv)
       return LDPS_ERR;
     }
   (*tv_register_claim_file) (onclaim_file);
-  if (!tv_register_all_symbols_read)
-    {
-      TV_MESSAGE (LDPL_FATAL, "No register_all_symbols_read hook");
-      fflush (NULL);
-      return LDPS_ERR;
-    }
-  (*tv_register_all_symbols_read) (onall_symbols_read);
-  if (!tv_register_cleanup)
-    {
-      TV_MESSAGE (LDPL_FATAL, "No register_cleanup hook");
-      fflush (NULL);
-      return LDPS_ERR;
-    }
-  (*tv_register_cleanup) (oncleanup);
+  if (tv_register_all_symbols_read)
+    (*tv_register_all_symbols_read) (onall_symbols_read);
+  if (tv_register_cleanup)
+    (*tv_register_cleanup) (oncleanup);
   fflush (NULL);
   return LDPS_OK;
 }

The plugin shouldn't register any hooks at all, if any of these aren't available.
There's no point in doing the register_claim_file, that will just be wasted work.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] ld: Call plugin hooks only if they are available

[H.J. Lu]

On Wed, Dec 16, 2020 at 10:34 AM Howard Chu <hyc@symas.com> wrote:
>
> H.J. Lu wrote:
> > On Wed, Dec 16, 2020 at 3:16 AM Nick Clifton via Binutils
> > <binutils@sourceware.org> wrote:
> >>
> >> Hi Howard,
> >>
> >>>> Why not just make the new plugin disable itself if the host tool
> >>>> doesn't provide the all_symbols_read entry point?
> >>>
> >>> Yeah, that'd be fine too. Just noop/silently exit if entry points are missing.
> >>> I can write that up if folks agree it's better.
> >>
> >> Please do.
> >>
> >> It turns out that my proposed fix breaks the LTO plugin, so it is a no-go. :-(
> >>
> >
> > I am going to check this in if there are no objections today.
> >
> diff --git a/ld/libdep_plugin.c b/ld/libdep_plugin.c
> index 2a7fdc4d0b..37ad7c7f97 100644
> --- a/ld/libdep_plugin.c
> +++ b/ld/libdep_plugin.c
> @@ -346,20 +346,10 @@ onload (struct ld_plugin_tv *tv)
>        return LDPS_ERR;
>      }
>    (*tv_register_claim_file) (onclaim_file);
> -  if (!tv_register_all_symbols_read)
> -    {
> -      TV_MESSAGE (LDPL_FATAL, "No register_all_symbols_read hook");
> -      fflush (NULL);
> -      return LDPS_ERR;
> -    }
> -  (*tv_register_all_symbols_read) (onall_symbols_read);
> -  if (!tv_register_cleanup)
> -    {
> -      TV_MESSAGE (LDPL_FATAL, "No register_cleanup hook");
> -      fflush (NULL);
> -      return LDPS_ERR;
> -    }
> -  (*tv_register_cleanup) (oncleanup);
> +  if (tv_register_all_symbols_read)
> +    (*tv_register_all_symbols_read) (onall_symbols_read);
> +  if (tv_register_cleanup)
> +    (*tv_register_cleanup) (oncleanup);
>    fflush (NULL);
>    return LDPS_OK;
>  }
>
> The plugin shouldn't register any hooks at all, if any of these aren't available.
> There's no point in doing the register_claim_file, that will just be wasted work.

My branch has a testcase which fails today:

https://gitlab.com/x86-binutils/binutils-gdb/-/commits/users/hjl/pr27081/master

Please send me a patch on top of mine to fix the testcase.

-- 
H.J.

Re: [PATCH] ld: Call plugin hooks only if they are available

[Howard Chu]

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

H.J. Lu wrote:
> On Wed, Dec 16, 2020 at 10:34 AM Howard Chu <hyc@symas.com> wrote:
>>
>> H.J. Lu wrote:
>>> On Wed, Dec 16, 2020 at 3:16 AM Nick Clifton via Binutils
>>> <binutils@sourceware.org> wrote:
>>>>
>>>> Hi Howard,
>>>>
>>>>>> Why not just make the new plugin disable itself if the host tool
>>>>>> doesn't provide the all_symbols_read entry point?
>>>>>
>>>>> Yeah, that'd be fine too. Just noop/silently exit if entry points are missing.
>>>>> I can write that up if folks agree it's better.
>>>>
>>>> Please do.
>>>>

>> The plugin shouldn't register any hooks at all, if any of these aren't available.
>> There's no point in doing the register_claim_file, that will just be wasted work.
> 
> My branch has a testcase which fails today:
> 
> https://gitlab.com/x86-binutils/binutils-gdb/-/commits/users/hjl/pr27081/master
> 
> Please send me a patch on top of mine to fix the testcase.
> 
Our emails crossed in flight; this is the same patch I just sent, but
based off your branch.

I don't know which testcases failed for you. "make check" for me fails
large numbers of cases.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

[-- Attachment #2: 0001-No-op-if-not-all-required-plugin-hooks-are-available.patch --]
[-- Type: text/x-patch, Size: 1233 bytes --]

From 6e605c845b366fb983940df8525a44515119966d Mon Sep 17 00:00:00 2001
From: Howard Chu <hyc@symas.com>
Date: Wed, 16 Dec 2020 19:04:59 +0000
Subject: [PATCH] No-op if not all required plugin hooks are available.

---
 ld/libdep_plugin.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/ld/libdep_plugin.c b/ld/libdep_plugin.c
index 37ad7c7f97..42b7844c9c 100644
--- a/ld/libdep_plugin.c
+++ b/ld/libdep_plugin.c
@@ -339,17 +339,13 @@ onload (struct ld_plugin_tv *tv)
   while ((tv++)->tv_tag != LDPT_NULL);
 
   /* Register hooks.  */
-  if (!tv_register_claim_file)
+  if (tv_register_claim_file && tv_register_all_symbols_read &&
+      tv_register_cleanup)
     {
-      TV_MESSAGE (LDPL_FATAL, "No register_claim_file hook");
-      fflush (NULL);
-      return LDPS_ERR;
+      (*tv_register_claim_file) (onclaim_file);
+      (*tv_register_all_symbols_read) (onall_symbols_read);
+      (*tv_register_cleanup) (oncleanup);
     }
-  (*tv_register_claim_file) (onclaim_file);
-  if (tv_register_all_symbols_read)
-    (*tv_register_all_symbols_read) (onall_symbols_read);
-  if (tv_register_cleanup)
-    (*tv_register_cleanup) (oncleanup);
   fflush (NULL);
   return LDPS_OK;
 }
-- 
2.25.1

[PATCH] ld: Skip libdep plugin if not all plugin hooks are available

[H.J. Lu]

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

On Wed, Dec 16, 2020 at 11:06 AM Howard Chu <hyc@symas.com> wrote:
>
> H.J. Lu wrote:
> > On Wed, Dec 16, 2020 at 10:34 AM Howard Chu <hyc@symas.com> wrote:
> >>
> >> H.J. Lu wrote:
> >>> On Wed, Dec 16, 2020 at 3:16 AM Nick Clifton via Binutils
> >>> <binutils@sourceware.org> wrote:
> >>>>
> >>>> Hi Howard,
> >>>>
> >>>>>> Why not just make the new plugin disable itself if the host tool
> >>>>>> doesn't provide the all_symbols_read entry point?
> >>>>>
> >>>>> Yeah, that'd be fine too. Just noop/silently exit if entry points are missing.
> >>>>> I can write that up if folks agree it's better.
> >>>>
> >>>> Please do.
> >>>>
>
> >> The plugin shouldn't register any hooks at all, if any of these aren't available.
> >> There's no point in doing the register_claim_file, that will just be wasted work.
> >
> > My branch has a testcase which fails today:
> >
> > https://gitlab.com/x86-binutils/binutils-gdb/-/commits/users/hjl/pr27081/master
> >
> > Please send me a patch on top of mine to fix the testcase.
> >
> Our emails crossed in flight; this is the same patch I just sent, but
> based off your branch.

This is the patch I am checking in.   Thanks.

> I don't know which testcases failed for you. "make check" for me fails
> large numbers of cases.
>

There should be no "make check" failures on Linux/x86-64.


-- 
H.J.

[-- Attachment #2: 0001-ld-Skip-libdep-plugin-if-not-all-plugin-hooks-are-av.patch --]
[-- Type: text/x-patch, Size: 3318 bytes --]

From f88e5a994ffa8bb058aff46fb96f35bf72c3707d Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 16 Dec 2020 06:22:46 -0800
Subject: [PATCH] ld: Skip libdep plugin if not all plugin hooks are available

Skip plugin if not all required plugin hooks are available.

2020-12-16  Howard Chu <hyc@symas.com>
	    H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/27081
	* libdep_plugin.c (onload): Skip if not all required plugin hooks
	are available.
	* testsuite/config/default.exp (dep_plug_opt): New.
	* testsuite/ld-elf/elf.exp: Pass $dep_plug_opt to nm.
	* testsuite/ld-elf/pr26391.fd: New file.
---
 ld/libdep_plugin.c              | 25 ++++++-------------------
 ld/testsuite/config/default.exp |  8 ++++++++
 ld/testsuite/ld-elf/elf.exp     |  5 ++++-
 ld/testsuite/ld-elf/pr26391.fd  |  4 ++++
 4 files changed, 22 insertions(+), 20 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/pr26391.fd

diff --git a/ld/libdep_plugin.c b/ld/libdep_plugin.c
index 2a7fdc4d0b..b81364fdca 100644
--- a/ld/libdep_plugin.c
+++ b/ld/libdep_plugin.c
@@ -339,27 +339,14 @@ onload (struct ld_plugin_tv *tv)
   while ((tv++)->tv_tag != LDPT_NULL);
 
   /* Register hooks.  */
-  if (!tv_register_claim_file)
+  if (tv_register_claim_file
+      && tv_register_all_symbols_read
+      && tv_register_cleanup)
     {
-      TV_MESSAGE (LDPL_FATAL, "No register_claim_file hook");
-      fflush (NULL);
-      return LDPS_ERR;
-    }
-  (*tv_register_claim_file) (onclaim_file);
-  if (!tv_register_all_symbols_read)
-    {
-      TV_MESSAGE (LDPL_FATAL, "No register_all_symbols_read hook");
-      fflush (NULL);
-      return LDPS_ERR;
-    }
-  (*tv_register_all_symbols_read) (onall_symbols_read);
-  if (!tv_register_cleanup)
-    {
-      TV_MESSAGE (LDPL_FATAL, "No register_cleanup hook");
-      fflush (NULL);
-      return LDPS_ERR;
+      (*tv_register_claim_file) (onclaim_file);
+      (*tv_register_all_symbols_read) (onall_symbols_read);
+      (*tv_register_cleanup) (oncleanup);
     }
-  (*tv_register_cleanup) (oncleanup);
   fflush (NULL);
   return LDPS_OK;
 }
diff --git a/ld/testsuite/config/default.exp b/ld/testsuite/config/default.exp
index f711fb792e..a8ad1fdb6f 100644
--- a/ld/testsuite/config/default.exp
+++ b/ld/testsuite/config/default.exp
@@ -443,3 +443,11 @@ if { [check_compiler_available] } {
 	}
     }
 }
+
+if {[file exists .libs/libdep.so]} {
+  set dep_plug_opt "--plugin .libs/libdep.so"
+} elseif {[file exists .libs/libdep.dll]} {
+  set dep_plug_opt "--plugin .libs/libdep.dll"
+} else {
+    set dep_plug_opt ""
+}
diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
index e08a6f5feb..a58b17b512 100644
--- a/ld/testsuite/ld-elf/elf.exp
+++ b/ld/testsuite/ld-elf/elf.exp
@@ -415,7 +415,10 @@ run_ld_link_tests [list \
 	"" \
 	"" \
 	{pr26391a.c pr26391b.c pr26391c.c pr26391d.c} \
-	{{nm "" pr26391.nd}} \
+	[list \
+	    [list "nm" "$dep_plug_opt" "pr26391.nd"] \
+	    [list "nm" "$dep_plug_opt" "pr26391.fd"] \
+	] \
 	"pr26391-5.o" \
 	"-fno-function-sections" \
     ] \
diff --git a/ld/testsuite/ld-elf/pr26391.fd b/ld/testsuite/ld-elf/pr26391.fd
new file mode 100644
index 0000000000..1921cd6c70
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr26391.fd
@@ -0,0 +1,4 @@
+#failif
+#...
+bfd plugin: No register_all_symbols_read hook
+#...
-- 
2.29.2

Re: [PATCH] ld: Skip libdep plugin if not all plugin hooks are available

[Howard Chu]

H.J. Lu wrote:
> On Wed, Dec 16, 2020 at 11:06 AM Howard Chu <hyc@symas.com> wrote:
>> Our emails crossed in flight; this is the same patch I just sent, but
>> based off your branch.
> 
> This is the patch I am checking in.   Thanks.

OK, looks good.
> 
>> I don't know which testcases failed for you. "make check" for me fails
>> large numbers of cases.
>>
> 
> There should be no "make check" failures on Linux/x86-64.

I must've had some other garbage in my build tree. Deleting my obj tree and
rerunning configure, everything is fine now.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] ld: Skip libdep plugin if not all plugin hooks are available

[H.J. Lu]

On Wed, Dec 16, 2020 at 1:26 PM Howard Chu <hyc@symas.com> wrote:
>
> H.J. Lu wrote:
> > On Wed, Dec 16, 2020 at 11:06 AM Howard Chu <hyc@symas.com> wrote:
> >> Our emails crossed in flight; this is the same patch I just sent, but
> >> based off your branch.
> >
> > This is the patch I am checking in.   Thanks.
>
> OK, looks good.

I checked in.   Can you take a look at

https://sourceware.org/bugzilla/show_bug.cgi?id=27082

Thanks.

-- 
H.J.

Re: [PATCH] ld: Call plugin hooks only if they are available

[Howard Chu]

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

Howard Chu wrote:
> H.J. Lu wrote:
>> On Wed, Dec 16, 2020 at 3:16 AM Nick Clifton via Binutils
>> <binutils@sourceware.org> wrote:
>>>
>>> Hi Howard,
>>>
>>>>> Why not just make the new plugin disable itself if the host tool
>>>>> doesn't provide the all_symbols_read entry point?
>>>>
>>>> Yeah, that'd be fine too. Just noop/silently exit if entry points are missing.
>>>> I can write that up if folks agree it's better.
>>>
>>> Please do.
>>>
>>> It turns out that my proposed fix breaks the LTO plugin, so it is a no-go. :-(
>>>
>>
>> I am going to check this in if there are no objections today.

> The plugin shouldn't register any hooks at all, if any of these aren't available.
> There's no point in doing the register_claim_file, that will just be wasted work.
> 
Something like this instead.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

[-- Attachment #2: 0001-No-op-if-not-all-required-plugin-hooks-are-available.patch --]
[-- Type: text/x-patch, Size: 1491 bytes --]

From fcd3630e056647a167fd7dbb0ad854faea8aa2c2 Mon Sep 17 00:00:00 2001
From: Howard Chu <hyc@symas.com>
Date: Wed, 16 Dec 2020 18:42:41 +0000
Subject: [PATCH] No-op if not all required plugin hooks are available.

---
 ld/libdep_plugin.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/ld/libdep_plugin.c b/ld/libdep_plugin.c
index 2a7fdc4d0b..42b7844c9c 100644
--- a/ld/libdep_plugin.c
+++ b/ld/libdep_plugin.c
@@ -339,27 +339,13 @@ onload (struct ld_plugin_tv *tv)
   while ((tv++)->tv_tag != LDPT_NULL);
 
   /* Register hooks.  */
-  if (!tv_register_claim_file)
+  if (tv_register_claim_file && tv_register_all_symbols_read &&
+      tv_register_cleanup)
     {
-      TV_MESSAGE (LDPL_FATAL, "No register_claim_file hook");
-      fflush (NULL);
-      return LDPS_ERR;
-    }
-  (*tv_register_claim_file) (onclaim_file);
-  if (!tv_register_all_symbols_read)
-    {
-      TV_MESSAGE (LDPL_FATAL, "No register_all_symbols_read hook");
-      fflush (NULL);
-      return LDPS_ERR;
-    }
-  (*tv_register_all_symbols_read) (onall_symbols_read);
-  if (!tv_register_cleanup)
-    {
-      TV_MESSAGE (LDPL_FATAL, "No register_cleanup hook");
-      fflush (NULL);
-      return LDPS_ERR;
+      (*tv_register_claim_file) (onclaim_file);
+      (*tv_register_all_symbols_read) (onall_symbols_read);
+      (*tv_register_cleanup) (oncleanup);
     }
-  (*tv_register_cleanup) (oncleanup);
   fflush (NULL);
   return LDPS_OK;
 }
-- 
2.25.1

Re: [PATCH] dependency list for static libraries

[Cary Coutant]

> > One big question
> > I still have is how to get this plugin to be used by default?
>
> I do not think that there is a way to guarantee this.  After all
> plugins, by design, are intended to be optional.
>
> Obviously if a build system is involved - one that is adding the
> --record-libdeps option when creating libraries - then it will be
> able to add the necessary linker command line option to invoke
> the plugin as well.

Did I miss any discussion about whether this feature should/could be
implemented directly in the linkers, rather than via a plugin? I
wouldn't be opposed to it.

-cary

Re: [PATCH] dependency list for static libraries

[Howard Chu]

Cary Coutant wrote:
>>> One big question
>>> I still have is how to get this plugin to be used by default?
>>
>> I do not think that there is a way to guarantee this.  After all
>> plugins, by design, are intended to be optional.
>>
>> Obviously if a build system is involved - one that is adding the
>> --record-libdeps option when creating libraries - then it will be
>> able to add the necessary linker command line option to invoke
>> the plugin as well.
> 
> Did I miss any discussion about whether this feature should/could be
> implemented directly in the linkers, rather than via a plugin? I
> wouldn't be opposed to it.

I would prefer if it were built in and active by default. I just
went with the plugin because it seemed to be the quickest way to get
the feature working.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] dependency list for static libraries

[Nick Clifton]

Hi Hoawrd, Hi Cary,

>> Did I miss any discussion about whether this feature should/could be
>> implemented directly in the linkers, rather than via a plugin? I
>> wouldn't be opposed to it.
> 
> I would prefer if it were built in and active by default. I just
> went with the plugin because it seemed to be the quickest way to get
> the feature working.

Plus if I remember correctly there was some dissension as to how exactly
this feature should be implemented, so using a plugin made sense.

Cheers
   Nick

Re: [PATCH] dependency list for static libraries

[Matthias Klose]

On 11/3/20 4:14 PM, Nick Clifton via Binutils wrote:
> Hi Howard,
> 
>> Whitespace fixes done, attached.
> 
> Sorry for the delay in reviewing this patch.  This revised form
> looks good to me, so I have gone ahead and applied it to the sources.

hijacking the 'l' option leads to build failures.

In 2.35 it's documented as:

       l   This modifier is accepted but not used.

at least the build fails in
https://launchpad.net/ubuntu/+source/afterstep/2.2.12-14/+build/20720741

I don't know about the history of this modifier, but giving it a completely new
meaning is odd.

In about a week I can give you a list of packages from a distribution test
rebuild, failing to build because of this.

Matthias

Re: [PATCH] dependency list for static libraries

[Howard Chu]

Matthias Klose wrote:
> On 11/3/20 4:14 PM, Nick Clifton via Binutils wrote:
>> Hi Howard,
>>
>>> Whitespace fixes done, attached.
>>
>> Sorry for the delay in reviewing this patch.  This revised form
>> looks good to me, so I have gone ahead and applied it to the sources.
> 
> hijacking the 'l' option leads to build failures.
> 
> In 2.35 it's documented as:
> 
>        l   This modifier is accepted but not used.
> 
> at least the build fails in
> https://launchpad.net/ubuntu/+source/afterstep/2.2.12-14/+build/20720741
> 
> I don't know about the history of this modifier, but giving it a completely new
> meaning is odd.

No known version of ar on any POSIX/Unix system in the past 30 years has used this flag,
so why are those packages using it?
> 
> In about a week I can give you a list of packages from a distribution test
> rebuild, failing to build because of this.
> 
> Matthias
> 


-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/