[PATCH] Add plugin interface to LD [0/4]

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

[PATCH] Add plugin interface to LD [0/4]

Dave Korn-9

    Hi list,

  The following set of patches implements the plugin API as seen in GOLD into
GNU LD.  I've done it in a set of steps that first add the infrastructure,
then fill out the functionality in logical progression, and I'll comment on
noticeable points of each patch as I post them, but first a few overall comments.

  It follows the design outlined at http://gcc.gnu.org/wiki/whopr/driver, with
one significant exception so far: the GET_INPUT_FILE and RELEASE_INPUT_FILE
methods aren't implemented yet (they return errors if called).

  I think it's OK to leave these unimplemented for now, because the GCC LTO
plugin doesn't use them, and that's the main target of this work.  I will add
them later, or sooner in the case that anything actually starts trying to use
them!

  There are two other unsupported features as well: it doesn't handle
libraries yet (in terms of offering the individual members to the claim files
hook), and it doesn't support ELF visibility settings.  These two things I
would like fix with follow-on patches over the coming days, but I think that
the feature as it stands is sufficiently complete to commit now anyway, for
the following reasons:

- ELF visibility handling doesn't need to be a first priority because ELF
targets should be using GOLD for best results anyway; this patch is sufficient
for non-ELF targets that don't have the option of using GOLD and don't need
visibility support anyway.

- Linking against library files still works, but the library contents won't be
available for LTO.  The rest of the program still will be, and since
LTO-compiled objects (and hence archives) contain native assembly sections as
well as LTO sections, the compiled code will still work.

- Since we only release once a year, I'd really like to have this in 2.21, for
the sake of supporting LTO on non-ELF targets when GCC 4.6.0 comes out,
otherwise it'll be another year until the whole toolchain fully supports LTO
on COFF (et al.), even if there are bugs that mean it's not fully working
until 2.21.1 (although I fully hope to have it working for 2.20.0).

- Even though it's not fully complete, it's good enough to start porting work
on the GCC LTO plugin, which will hopefully shake out any remaining bugs
fairly quickly; like before the end of GCC stage 1 next month.

- It's only conditionally compiled in by a configure-time option, so adding it
now can't break anything; but it at least gives us the chance of having LTO
support in the 2.21/4.6 combination.

  I've been testing it on both i686-pc-cygwin and i686-pc-linux-gnu (with
--enable-64-bit-bfd) and it works on both, and has the testcases to show it.
I hope it's not too controversial to commit right now because it really is at
the 95% point and pretty safe.

  Notes on the implementation will follow in the mails carrying the individual
patches.

  What does everyone think?  Ok?

    cheers,
      DaveK

Reply | Threaded
Open this post in threaded view
|

[PATCH] Add plugin interface to LD [1/4] Infrastructure.

Dave Korn-9

  This is the first patch in the series: it adds the infrastructure for
parsing plugin-related command line options, and for loading, initialising,
and cleaning up the plugins.  Notes, in no particular order:

- I added a --enable-plugins command to ld's configure before I remembered
that there's one in the binutils/ subdir (for nm and ar) as well.  Should I
perhaps change it to --enable-ld-plugins (or something else) so that it can be
controlled separately?  Should I rewrite it to use config/plugins.m4?

- Is it OK to put an unguarded "#include <stdarg.h>" in ldmisc.h?

