Re: [PATCH] dependency list for static libraries

[Howard Chu <hyc@symas.com>]

[-- 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