This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [plugins] [patch] Initial implementation of GCC plugin support


On Fri, Feb 20, 2009 at 2:53 PM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
>
>>         * c.opt: Add -fplugin= and -fplugin-arg-.
>>         * c-opts.c (c_common_handle_option): Handle OPT_fplugin_ and
>>         OPT_fplugin_arg_.
>
> Plugin options should not be specific to C-family languages; this should
> be done in language-independent files instead.

Done as suggested.

>
>>         * Makefile.in: Add libs needed for plugin support. Also add plugin.o.
>
> You'll need to allow plugins to be disabled at configure time and make the
> configure scripts control the libraries / link options used for plugin
> support.  (This first patch is for ELF hosts, which is fine, but you need
> to make sure plugins get disabled by default for non-ELF hosts until
> someone contributes support for them to avoid breaking such hosts.)
>

Done as suggested.


On Fri, Feb 20, 2009 at 6:45 PM, Basile STARYNKEVITCH
<basile@starynkevitch.net> wrote:
> I would suggest using RTLD_NOW instead of RTLD_LAZY when calling dlopen.
> If the dynamic load is lazy, undefined references are caught later (and
> crash the compiler). This is really annoying for GCC plugins. We don't
> dlopen often (only at start up time), and we would prefer a strict load
> (even if it is a bit slower) which catch more errors.

Sounds reasonable. Changed  RTLD_LAZY to RTLD_NOW.

>
> One could also imagine that GCC plugins could define a
>  const char gcc_version_expected_by_plugin[]="4.4.0";
> and some additional code just after the dlopen to warn against plugin
> incompatibility. This could be implemented later (but I definitely believe
> it is very useful).

There are still lively discussions on this topic, so I will leave it
for later patches.

>
> In gcc/tree-pass.h
> +extern void register_one_dump_file (struct opt_pass *);
> I would suggest adding a small comment before.

Done as suggested.


2009/2/23 Zbigniew Chamski <zbigniew.chamski@gmail.com>:
> +#define POINTER_CAST(TOTYPE, X) ((__extension__(union {void * _q;
> TOTYPE _nq;})(X))._nq)
>
> and
>
> +  plugin_init = POINTER_CAST (plugin_init_func,
> +                              dlsym (dl_handle, str_plugin_init_func_name));
>
> There's a portability issue: __extension__ won't work when
> bootstrapping GCC on a non-GCC system (some still exist.)  Since
> there's only one / a few types to cast to, II'd suggest using explicit
> union declarations.  The penalty is not big - for now, there's just
> one symbol resolved per plugin.  In addition this will give an added
> benefit of supporting value sanity checking on pointers between
> reading the void * value and assigning it to the target pointer.

Done as suggested.

I will send out a revised patch with the above-mentioned changes
later. Thanks a lot for the review comments.

Le-chun


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]