- There are a couple of ever-so-slightly long lines and not enough comments;
both of these points get fixed in the later patches.  I've split them up for
ease of review but would like to apply them either in one, or in rapid
succession, so that it'll be corrected immediately.

    include/ChangeLog:

        * plugin-api.h (LDPT_GNU_LD_VERSION): New ld_plugin_tag enum member.

    ld/ChangeLog:

        * configure.in: Add --enable-plugins option and additional AC_CHECKs
        for required headers and functions.
        (ENABLE_PLUGINS): Add related automake conditional.
        * configure: Regenerate.
        * config.in: Likewise.
        * Makefile.am (PLUGIN_C): Declare plugin C source file, conditional
        on ENABLE_PLUGINS being defined.
        (PLUGIN_H): Likewise for header file.
        (PLUGIN_OBJEXT): Likewise for object file.
        (PLUGIN_CFLAGS): Likewise -D flag required to compile plugin support.
        (AM_CFLAGS): Use PLUGIN_CFLAGS.
        (CFILES): Use PLUGIN_C.
        (HFILES): Use PLUGIN_H.
        (OFILES): Use PLUGIN_OBJEXT.
        (ld_new_SOURCES): Use PLUGIN_C.
        (noinst_LTLIBRARIES)[ENABLE_PLUGINS]: Declare test plugin.
        (libldtestplug_la_SOURCES)[ENABLE_PLUGINS]: Add automake definition
        for test plugin.
        (libldtestplug_la_CFLAGS)[ENABLE_PLUGINS]: Likewise.
        (libldtestplug_la_LDFLAGS)[ENABLE_PLUGINS]: Likewise.
        * Makefile.in: Regenerate.
        * sysdep.h: Include unistd.h and one of fcntl.h or sys/file.h where
        available.
        (O_RDONLY): Supply default definition likewise to bfd's sysdep.h
        (O_WRONLY): Likewise.
        (O_RDWR): Likewise.
        (O_ACCMODE): Likewise.
        (SEEK_SET): Likewise.
        (SEEK_CUR): Likewise.
        (SEEK_END): Likewise.
        * ldmisc.c (vfinfo): Make non-static.
        * ldmisc.h: Include stdarg.h for va_list type.
        (vfinfo): Declare extern prototype.
        * lexsup.c (enum option_values)[ENABLE_PLUGINS]: Add new entries for
        OPTION_PLUGIN and OPTION_PLUGIN_ARG.
        (ld_options[])[ENABLE_PLUGINS]: Add option data for the above two.
        (parse_args)[ENABLE_PLUGINS]: Handle them, and load all plugins once
        option parsing is complete.
        * ldmain.c (main)[ENABLE_PLUGINS]: Call plugin cleanup hooks just
        after lang_finish.
        * plugin.c: New source file.
        * plugin.h: Likewise new header.
        * testplug.c: New source file.

    ld/testsuite/ChangeLog:

        * lib/ld-lib.exp (proc regexp_diff): Extend verbose debug output.
        (proc set_file_contents): Write a file with the supplied content.
        (run_ld_link_tests): Add new 'ld' action to test linker output.
        (proc check_plugin_api_available): Return true if linker under test
        supports the plugin API.
        * ld-plugin/func.c: New test source file.
        * ld-plugin/main.c: Likewise.
        * ld-plugin/text.c: Likewise.
        * ld-plugin/plugin-1.d: New dump test output pattern script.
        * ld-plugin/plugin-2.d: Likewise.
        * ld-plugin/plugin-3.d: Likewise.
        * ld-plugin/plugin-4.d: Likewise.
        * ld-plugin/plugin-5.d: Likewise.
        * ld-plugin/plugin.exp: New test control script.

    cheers,
      DaveK

