* dependency list for static libraries @ 2017-09-19 15:49 Howard Chu 2017-09-19 15:52 ` Simon Richter 2017-09-19 16:54 ` Joseph Myers 0 siblings, 2 replies; 64+ messages in thread From: Howard Chu @ 2017-09-19 15:49 UTC (permalink / raw) To: binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: dependency list for static libraries 2017-09-19 15:49 dependency list for static libraries Howard Chu @ 2017-09-19 15:52 ` Simon Richter [not found] ` <WM!bae999665f49907786872b93f01ac98d53e7b97e29b4228399d8baadf9ec0ab33db74467d73c998225b250ba1d00a4c0!@mailstronghold-3.zmailcloud.com> 2017-09-19 16:54 ` Joseph Myers 1 sibling, 1 reply; 64+ messages in thread From: Simon Richter @ 2017-09-19 15:52 UTC (permalink / raw) To: binutils [-- 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 --] ^ permalink raw reply [flat|nested] 64+ messages in thread
[parent not found: <WM!bae999665f49907786872b93f01ac98d53e7b97e29b4228399d8baadf9ec0ab33db74467d73c998225b250ba1d00a4c0!@mailstronghold-3.zmailcloud.com>]
* Re: dependency list for static libraries [not found] ` <WM!bae999665f49907786872b93f01ac98d53e7b97e29b4228399d8baadf9ec0ab33db74467d73c998225b250ba1d00a4c0!@mailstronghold-3.zmailcloud.com> @ 2017-09-19 16:04 ` Howard Chu 2017-09-20 1:42 ` R0b0t1 0 siblings, 1 reply; 64+ messages in thread From: Howard Chu @ 2017-09-19 16:04 UTC (permalink / raw) To: Simon Richter, binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: dependency list for static libraries 2017-09-19 16:04 ` Howard Chu @ 2017-09-20 1:42 ` R0b0t1 0 siblings, 0 replies; 64+ messages in thread From: R0b0t1 @ 2017-09-20 1:42 UTC (permalink / raw) To: Howard Chu; +Cc: Simon Richter, binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: dependency list for static libraries 2017-09-19 15:49 dependency list for static libraries Howard Chu 2017-09-19 15:52 ` Simon Richter @ 2017-09-19 16:54 ` Joseph Myers [not found] ` <WM!83b6ad7285aa96ce69fcd1944d4eae8f20e5f19dfbf161f45313f5393bcffe1b77231520b8f4e24145a3f85eeafb39ed!@mailstronghold-1.zmailcloud.com> 1 sibling, 1 reply; 64+ messages in thread From: Joseph Myers @ 2017-09-19 16:54 UTC (permalink / raw) To: Howard Chu; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
[parent not found: <WM!83b6ad7285aa96ce69fcd1944d4eae8f20e5f19dfbf161f45313f5393bcffe1b77231520b8f4e24145a3f85eeafb39ed!@mailstronghold-1.zmailcloud.com>]
* Re: dependency list for static libraries [not found] ` <WM!83b6ad7285aa96ce69fcd1944d4eae8f20e5f19dfbf161f45313f5393bcffe1b77231520b8f4e24145a3f85eeafb39ed!@mailstronghold-1.zmailcloud.com> @ 2017-09-19 22:01 ` Howard Chu 2017-09-20 0:20 ` Joseph Myers 2020-09-03 20:42 ` Howard Chu 0 siblings, 2 replies; 64+ messages in thread From: Howard Chu @ 2017-09-19 22:01 UTC (permalink / raw) To: Joseph Myers; +Cc: binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: dependency list for static libraries 2017-09-19 22:01 ` Howard Chu @ 2017-09-20 0:20 ` Joseph Myers 2020-09-03 20:42 ` Howard Chu 1 sibling, 0 replies; 64+ messages in thread From: Joseph Myers @ 2017-09-20 0:20 UTC (permalink / raw) To: Howard Chu; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: dependency list for static libraries 2017-09-19 22:01 ` Howard Chu 2017-09-20 0:20 ` Joseph Myers @ 2020-09-03 20:42 ` Howard Chu 2020-09-22 10:39 ` Nick Clifton 1 sibling, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-09-03 20:42 UTC (permalink / raw) Cc: binutils [-- 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: dependency list for static libraries 2020-09-03 20:42 ` Howard Chu @ 2020-09-22 10:39 ` Nick Clifton 2020-09-22 11:42 ` Howard Chu 0 siblings, 1 reply; 64+ messages in thread From: Nick Clifton @ 2020-09-22 10:39 UTC (permalink / raw) To: Howard Chu; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: dependency list for static libraries 2020-09-22 10:39 ` Nick Clifton @ 2020-09-22 11:42 ` Howard Chu 2020-09-22 13:12 ` Nick Clifton 0 siblings, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-09-22 11:42 UTC (permalink / raw) To: Nick Clifton; +Cc: binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: dependency list for static libraries 2020-09-22 11:42 ` Howard Chu @ 2020-09-22 13:12 ` Nick Clifton 2020-09-22 16:23 ` [PATCH] " Howard Chu 0 siblings, 1 reply; 64+ messages in thread From: Nick Clifton @ 2020-09-22 13:12 UTC (permalink / raw) To: Howard Chu; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-09-22 13:12 ` Nick Clifton @ 2020-09-22 16:23 ` Howard Chu 2020-09-22 17:16 ` Fangrui Song 2020-09-22 20:46 ` Howard Chu 0 siblings, 2 replies; 64+ messages in thread From: Howard Chu @ 2020-09-22 16:23 UTC (permalink / raw) To: Nick Clifton; +Cc: binutils [-- 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-09-22 16:23 ` [PATCH] " Howard Chu @ 2020-09-22 17:16 ` Fangrui Song 2020-09-22 17:55 ` Howard Chu 2020-09-22 20:46 ` Howard Chu 1 sibling, 1 reply; 64+ messages in thread From: Fangrui Song @ 2020-09-22 17:16 UTC (permalink / raw) To: Howard Chu; +Cc: Nick Clifton, binutils 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 { } { ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-09-22 17:16 ` Fangrui Song @ 2020-09-22 17:55 ` Howard Chu 0 siblings, 0 replies; 64+ messages in thread From: Howard Chu @ 2020-09-22 17:55 UTC (permalink / raw) To: Fangrui Song; +Cc: Nick Clifton, binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-09-22 16:23 ` [PATCH] " Howard Chu 2020-09-22 17:16 ` Fangrui Song @ 2020-09-22 20:46 ` Howard Chu 2020-09-23 11:52 ` Nick Clifton 1 sibling, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-09-22 20:46 UTC (permalink / raw) To: Nick Clifton; +Cc: binutils [-- 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-09-22 20:46 ` Howard Chu @ 2020-09-23 11:52 ` Nick Clifton 2020-09-23 15:29 ` Howard Chu 2020-10-28 14:56 ` Howard Chu 0 siblings, 2 replies; 64+ messages in thread From: Nick Clifton @ 2020-09-23 11:52 UTC (permalink / raw) To: Howard Chu, Fangrui Song; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-09-23 11:52 ` Nick Clifton @ 2020-09-23 15:29 ` Howard Chu 2020-09-24 5:21 ` Fangrui Song 2020-10-28 14:56 ` Howard Chu 1 sibling, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-09-23 15:29 UTC (permalink / raw) To: Nick Clifton, Fangrui Song; +Cc: binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-09-23 15:29 ` Howard Chu @ 2020-09-24 5:21 ` Fangrui Song 2020-09-24 9:19 ` Howard Chu 0 siblings, 1 reply; 64+ messages in thread From: Fangrui Song @ 2020-09-24 5:21 UTC (permalink / raw) To: Howard Chu; +Cc: Nick Clifton, binutils 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-09-24 5:21 ` Fangrui Song @ 2020-09-24 9:19 ` Howard Chu 2020-09-24 9:30 ` Howard Chu 2020-09-28 11:07 ` Howard Chu 0 siblings, 2 replies; 64+ messages in thread From: Howard Chu @ 2020-09-24 9:19 UTC (permalink / raw) To: Fangrui Song; +Cc: Nick Clifton, binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-09-24 9:19 ` Howard Chu @ 2020-09-24 9:30 ` Howard Chu 2020-09-28 11:07 ` Howard Chu 1 sibling, 0 replies; 64+ messages in thread From: Howard Chu @ 2020-09-24 9:30 UTC (permalink / raw) To: Fangrui Song; +Cc: Nick Clifton, binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-09-24 9:19 ` Howard Chu 2020-09-24 9:30 ` Howard Chu @ 2020-09-28 11:07 ` Howard Chu 1 sibling, 0 replies; 64+ messages in thread From: Howard Chu @ 2020-09-28 11:07 UTC (permalink / raw) To: Fangrui Song; +Cc: binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-09-23 11:52 ` Nick Clifton 2020-09-23 15:29 ` Howard Chu @ 2020-10-28 14:56 ` Howard Chu 2020-11-03 15:14 ` Nick Clifton 1 sibling, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-10-28 14:56 UTC (permalink / raw) To: Nick Clifton, Fangrui Song; +Cc: binutils [-- 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-10-28 14:56 ` Howard Chu @ 2020-11-03 15:14 ` Nick Clifton 2020-11-03 15:31 ` Howard Chu ` (2 more replies) 0 siblings, 3 replies; 64+ messages in thread From: Nick Clifton @ 2020-11-03 15:14 UTC (permalink / raw) To: Howard Chu, Fangrui Song; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-03 15:14 ` Nick Clifton @ 2020-11-03 15:31 ` Howard Chu 2020-11-08 1:39 ` Alan Modra 2020-11-04 0:33 ` Howard Chu 2020-12-23 13:27 ` Matthias Klose 2 siblings, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-11-03 15:31 UTC (permalink / raw) To: Nick Clifton, Fangrui Song; +Cc: binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-03 15:31 ` Howard Chu @ 2020-11-08 1:39 ` Alan Modra 2020-11-08 15:07 ` Howard Chu 0 siblings, 1 reply; 64+ messages in thread From: Alan Modra @ 2020-11-08 1:39 UTC (permalink / raw) To: Howard Chu; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-08 1:39 ` Alan Modra @ 2020-11-08 15:07 ` Howard Chu 2020-11-09 0:01 ` Alan Modra 0 siblings, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-11-08 15:07 UTC (permalink / raw) To: Alan Modra; +Cc: binutils [-- 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-08 15:07 ` Howard Chu @ 2020-11-09 0:01 ` Alan Modra 2020-11-10 2:44 ` Howard Chu 0 siblings, 1 reply; 64+ messages in thread From: Alan Modra @ 2020-11-09 0:01 UTC (permalink / raw) To: Howard Chu; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-09 0:01 ` Alan Modra @ 2020-11-10 2:44 ` Howard Chu 2020-11-10 11:07 ` Alan Modra 0 siblings, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-11-10 2:44 UTC (permalink / raw) To: Alan Modra; +Cc: binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-10 2:44 ` Howard Chu @ 2020-11-10 11:07 ` Alan Modra 2020-11-11 14:57 ` Howard Chu 0 siblings, 1 reply; 64+ messages in thread From: Alan Modra @ 2020-11-10 11:07 UTC (permalink / raw) To: Howard Chu; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-10 11:07 ` Alan Modra @ 2020-11-11 14:57 ` Howard Chu 2020-11-11 14:59 ` Howard Chu 0 siblings, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-11-11 14:57 UTC (permalink / raw) To: Alan Modra; +Cc: binutils [-- 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-11 14:57 ` Howard Chu @ 2020-11-11 14:59 ` Howard Chu 2020-11-17 14:01 ` Nick Clifton 0 siblings, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-11-11 14:59 UTC (permalink / raw) To: Alan Modra; +Cc: binutils [-- 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-11 14:59 ` Howard Chu @ 2020-11-17 14:01 ` Nick Clifton 0 siblings, 0 replies; 64+ messages in thread From: Nick Clifton @ 2020-11-17 14:01 UTC (permalink / raw) To: Howard Chu, Alan Modra; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH] dependency list for static libraries 2020-11-03 15:14 ` Nick Clifton 2020-11-03 15:31 ` Howard Chu @ 2020-11-04 0:33 ` Howard Chu 2020-11-04 11:01 ` Nick Clifton 2020-12-23 13:27 ` Matthias Klose 2 siblings, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-11-04 0:33 UTC (permalink / raw) To: Nick Clifton; +Cc: binutils [-- 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-04 0:33 ` Howard Chu @ 2020-11-04 11:01 ` Nick Clifton 2020-11-04 14:50 ` Howard Chu 2020-12-15 20:33 ` [PATCH] dependency list for static libraries Cary Coutant 0 siblings, 2 replies; 64+ messages in thread From: Nick Clifton @ 2020-11-04 11:01 UTC (permalink / raw) To: Howard Chu; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-04 11:01 ` Nick Clifton @ 2020-11-04 14:50 ` Howard Chu 2020-11-06 12:38 ` Nick Clifton 2020-11-13 14:40 ` Howard Chu 2020-12-15 20:33 ` [PATCH] dependency list for static libraries Cary Coutant 1 sibling, 2 replies; 64+ messages in thread From: Howard Chu @ 2020-11-04 14:50 UTC (permalink / raw) To: Nick Clifton; +Cc: binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-04 14:50 ` Howard Chu @ 2020-11-06 12:38 ` Nick Clifton 2020-11-13 14:40 ` Howard Chu 1 sibling, 0 replies; 64+ messages in thread From: Nick Clifton @ 2020-11-06 12:38 UTC (permalink / raw) To: Howard Chu; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-04 14:50 ` Howard Chu 2020-11-06 12:38 ` Nick Clifton @ 2020-11-13 14:40 ` Howard Chu 2020-11-24 17:49 ` Howard Chu 1 sibling, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-11-13 14:40 UTC (permalink / raw) To: Nick Clifton; +Cc: binutils [-- 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-13 14:40 ` Howard Chu @ 2020-11-24 17:49 ` Howard Chu 2020-11-25 11:17 ` Nick Clifton 0 siblings, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-11-24 17:49 UTC (permalink / raw) To: Nick Clifton; +Cc: binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-24 17:49 ` Howard Chu @ 2020-11-25 11:17 ` Nick Clifton 2020-12-01 0:08 ` Howard Chu 0 siblings, 1 reply; 64+ messages in thread From: Nick Clifton @ 2020-11-25 11:17 UTC (permalink / raw) To: Howard Chu; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-25 11:17 ` Nick Clifton @ 2020-12-01 0:08 ` Howard Chu 2020-12-14 14:28 ` Nick Clifton 0 siblings, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-12-01 0:08 UTC (permalink / raw) To: Nick Clifton; +Cc: binutils [-- 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-12-01 0:08 ` Howard Chu @ 2020-12-14 14:28 ` Nick Clifton 2020-12-15 16:17 ` Jim Wilson 0 siblings, 1 reply; 64+ messages in thread From: Nick Clifton @ 2020-12-14 14:28 UTC (permalink / raw) To: Howard Chu; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-12-14 14:28 ` Nick Clifton @ 2020-12-15 16:17 ` Jim Wilson 2020-12-15 16:22 ` Jeff Law 0 siblings, 1 reply; 64+ messages in thread From: Jim Wilson @ 2020-12-15 16:17 UTC (permalink / raw) To: Nick Clifton; +Cc: Howard Chu, Binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-12-15 16:17 ` Jim Wilson @ 2020-12-15 16:22 ` Jeff Law 2020-12-15 16:50 ` Nick Clifton 0 siblings, 1 reply; 64+ messages in thread From: Jeff Law @ 2020-12-15 16:22 UTC (permalink / raw) To: Jim Wilson, Nick Clifton; +Cc: Binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-12-15 16:22 ` Jeff Law @ 2020-12-15 16:50 ` Nick Clifton 2020-12-15 19:11 ` Jeff Law 2020-12-15 20:22 ` Cary Coutant 0 siblings, 2 replies; 64+ messages in thread From: Nick Clifton @ 2020-12-15 16:50 UTC (permalink / raw) To: Jeff Law, Jim Wilson; +Cc: Binutils [-- 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; ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-12-15 16:50 ` Nick Clifton @ 2020-12-15 19:11 ` Jeff Law 2020-12-15 20:04 ` Jim Wilson 2020-12-15 20:22 ` Cary Coutant 1 sibling, 1 reply; 64+ messages in thread From: Jeff Law @ 2020-12-15 19:11 UTC (permalink / raw) To: Nick Clifton, Jim Wilson; +Cc: Binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-12-15 19:11 ` Jeff Law @ 2020-12-15 20:04 ` Jim Wilson 0 siblings, 0 replies; 64+ messages in thread From: Jim Wilson @ 2020-12-15 20:04 UTC (permalink / raw) To: Jeff Law; +Cc: Nick Clifton, Binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-12-15 16:50 ` Nick Clifton 2020-12-15 19:11 ` Jeff Law @ 2020-12-15 20:22 ` Cary Coutant 2020-12-15 20:51 ` Howard Chu 1 sibling, 1 reply; 64+ messages in thread From: Cary Coutant @ 2020-12-15 20:22 UTC (permalink / raw) To: Nick Clifton; +Cc: Jeff Law, Jim Wilson, Binutils > 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-12-15 20:22 ` Cary Coutant @ 2020-12-15 20:51 ` Howard Chu 2020-12-16 11:16 ` Nick Clifton 0 siblings, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-12-15 20:51 UTC (permalink / raw) To: Cary Coutant, Nick Clifton; +Cc: Binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-12-15 20:51 ` Howard Chu @ 2020-12-16 11:16 ` Nick Clifton 2020-12-16 14:49 ` [PATCH] ld: Call plugin hooks only if they are available H.J. Lu 0 siblings, 1 reply; 64+ messages in thread From: Nick Clifton @ 2020-12-16 11:16 UTC (permalink / raw) To: Howard Chu, Cary Coutant; +Cc: Binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH] ld: Call plugin hooks only if they are available 2020-12-16 11:16 ` Nick Clifton @ 2020-12-16 14:49 ` H.J. Lu 2020-12-16 18:34 ` Howard Chu 0 siblings, 1 reply; 64+ messages in thread From: H.J. Lu @ 2020-12-16 14:49 UTC (permalink / raw) To: Nick Clifton; +Cc: Howard Chu, Cary Coutant, Binutils [-- 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] ld: Call plugin hooks only if they are available 2020-12-16 14:49 ` [PATCH] ld: Call plugin hooks only if they are available H.J. Lu @ 2020-12-16 18:34 ` Howard Chu 2020-12-16 18:40 ` H.J. Lu 2020-12-16 18:44 ` [PATCH] ld: Call plugin hooks only if they " Howard Chu 0 siblings, 2 replies; 64+ messages in thread From: Howard Chu @ 2020-12-16 18:34 UTC (permalink / raw) To: H.J. Lu, Nick Clifton; +Cc: Cary Coutant, Binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] ld: Call plugin hooks only if they are available 2020-12-16 18:34 ` Howard Chu @ 2020-12-16 18:40 ` H.J. Lu 2020-12-16 19:06 ` Howard Chu 2020-12-16 18:44 ` [PATCH] ld: Call plugin hooks only if they " Howard Chu 1 sibling, 1 reply; 64+ messages in thread From: H.J. Lu @ 2020-12-16 18:40 UTC (permalink / raw) To: Howard Chu; +Cc: Nick Clifton, Cary Coutant, Binutils 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] ld: Call plugin hooks only if they are available 2020-12-16 18:40 ` H.J. Lu @ 2020-12-16 19:06 ` Howard Chu 2020-12-16 19:11 ` [PATCH] ld: Skip libdep plugin if not all plugin hooks " H.J. Lu 0 siblings, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-12-16 19:06 UTC (permalink / raw) To: H.J. Lu; +Cc: Nick Clifton, Cary Coutant, Binutils [-- 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH] ld: Skip libdep plugin if not all plugin hooks are available 2020-12-16 19:06 ` Howard Chu @ 2020-12-16 19:11 ` H.J. Lu 2020-12-16 21:26 ` Howard Chu 0 siblings, 1 reply; 64+ messages in thread From: H.J. Lu @ 2020-12-16 19:11 UTC (permalink / raw) To: Howard Chu; +Cc: Nick Clifton, Cary Coutant, Binutils [-- 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] ld: Skip libdep plugin if not all plugin hooks are available 2020-12-16 19:11 ` [PATCH] ld: Skip libdep plugin if not all plugin hooks " H.J. Lu @ 2020-12-16 21:26 ` Howard Chu 2020-12-16 21:47 ` H.J. Lu 0 siblings, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-12-16 21:26 UTC (permalink / raw) To: H.J. Lu; +Cc: Nick Clifton, Cary Coutant, Binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] ld: Skip libdep plugin if not all plugin hooks are available 2020-12-16 21:26 ` Howard Chu @ 2020-12-16 21:47 ` H.J. Lu 0 siblings, 0 replies; 64+ messages in thread From: H.J. Lu @ 2020-12-16 21:47 UTC (permalink / raw) To: Howard Chu; +Cc: Nick Clifton, Cary Coutant, Binutils 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] ld: Call plugin hooks only if they are available 2020-12-16 18:34 ` Howard Chu 2020-12-16 18:40 ` H.J. Lu @ 2020-12-16 18:44 ` Howard Chu 1 sibling, 0 replies; 64+ messages in thread From: Howard Chu @ 2020-12-16 18:44 UTC (permalink / raw) To: H.J. Lu, Nick Clifton; +Cc: Cary Coutant, Binutils [-- 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-04 11:01 ` Nick Clifton 2020-11-04 14:50 ` Howard Chu @ 2020-12-15 20:33 ` Cary Coutant 2020-12-15 20:53 ` Howard Chu 1 sibling, 1 reply; 64+ messages in thread From: Cary Coutant @ 2020-12-15 20:33 UTC (permalink / raw) To: Nick Clifton; +Cc: Howard Chu, Binutils > > 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-12-15 20:33 ` [PATCH] dependency list for static libraries Cary Coutant @ 2020-12-15 20:53 ` Howard Chu 2020-12-16 11:18 ` Nick Clifton 0 siblings, 1 reply; 64+ messages in thread From: Howard Chu @ 2020-12-15 20:53 UTC (permalink / raw) To: Cary Coutant, Nick Clifton; +Cc: Binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-12-15 20:53 ` Howard Chu @ 2020-12-16 11:18 ` Nick Clifton 0 siblings, 0 replies; 64+ messages in thread From: Nick Clifton @ 2020-12-16 11:18 UTC (permalink / raw) To: Howard Chu, Cary Coutant; +Cc: Binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-11-03 15:14 ` Nick Clifton 2020-11-03 15:31 ` Howard Chu 2020-11-04 0:33 ` Howard Chu @ 2020-12-23 13:27 ` Matthias Klose 2020-12-23 18:23 ` Howard Chu 2 siblings, 1 reply; 64+ messages in thread From: Matthias Klose @ 2020-12-23 13:27 UTC (permalink / raw) To: Nick Clifton, Howard Chu, Fangrui Song; +Cc: binutils 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 ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries 2020-12-23 13:27 ` Matthias Klose @ 2020-12-23 18:23 ` Howard Chu 0 siblings, 0 replies; 64+ messages in thread From: Howard Chu @ 2020-12-23 18:23 UTC (permalink / raw) To: Matthias Klose, Nick Clifton, Fangrui Song; +Cc: binutils 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/ ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH] dependency list for static libraries
@ 2020-09-30 10:33 Peter Smith
2020-10-28 14:35 ` Howard Chu
0 siblings, 1 reply; 64+ messages in thread
From: Peter Smith @ 2020-09-30 10:33 UTC (permalink / raw)
To: Fangrui Song; +Cc: binutils
> 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.
Apologies in advance if the webmail client mangles the response or threading.
Just wanted to add to MaskRay's concerns about what the semantics are for a linker.
* Where are the static library dependencies placed in the order of input files? Immediately after the static library seems the most logical choice although this will make it harder for users to interpose objects/libraries to override choices.
* What happens when there are cyclic dependencies between libraries? Would the user be expected to know to wrap an individual library in a start-group end-group? Should the linker implicitly add start-group end-group around the dependencies?
* This does seem like it will work for libraries designed to be incorporated as a whole (like a dynamic library), but it seems like it could have awkward corner cases.
Although not fundamentally opposed to this; personally I'm not a fan of encoding dependencies for static libraries. I see these more of a collection of objects and not a single entity like a dynamic library. If static libraries are being used whole, one way of implementing this would be to add a member to the library that is just a linker script with INPUT and GROUP commands, and link using --whole-archive.
Peter
(infrequent LLD contributor)
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH] dependency list for static libraries 2020-09-30 10:33 Peter Smith @ 2020-10-28 14:35 ` Howard Chu 0 siblings, 0 replies; 64+ messages in thread From: Howard Chu @ 2020-10-28 14:35 UTC (permalink / raw) To: Peter Smith, Fangrui Song; +Cc: binutils Peter Smith via Binutils wrote: > Apologies in advance if the webmail client mangles the response or threading. > > Just wanted to add to MaskRay's concerns about what the semantics are for a linker. > * Where are the static library dependencies placed in the order of input files? Immediately after the static library seems the most logical choice although this will make it harder for users to interpose objects/libraries to override choices. One major advantage of this approach is that if an end user needs to make changes, they can easily do so without requiring the original source code or a compiler. > * What happens when there are cyclic dependencies between libraries? Would the user be expected to know to wrap an individual library in a start-group end-group? Should the linker implicitly add start-group end-group around the dependencies? I don't see how this is relevant. Circular dependencies would be a problem and would require the same solution regardless. > * This does seem like it will work for libraries designed to be incorporated as a whole (like a dynamic library), but it seems like it could have awkward corner cases. > > Although not fundamentally opposed to this; personally I'm not a fan of encoding dependencies for static libraries. I see these more of a collection of objects and not a single entity like a dynamic library. If static libraries are being used whole, one way of implementing this would be to add a member to the library that is just a linker script with INPUT and GROUP commands, and link using --whole-archive. If you're making a lot of use of --whole-archive You're Doing It Wrong. Should just use "ld -r -o aggregate.o foo.o bar.o ..." and save the linker trouble later. Though this raises an interesting point, why not just eliminate the distinction between libraries and individual .o files, like the AIX linker does? I.e., the AIX linker is capable of extracting individual required symbols from a .o file, instead of just linking the entire thing as a unit. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/ ^ permalink raw reply [flat|nested] 64+ messages in thread
end of thread, other threads:[~2020-12-23 18:23 UTC | newest]
Thread overview: 64+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-19 15:49 dependency list for static libraries Howard Chu
2017-09-19 15:52 ` Simon Richter
[not found] ` <WM!bae999665f49907786872b93f01ac98d53e7b97e29b4228399d8baadf9ec0ab33db74467d73c998225b250ba1d00a4c0!@mailstronghold-3.zmailcloud.com>
2017-09-19 16:04 ` Howard Chu
2017-09-20 1:42 ` R0b0t1
2017-09-19 16:54 ` Joseph Myers
[not found] ` <WM!83b6ad7285aa96ce69fcd1944d4eae8f20e5f19dfbf161f45313f5393bcffe1b77231520b8f4e24145a3f85eeafb39ed!@mailstronghold-1.zmailcloud.com>
2017-09-19 22:01 ` Howard Chu
2017-09-20 0:20 ` Joseph Myers
2020-09-03 20:42 ` Howard Chu
2020-09-22 10:39 ` Nick Clifton
2020-09-22 11:42 ` Howard Chu
2020-09-22 13:12 ` Nick Clifton
2020-09-22 16:23 ` [PATCH] " Howard Chu
2020-09-22 17:16 ` Fangrui Song
2020-09-22 17:55 ` Howard Chu
2020-09-22 20:46 ` Howard Chu
2020-09-23 11:52 ` Nick Clifton
2020-09-23 15:29 ` Howard Chu
2020-09-24 5:21 ` Fangrui Song
2020-09-24 9:19 ` Howard Chu
2020-09-24 9:30 ` Howard Chu
2020-09-28 11:07 ` Howard Chu
2020-10-28 14:56 ` Howard Chu
2020-11-03 15:14 ` Nick Clifton
2020-11-03 15:31 ` Howard Chu
2020-11-08 1:39 ` Alan Modra
2020-11-08 15:07 ` Howard Chu
2020-11-09 0:01 ` Alan Modra
2020-11-10 2:44 ` Howard Chu
2020-11-10 11:07 ` Alan Modra
2020-11-11 14:57 ` Howard Chu
2020-11-11 14:59 ` Howard Chu
2020-11-17 14:01 ` Nick Clifton
2020-11-04 0:33 ` Howard Chu
2020-11-04 11:01 ` Nick Clifton
2020-11-04 14:50 ` Howard Chu
2020-11-06 12:38 ` Nick Clifton
2020-11-13 14:40 ` Howard Chu
2020-11-24 17:49 ` Howard Chu
2020-11-25 11:17 ` Nick Clifton
2020-12-01 0:08 ` Howard Chu
2020-12-14 14:28 ` Nick Clifton
2020-12-15 16:17 ` Jim Wilson
2020-12-15 16:22 ` Jeff Law
2020-12-15 16:50 ` Nick Clifton
2020-12-15 19:11 ` Jeff Law
2020-12-15 20:04 ` Jim Wilson
2020-12-15 20:22 ` Cary Coutant
2020-12-15 20:51 ` Howard Chu
2020-12-16 11:16 ` Nick Clifton
2020-12-16 14:49 ` [PATCH] ld: Call plugin hooks only if they are available H.J. Lu
2020-12-16 18:34 ` Howard Chu
2020-12-16 18:40 ` H.J. Lu
2020-12-16 19:06 ` Howard Chu
2020-12-16 19:11 ` [PATCH] ld: Skip libdep plugin if not all plugin hooks " H.J. Lu
2020-12-16 21:26 ` Howard Chu
2020-12-16 21:47 ` H.J. Lu
2020-12-16 18:44 ` [PATCH] ld: Call plugin hooks only if they " Howard Chu
2020-12-15 20:33 ` [PATCH] dependency list for static libraries Cary Coutant
2020-12-15 20:53 ` Howard Chu
2020-12-16 11:18 ` Nick Clifton
2020-12-23 13:27 ` Matthias Klose
2020-12-23 18:23 ` Howard Chu
2020-09-30 10:33 Peter Smith
2020-10-28 14:35 ` Howard Chu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).