[patch] Add plugin support for bfd

Rafael Espindola espindola@google.com
Thu Apr 30 17:26:00 GMT 2009


Patch with some of the comments implemented is attached.

>>+[plugins=no])
>
> any reason to not enable plugins by default ?  i guess since it requires
> dlopen and/or -ldl, that's a good reason to disable by default.

That is it.

> which leads to the next part -- this code should be checking for dlopen
> support and erroring out if someone tries to enable plugins but they dont have
> libdl support.

Is there a canonical way to do this?

> also, if instead you updated one of the bfd in header files with like:
> #define BFD_SUPPORTS_PLUGINS @...@
> you shouldnt need to touch any other configure.in files as all the other
> projects can key off this define ...

The problem is that I need the value in the Makefiles, not just on the
C source code:

+if PLUGINS
+LIBDL = -ldl
+endif

>>+static enum ld_plugin_status
>>+message (int level ATTRIBUTE_UNUSED,
>>+       const char * format, ...)
>
> do you really need your own helper function for output ?

Yes, part of the plugin API.


>>+  plugin_dir = concat (BINDIR, "/../lib/bfd-plugins", NULL);
>
> assuming/requiring the program being executed (ld in this case) is relative to
> its library directory smells completely broken to me

It is looking for its plugins, not a generic library. That was
suggested by Joseph S. Myers. What would you suggest instead?

>>+++ b/binutils/Makefile.am
>>+if PLUGINS
>>+LIBDL = -ldl
>>+endif
>
> this looks broken/backwards to me, as do the rest of the LDADD changes in the
> Makefile.am files.  these programs arent using libdl, so why do they need to
> link against it  binutils uses libtool for library creation, so the -ldl
> should be in libbfd.la so that everyone else will get it automatically and you
> dont need this stuff.  it will also let you drop the changes for
> gprof/ld/gas/etc... that dont actually invoke any plugin code directly.

I remember having a link error, but I don't remember the details. Will
try without it and see what breaks.

>>+++ b/binutils/ar.c
>>+      bfd_plugin_set_plugin (argv[2]);
>
> there is no conditional compile here, so ar will fail to link if plugin
> support is disabled ?  same for nm.c

The function was added to bfd.c to avoid that. I can move it to
plugin.c and add conditional compilation if you prefer.

> -mike
>


Cheers,
-- 
Rafael Avila de Espindola

Google | Gordon House | Barrow Street | Dublin 4 | Ireland
Registered in Dublin, Ireland | Registration Number: 368047
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bfd-plugin.patch
Type: text/x-patch
Size: 28723 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20090430/15d798b2/attachment.bin>


More information about the Binutils mailing list