Re: [PATCH] dependency list for static libraries

[Howard Chu <hyc@symas.com>]

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