ld-plugin-api-1-infra.diff (54K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.

Dave Korn-9
In reply to this post by Dave Korn-9


  This patch applies on top of the previous one to add the mentioned
functionality.

-  When a plugin claims an input file, we create an empty BFD from scratch and
use it to store the symbols returned by the plugin, so that they can then take
part in the link and be resolved by the linker.  Later on, we'll be able to
swap the resolved symbols in the linker hash table around so that they point
at the real versions of these symbols that are in the object files that the
plugin generates and adds back to the link.

- I added a single-bit flag to the input statement struct to track whether a
file has been claimed or not.  I didn't actually end up using it yet, so I
could remove it if desired, but it's only a single bit and doesn't change the
total size of the struct any.

- There's a minor bug in the definition of IRONLY_SUFFIX_LEN causing
is_ir_dummy_bfd to always return FALSE, but this function isn't used until the
next patch anyway.  (During an earlier stage of development I did use it
during this patch, until I reimplemented things slightly differently, but
since I knew I'd need it for the next patch anyway, I didn't take it out; at
which time it was corrected.)

- I'd certainly appreciate a second pair of eyes looking at the logic in
asymbol_from_plugin_symbol by which I derive the fags etc. for a bfd asymbol
from the ld plugin symbol struct.

  ld/ChangeLog:

        * Makefile.am (libldtestplug_la_LDFLAGS): Link against libiberty.
        * Makefile.in: Regenerate.
        * ldfile.c (ldfile_try_open_bfd)[ENABLE_PLUGINS]: Don't return early
        during compat checks if they pass, instead offer any successfully
        opened and accepted file to the plugin claim file hooks chain.  Create
        a dummy bfd to accept symbols added by the plugin, if the plugin
        claims the file.
        * ldlang.c (lang_process)[ENABLE_PLUGINS]: Call plugin all symbols
        read hook chain before ldemul_after_open.
        * ldlang.h (struct lang_input_statement_struct): Add new single-bit
        'claimed' flag.
        * plugin.c: Fix order of includes.
        (plugin_get_ir_dummy_bfd): New function to create the dummy bfd used
        to store symbols for ir-only files.
        (is_ir_dummy_bfd): New function to check if a bfd is ir-only.
        (asymbol_from_plugin_symbol): New function converts symbol formats.
        (add_symbols): Call it to convert plugin syms to bfd syms and add
        them to the dummy bfd.
        * plugin.h: Add missing include guards.
        (plugin_get_ir_dummy_bfd): Add prototype.
        (is_ir_dummy_bfd): Likewise.
        * testplug.c: Fix order of includes.
        (TV_MESSAGE): New helper macro.
        (struct claim_file): New struct.
        (claim_file_t): New typedef.
        (tag_names[]): Make static.
        (claimfiles_list): New variable.
        (claimfiles_tail_chain_ptr): Likewise.
        (last_claimfile): Likewise.
        (record_claim_file): Record a file to claim on a singly-linked list.
        (parse_symdefstr): Parse an ASCII representation of a symbol from a
        plugin option into the fields of a struct ld_plugin_symbol.
        (record_claimed_file_symbol):  Use it to parse plugin option for
        adding a symbol.
        (parse_option): Parse claim file and add symbol options.
        (dump_tv_tag): Use TV_MESSAGE.
        (onload): Likewise.
        (onclaim_file): Make static.  Use TV_MESSAGE.  Scan list of files to
        claim and claim this file if required, adding any symbols specified.
        (onall_symbols_read): Make static and use TV_MESSAGE.
        (oncleanup): Likewise.

  ld/testsuite/ChangeLog:

        * ld-plugin/plugin-3.d: Enable regexes for new functionality.
        * ld-plugin/plugin-5.d: Likewise.
        * ld-plugin/plugin-6.d: New testcase.
        * ld-plugin/plugin-7.d: Likewise.
        * ld-plugin/plugin.exp: Use 'nm' on compiled test objects to determine
        whether symbols in plugin arguments need an underscore prefix.  Add
        new plugin-6.d and plugin-7.d testcases.


    cheers,
      DaveK


ld-plugin-api-2-claimfiles-addsyms.diff (34K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH] Add plugin interface to LD [3/4] Get symbols and resolutions.

Dave Korn-9
In reply to this post by Dave Korn-9

  This one works by using the bfd linker's "notice symbol" callback to track
all symbols that are referenced by non-IR files, so that we can later
determine for prevailing defs from IR files whether or not they are IR-only.
It's pretty simple and the only minor trickiness is turing on notice_all
without inadvertently behaving like we've invoked --trace-sym on the whole world.

- As with the asymbol conversion in the previous patch, I'd appreciate extra
eyeballs looking at the logic here in get_symbols by which I determine
LDPR_xxx resolution types from the info in the linker hash table, just to
double-check my working.

  ld/ChangeLog:

        * ldmain.c (notice)[ENABLE_PLUGINS]: Call plugin_notice.
        * plugin.c (non_ironly_hash): Add new bfd hash table.
        (plugin_load_plugins): Exit early if no plugins to load.  If plugins
        do load successfully, set notice_all flag in link info.
        (add_symbols): Clean up for-loop slightly.
        (get_symbols): Implement.
        (init_non_ironly_hash): Lazily init non_ironly_hash table.
        (plugin_notice): Record symbols referenced from non-IR files in the
        non_ironly_hash.  Suppress tracing, cref generation and nocrossrefs
        tracking for symbols from dummy IR bfds.
        * plugin.h: Fix formatting.
        (plugin_notice): Add prototype.
        * testplug.c (dumpresolutions): New global var.
        (parse_options): Accept "dumpresolutions".
        (onall_symbols_read): Get syms and dump resolutions if it was given.

  ld/testsuite/ChangeLog:

        * ld-plugin/plugin-8.d: New testcase.
        * ld-plugin/plugin.exp: Invoke it.

    cheers,
      DaveK



ld-plugin-api-3-get-symbols.diff (15K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH] Add plugin interface to LD [4/4] Add files, libs and dir paths.

Dave Korn-9
In reply to this post by Dave Korn-9
  This is the final functionality needed to start supporting the GCC LTO plugin.

- Adding new files/libs/search dirs are fairly trivial callouts to existing
functions in ld, so hopefully the added files should behave just like real
ones, with consistent lang_statements and all.

- It makes use of the bfd linker's multiple-definitions callback to spot when
a freshly-added object file contains a real symbol that was initially supplied
from an IR file, and replaces the old definition with the new one.  This
should correctly cut the dummy bfd right out of the final link, I hope -
unless someone knows some reason why whipping the rug out from under BFD's
feet like this wouldn't work or might confuse it?


  ld/ChangeLog:

        * ldlang.c (lang_process)[ENABLE_PLUGINS]: Move invocation of
        plugin_call_all_symbols_read to before setting of gc_sym_list, and
        open any new input files that may have been added during it.
        * ldmain.c (multiple_definition)[ENABLE_PLUGINS]: Call out to
        plugin_multiple_definition and let it have first say over what to do
        with the clashing definitions.
        * plugin.c (no_more_claiming): New boolean variable.
        (plugin_cached_allow_multiple_defs): Likewise.
        (plugin_call_claim_file): Don't do anything when no_more_claiming set.
        (plugin_call_all_symbols_read): Set it.  Disable link info
        "allow_multiple_definition" flag, but cache its value.
        (add_input_file): Implement.
        (add_input_library): Likewise.
        (set_extra_library_path): Likewise.
        (plugin_multiple_definition): New function.
        * plugin.h (plugin_multiple_definition): Add prototype.
        * testplug.c (addfile_enum_t): New enumerated typedef.
        (add_file_t): New struct typedef.
        (addfiles_list): New variable.
        (addfiles_tail_chain_ptr): Likewise.
        (record_add_file): New function.
        (parse_option): Parse "add:", "lib:" and "dir:" options and call it.
        (onall_symbols_read): Iterate the list of new files, libs and dirs,
        adding them.

  ld/testsuite/ChangeLog:

        * testsuite/ld-plugin/plugin-9.d: New testcase.
        * ld-plugin/plugin.exp: Invoke it.


    cheers,
      DaveK



ld-plugin-api-4-add-files.diff (15K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.

Ralf Wildenhues
In reply to this post by Dave Korn-9
Hi Dave,

* Dave Korn wrote on Thu, Sep 23, 2010 at 07:31:41AM CEST:
> --- a/ld/Makefile.am
> +++ b/ld/Makefile.am
> @@ -1989,7 +1989,7 @@ if ENABLE_PLUGINS
>  noinst_LTLIBRARIES = libldtestplug.la
>  libldtestplug_la_SOURCES = testplug.c
>  libldtestplug_la_CFLAGS= -g -O2
> -libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere
> +libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere -Wl,$(LIBIBERTY)

Why the '-Wl,' prefix?  I'd have expected $(LIBIBERTY) to be added to
libldtestplug_la_LIBADD.

If you need to work around libtool warning about adding static library
deps to a shared library, then I suggest at least -Wc, rather than -Wl,
but still I think you will run into troubles on static-only systems.

>  endif
>  
>  # DOCUMENTATION TARGETS

Cheers,
Ralf
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add plugin interface to LD [1/4] Infrastructure.

Ralf Wildenhues
In reply to this post by Dave Korn-9
Hi Dave,

* Dave Korn wrote on Thu, Sep 23, 2010 at 07:31:17AM CEST:

> --- a/ld/Makefile.am
> +++ b/ld/Makefile.am
> @@ -21,6 +21,21 @@ WARN_CFLAGS = @WARN_CFLAGS@
>  NO_WERROR = @NO_WERROR@
>  AM_CFLAGS = $(WARN_CFLAGS)
>  
> +# Conditionally enable the plugin interface.
> +if ENABLE_PLUGINS
> +PLUGIN_C = plugin.c
> +PLUGIN_H = plugin.h
> +PLUGIN_OBJEXT = plugin.@OBJEXT@

Maybe rather PLUGIN_OBJECT, because it's not only an extension?

> +PLUGIN_CFLAGS = -DENABLE_PLUGINS
> +else
> +PLUGIN_C =
> +PLUGIN_H =
> +PLUGIN_OBJEXT =
> +PLUGIN_CFLAGS =
> +endif
> +
> +AM_CFLAGS += $(PLUGIN_CFLAGS)

AM_CPPFLAGS for -D flags, I'd say.

Cheers,
Ralf
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add plugin interface to LD [1/4] Infrastructure.

Dave Korn-9
On 23/09/2010 06:41, Ralf Wildenhues wrote:

> Maybe rather PLUGIN_OBJECT, because it's not only an extension?

> AM_CPPFLAGS for -D flags, I'd say.

  Thanks, will do!

    cheers,
      DaveK

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.

Dave Korn-9
In reply to this post by Ralf Wildenhues
On 23/09/2010 06:36, Ralf Wildenhues wrote:

> Hi Dave,
>
> * Dave Korn wrote on Thu, Sep 23, 2010 at 07:31:41AM CEST:
>> --- a/ld/Makefile.am
>> +++ b/ld/Makefile.am
>> @@ -1989,7 +1989,7 @@ if ENABLE_PLUGINS
>>  noinst_LTLIBRARIES = libldtestplug.la
>>  libldtestplug_la_SOURCES = testplug.c
>>  libldtestplug_la_CFLAGS= -g -O2
>> -libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere
>> +libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere -Wl,$(LIBIBERTY)
>
> Why the '-Wl,' prefix?  I'd have expected $(LIBIBERTY) to be added to
> libldtestplug_la_LIBADD.

  libiberty isn't built under libtool control at all, AFAIK, and is only ever
built as a static archive, so I was under the impression I've got to pass it
around behind libtool's back like this.

> If you need to work around libtool warning about adding static library
> deps to a shared library, then I suggest at least -Wc, rather than -Wl,
> but still I think you will run into troubles on static-only systems.

  What kind of problems?

    cheers,
      DaveK
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.

Dave Korn-9
On 23/09/2010 07:25, Dave Korn wrote:
> On 23/09/2010 06:36, Ralf Wildenhues wrote:

>> If you need to work around libtool warning about adding static library
>> deps to a shared library, then I suggest at least -Wc, rather than -Wl,
>> but still I think you will run into troubles on static-only systems.
>
>   What kind of problems?

  Uh, wait a minute, what are we even talking about?  Plugins only exist as
DSOs.  There aren't going to be any plugins on static-only systems, right?

  I'm going to go get a few hours sleep!

    cheers,
      DaveK

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.

Ralf Wildenhues
In reply to this post by Dave Korn-9
* Dave Korn wrote on Thu, Sep 23, 2010 at 08:25:42AM CEST:

> On 23/09/2010 06:36, Ralf Wildenhues wrote:
> > * Dave Korn wrote on Thu, Sep 23, 2010 at 07:31:41AM CEST:
> >> --- a/ld/Makefile.am
> >> +++ b/ld/Makefile.am
> >> @@ -1989,7 +1989,7 @@ if ENABLE_PLUGINS
> >>  noinst_LTLIBRARIES = libldtestplug.la
> >>  libldtestplug_la_SOURCES = testplug.c
> >>  libldtestplug_la_CFLAGS= -g -O2
> >> -libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere
> >> +libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere -Wl,$(LIBIBERTY)
> >
> > Why the '-Wl,' prefix?  I'd have expected $(LIBIBERTY) to be added to
> > libldtestplug_la_LIBADD.
>
>   libiberty isn't built under libtool control at all, AFAIK, and is only ever
> built as a static archive, so I was under the impression I've got to pass it
> around behind libtool's back like this.

Not because of that, no.

> > If you need to work around libtool warning about adding static library
> > deps to a shared library, then I suggest at least -Wc, rather than -Wl,
> > but still I think you will run into troubles on static-only systems.
>
>   What kind of problems?

Ordering.  You can't rely on libtool expanding -Wl, link flags after the
objects, meaning that libiberty may not be picked up at all.  (With
static linking, objects and libraries are evaluated in a strict command
line order, so all needed libraries must come after objects that need
them.)

Cheers,
Ralf
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.

Ralf Wildenhues
In reply to this post by Dave Korn-9
* Dave Korn wrote on Thu, Sep 23, 2010 at 08:35:26AM CEST:

> On 23/09/2010 07:25, Dave Korn wrote:
> > On 23/09/2010 06:36, Ralf Wildenhues wrote:
>
> >> If you need to work around libtool warning about adding static library
> >> deps to a shared library, then I suggest at least -Wc, rather than -Wl,
> >> but still I think you will run into troubles on static-only systems.
> >
> >   What kind of problems?
>
>   Uh, wait a minute, what are we even talking about?  Plugins only exist as
> DSOs.  There aren't going to be any plugins on static-only systems, right?

OK but ordering issues exist elsewhere too.

For static-only systems it might be sufficient to just not build this
thing when $enable_shared is not true in configure after
AC_PROG_LIBTOOL.

Cheers,
Ralf
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add plugin interface to LD [0/4]

Richard Henderson-2
In reply to this post by Dave Korn-9
On 09/22/2010 10:29 PM, Dave Korn wrote:
> - ELF visibility handling doesn't need to be a first priority because ELF
> targets should be using GOLD for best results anyway; this patch is sufficient
> for non-ELF targets that don't have the option of using GOLD and don't need
> visibility support anyway.

"Should be", but there's a whole collection of elf targets that
are unlikely to ever be re-written for GOLD.

But you're right that it needn't be a first priority.


r~
Reply | Threaded
Open this post in threaded view
|

[PATCH] Add plugin interface to LD [0/4] Support ELF symbol visibility

Dave Korn-9
On 23/09/2010 17:18, Richard Henderson wrote:

> On 09/22/2010 10:29 PM, Dave Korn wrote:
>> - ELF visibility handling doesn't need to be a first priority because ELF
>> targets should be using GOLD for best results anyway; this patch is sufficient
>> for non-ELF targets that don't have the option of using GOLD and don't need
>> visibility support anyway.
>
> "Should be", but there's a whole collection of elf targets that
> are unlikely to ever be re-written for GOLD.
>
> But you're right that it needn't be a first priority.
>
>
> r~

  Yeah, but it's like about /four/ extra statements or so... so here it is!

  ld/ChangeLog:

        * plugin.c (asymbol_from_plugin_symbol): If the bfd is an ELF bfd,
        find the elf symbol data and set the visibility in the st_other field.

  ld/testsuite/ChangeLog:

        * plugin-ignore.d: New dump test control script.
        * plugin-vis-1.d: Likewise.
        * plugin.exp: Add list of ELF-only tests and run them if testing on
        an ELF target.

    cheers,
      DaveK


ld-plugin-api-5-elf-vis.diff (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add plugin interface to LD [4/4] Add files, libs and dir paths.

Dave Korn-9
In reply to this post by Dave Korn-9
On 23/09/2010 06:32, Dave Korn wrote:

  Spot of self-review here:

> --- a/ld/ldmain.c
> +++ b/ld/ldmain.c
> @@ -889,7 +889,7 @@ add_archive_element (struct bfd_link_info *info,
>     multiple times.  */
>  
>  static bfd_boolean
> -multiple_definition (struct bfd_link_info *info ATTRIBUTE_UNUSED,
> +multiple_definition (struct bfd_link_info *info,
>       const char *name,
>       bfd *obfd,
>       asection *osec,

  Don't do that, because it's only used when ENABLE_PLUGINS.  Will remember
not to commit with this change.

    cheers,
      DaveK

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add plugin interface to LD [5/4] Support ELF symbol visibility

Dave Korn-9
In reply to this post by Dave Korn-9
On 24/09/2010 01:53, Dave Korn wrote:

> Subject: Re: [PATCH] Add plugin interface to LD [0/4] Support ELF symbol
> visibility

  Oops.  That was, of course, patch five.  Subject line updated.

    cheers,
      DaveK




Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.

Dave Korn-9
In reply to this post by Dave Korn-9
On 23/09/2010 06:31, Dave Korn wrote:

  More self-review:

> @@ -290,6 +301,59 @@ ldfile_try_open_bfd (const char *attempt,
>   }
>      }
>  
> +#ifdef ENABLE_PLUGINS
> +  /* If plugins are active, they get first chance to claim
> +     any successfully-opened input file.  We skip archives
> +     here; the plugin wants us to offer it the individual
> +     members when we enumerate them, not the whole file.  We
> +     also ignore corefiles, because that's just weird.  It is
> +     a needed side-effect of calling  bfd_check_format with
> +     bfd_object that it sets the bfd's arch and mach, which
> +     will be needed when and if we want to bfd_create a new
> +     one using this one as a template.  */
> +  int fildes = bfd_check_format (entry->the_bfd, bfd_object)
> + ? open (attempt, O_RDONLY


  Oops.  C99-style definition mingling amongst code; will have to refactor it
before commit.

    cheers,
      DaveK



Reply | Threaded
Open this post in threaded view
|

[PATCH] Add plugin interface to LD [6/4] Add archive support to plugin interface.

Dave Korn-9
In reply to this post by Dave Korn-9

    Hello everyone,


  And here is the final part of this patch series (for now, anyway - bugs will
probably need fixing later, but I won't discover them until I start playing
with the LTO plugin).  It allows plugins to deal with library archives, by
offering each of the individual archive members chosen for adding into the
link to the plugins via the API "claim files" hook.

  This one isn't as neat as the other patches.  I couldn't see any way to use
the existing functionality of the linker "add_archive_element" callback to
supply an alternative BFD with its own set of symbols from the IR, so I had to
extend BFD to make this possible.  And I didn't want to change the interface
of add_archive_element, since it's one of the few actual public interfaces of
libbfd, and we don't have any versioning.

  So that led me to take a somewhat ugly solution: I added a new member to
struct areltdata, and use that to communicate between libbfd and the linker
callback.  The member gets zeroed immediately before calling the callback, and
if on return it has been set to point to a BFD, the symbols from that BFD are
added to the linker hash table instead of the archive element's own symbols.
It's not ideal, but I think it's OK enough; there are only so many places from
which BFD calls the hook.  If anyone has a better idea how to implement this,
I'm all ears.  Otherwise, OK?


  bfd/ChangeLog:

        * aoutx.h (aout_link_check_ar_symbols): Clear arelt_substitute_bfd
        before calling add_archive_element callback.
        (aout_link_check_archive_element): Handle arelt_substitute_bfd if it
        was set during add_archive_element callback.
        * cofflink.c (coff_link_check_ar_symbols): Clear arelt_substitute_bfd
        before calling add_archive_element callback.
        (coff_link_check_archive_element): Handle arelt_substitute_bfd if it
        was set during add_archive_element callback.
        * ecoff.c (read_ext_syms_and_strs): New function holds symbol-reading
        code factored-out from ecoff_link_check_archive_element.
        (reread_ext_syms_and_strs): Clear old symbols and call it.
        (ecoff_link_check_archive_element): Clear arelt_substitute_bfd before
        calling add_archive_element callback and handle the substitution if
        the callback sets it.
        (ecoff_link_add_archive_symbols): Likewise.
        * elflink.c (elf_link_add_archive_symbols): Likewise.
        * libbfd-in.h (struct areltdata): Add new substitute_bfd member.
        (arelt_substitute_bfd): Add new accessor macro for it.
        * libbfd.h: Regenerate.
        * linker.c (generic_link_check_archive_element): Clear
        arelt_substitute_bfd before calling add_archive_element callback and
        handle the substitution if the callback sets it.
        * pdp11.c (aout_link_check_ar_symbols): Clear arelt_substitute_bfd
        before calling add_archive_element callback.
        (aout_link_check_archive_element): Handle arelt_substitute_bfd if it
        was set during add_archive_element callback.
        * vms-alpha.c (alpha_vms_link_add_archive_symbols): Clear
        arelt_substitute_bfd before calling add_archive_element callback and
        handle the substitution if the callback sets it.
        * xcofflink.c (xcoff_link_check_dynamic_ar_symbols): Clear
        arelt_substitute_bfd before calling add_archive_element callback.
        (xcoff_link_check_ar_symbols): Likewise.
        (xcoff_link_check_archive_element): Handle arelt_substitute_bfd if it
        was set during add_archive_element callback.

  ld/ChangeLog:

        * ldlang.c (load_symbols): Clear arelt_substitute_bfd before calling
        add_archive_element callback and handle the substitution if the
        callback sets it.
        * ldmain.c (add_archive_element)[ENABLE_PLUGINS]: Offer the archive
        member to the plugins and if claimed set arelt_substitute_bfd to point
        to the dummy IR-only BFD.

  ld/testsuite/ChangeLog:

        * ld-plugin/plugin-10.d: New dump test control script.
        * ld-plugin/plugin-11.d: Likewise.
        * ld-plugin/plugin.exp: Run them.

  include/ChangeLog:

        * include/bfdlink.h (struct_bfd_link_callbacks): Document new
        facility for add_archive_element callback to indicate a replacement
        bfd to be added to the hash table in place of the original element.


(As before, tested on i686-pc-cygwin and i686-pc-linux-gnu.)


    cheers,
      DaveK


ld-plugin-api-6-libs.diff (31K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.

Dave Korn-9
In reply to this post by Ralf Wildenhues
[ Sorry to return to this issue, but I still haven't fixed it yet. ]

On 23/09/2010 07:13, Ralf Wildenhues wrote:

> * Dave Korn wrote on Thu, Sep 23, 2010 at 08:25:42AM CEST:
>> On 23/09/2010 06:36, Ralf Wildenhues wrote:
>>> * Dave Korn wrote on Thu, Sep 23, 2010 at 07:31:41AM CEST:
>>>> --- a/ld/Makefile.am
>>>> +++ b/ld/Makefile.am
>>>> @@ -1989,7 +1989,7 @@ if ENABLE_PLUGINS
>>>>  noinst_LTLIBRARIES = libldtestplug.la
>>>>  libldtestplug_la_SOURCES = testplug.c
>>>>  libldtestplug_la_CFLAGS= -g -O2
>>>> -libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere
>>>> +libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere -Wl,$(LIBIBERTY)
>>> Why the '-Wl,' prefix?  I'd have expected $(LIBIBERTY) to be added to
>>> libldtestplug_la_LIBADD.
>>   libiberty isn't built under libtool control at all, AFAIK, and is only ever
>> built as a static archive, so I was under the impression I've got to pass it
>> around behind libtool's back like this.
>
> Not because of that, no.
>
>>> If you need to work around libtool warning about adding static library
>>> deps to a shared library, then I suggest at least -Wc, rather than -Wl,
>>> but still I think you will run into troubles on static-only systems.
>>   What kind of problems?
>
> Ordering.  You can't rely on libtool expanding -Wl, link flags after the
> objects, meaning that libiberty may not be picked up at all.  (With
> static linking, objects and libraries are evaluated in a strict command
> line order, so all needed libraries must come after objects that need
> them.)

  Seems I'm not the only one who has this problem.  The makefile for GCC's
lto-plugin links libiberty like so:

> liblto_plugin_la_LIBADD = $(LIBELFLIBS) ../libiberty/pic/libiberty.a




Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add plugin interface to LD [2/4] Claim files and add symbols.

Dave Korn-9
[ groan, fat-fingered the send key prematurely... ]

[ Sorry to return to this issue, but I still haven't fixed it yet. ]

On 23/09/2010 07:13, Ralf Wildenhues wrote:

> * Dave Korn wrote on Thu, Sep 23, 2010 at 08:25:42AM CEST:
>> On 23/09/2010 06:36, Ralf Wildenhues wrote:
>>> * Dave Korn wrote on Thu, Sep 23, 2010 at 07:31:41AM CEST:
>>>> --- a/ld/Makefile.am
>>>> +++ b/ld/Makefile.am
>>>> @@ -1989,7 +1989,7 @@ if ENABLE_PLUGINS
>>>>  noinst_LTLIBRARIES = libldtestplug.la
>>>>  libldtestplug_la_SOURCES = testplug.c
>>>>  libldtestplug_la_CFLAGS= -g -O2
>>>> -libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere
>>>> +libldtestplug_la_LDFLAGS = -no-undefined -rpath /nowhere -Wl,$(LIBIBERTY)
>>> Why the '-Wl,' prefix?  I'd have expected $(LIBIBERTY) to be added to
>>> libldtestplug_la_LIBADD.
>>   libiberty isn't built under libtool control at all, AFAIK, and is only ever
>> built as a static archive, so I was under the impression I've got to pass it
>> around behind libtool's back like this.
>
> Not because of that, no.
>
>>> If you need to work around libtool warning about adding static library
>>> deps to a shared library, then I suggest at least -Wc, rather than -Wl,
>>> but still I think you will run into troubles on static-only systems.
>>   What kind of problems?
>
> Ordering.  You can't rely on libtool expanding -Wl, link flags after the
> objects, meaning that libiberty may not be picked up at all.  (With
> static linking, objects and libraries are evaluated in a strict command
> line order, so all needed libraries must come after objects that need
> them.)

  Seems I'm not the only one who has this problem.  The makefile for GCC's
lto-plugin links libiberty like so:

> liblto_plugin_la_LIBADD = $(LIBELFLIBS) ../libiberty/pic/libiberty.a

which produces this warning on Linux:

> /bin/sh ./libtool --tag=CC   --mode=link /gnu/gcc/obj.clean/./prev-gcc/xgcc -B/gnu/gcc/obj.clean/./prev-gcc/ -B/opt/gold/i686-pc-linux-gnu/bin/ -B/opt/gold/i686-pc-linux-gnu/bin/ -B/opt/gold/i686-pc-linux-gnu/lib/ -isystem /opt/gold/i686-pc-linux-gnu/include -isystem /opt/gold/i686-pc-linux-gnu/sys-include    -Wall -Werror -g -O2 -fomit-frame-pointer -gtoggle   -o liblto_plugin.la -rpath /opt/gold/libexec/gcc/i686-pc-linux-gnu/4.6.0 lto-plugin.lo -lelf ../libiberty/pic/libiberty.a
>
> *** Warning: Linking the shared library liblto_plugin.la against the
> *** static library ../libiberty/pic/libiberty.a is not portable!


  When I try using LIBADD to link a static archive into a shared library on
Cygwin, such as like so:

> libldtestplug_la_LIBADD = $(LIBIBERTY)

... that warning becomes an error:

> *** Warning: Trying to link with static lib archive ../libiberty/libiberty.a.
> *** I have the capability to make that library automatically link in when
> *** you link to this library.  But I can only do this if you have a
> *** shared version of the library, which you do not appear to have
> *** because the file extensions .a of this argument makes me believe
> *** that it is just a static archive that I should not use here.

... and the link fails with unresolved symbols:

> Creating library file: .libs/libldtestplug.dll.a.libs/libldtestplug_la-testplug.
> o: In function `record_claim_file':
> /gnu/binutils/git.repo/binutils/ld/testplug.c:148: undefined reference to `_xmalloc'
> .libs/libldtestplug_la-testplug.o: In function `record_claimed_file_symbol':
> /gnu/binutils/git.repo/binutils/ld/testplug.c:261: undefined reference to `_xrealloc'
> .libs/libldtestplug_la-testplug.o: In function `record_add_file':
> /gnu/binutils/git.repo/binutils/ld/testplug.c:166: undefined reference to `_xmalloc'
> /gnu/binutils/git.repo/binutils/ld/testplug.c:166: undefined reference to `_xmalloc'
> /gnu/binutils/git.repo/binutils/ld/testplug.c:166: undefined reference to `_xmalloc'
> collect2: ld returned 1 exit status
>
> make[2]: *** [libldtestplug.la] Error 1

  So if -Wl won't work, what can I do?  Do I have to build a libtool
convenience library out of the libiberty object files?

    cheers,
      DaveK

12