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

[Diego Novillo]

On Mon, Feb 23, 2009 at 22:04, Le-Chun Wu <lcwu@google.com> wrote:

> 2009-02-23  Le-Chun Wu  <lcwu@google.com>
>
>        * configure.ac: Add --enalbe-plugin support. Set pluginlibs only for
>        ELF targets.
>        * configure: Regenerate.
>        * Makefile.tpl: Define and export PLUGINLIBS.
>        * Makefile.in: Regenerate.
>        * gcc/configure.ac: Add PLUGINLIBS variable.
>        * gcc/configure: Regenerate.
>        * gcc/Makefile.in: Add PLUGINLIBS to link command. Add/modify dependencies
>        for plugin.o and files including plugin.h.
>        * gcc/tree-pass.h (register_one_dump_file): Add a prototype for
>        register_one_dump_file.
>        * gcc/toplev.c (compile_file): Call initialize_plugins.
>        (do_compile): Call invoke_plugin_callbacks.
>        (toplev_main): Call invoke_plugin_callbacks.
>        * gcc/cp/decl.c (finish_function): Call invoke_plugin_callbacks.
>        * gcc/cp/parser.c (cp_parser_type_specifier): Call invoke_plugin_callbacks.
>        * gcc/cp/Make-lang.in: Modify dependencies of files including plugin.h.
>        * gcc/opts.c (common_handle_option): Handle OPT_fplugin_ and
>        OPT_fplugin_arg_.
>        * gcc/common.opt: Add -fplugin= and -fplugin-arg-.
>        * gcc/gcc-plugin.h: New public header file for plugins to include.
>        * gcc/plugin.c: New source file.
>        * gcc/plugin.h: New internal header file.
>        * gcc/passes.c (register_one_dump_file): Make it external.
>        * gcc/c-parser.c (c_parser_declspecs): Call invoke_plugin_callbacks.
>

Thanks for doing this.  The patch looks fine in general and it's
OK to commit to the plugins branch with the revisions below.

There are some items that could be addressed in subsequent
patches:

- Need to have pre/post checks at the point of plugin insertion.

- Should we have configurable plugin insertion points?  I think
  that's too error prone, though.

- The API should be documented in the internals manual.  I
  suggest adding a new file doc/plugin.texi linked from
  gccint.texi.

> ===================================================================
> --- gcc/toplev.c	(revision 144307)
> +++ gcc/toplev.c	(working copy)
> @@ -84,6 +84,7 @@ along with GCC; see the file COPYING3.
>  #include "tree-mudflap.h"
>  #include "tree-pass.h"
>  #include "gimple.h"
> +#include "plugin.h"

Need to add plugin.h to the list of dependencies for toplev.o.

> +enum plugin_event
> +{
> +  PLUGIN_PASS_MANAGER_SETUP,    /* To hook into pass manager.  */
> +  PLUGIN_FINISH_STRUCT,

Needs comment.

> @@ -854,6 +854,14 @@ fpie
>  Common Report Var(flag_pie,1) VarExists
>  Generate position-independent code for executables if possible (small mode)
>
> +fplugin=
> +Common Joined RejectNegative
> +Specify a plugin to load
> +
> +fplugin-arg-
> +Common Joined RejectNegative
> +-fplugin-arg-<name>-<key>[=<value>] Specify argument <key>=<value> for plugin <name>

I like this better than the original -fNAME.  The wiki document
will need to be updated.

> +
> +/* Given a plugin's full-path name, e.g. /pass/to/NAME.so, return NAME.  */

s/NAME/FULL_NAME/

> +  base_name = XNEWVEC (char, name_len + 1);
> +  strncpy (base_name, name_start, name_len);
> +  base_name[name_len] = '\0';

Better call lbasename() and then just strip the .so suffix.

> +  if (*slot)
> +    {
> +      plugin = (struct plugin_name_args *) *slot;
> +      if (strcmp (plugin->full_name, plugin_name))
> +        error (G_("Plugin %s was specified with different paths:\n%s\n%s"),
> +               plugin->base_name, plugin->full_name, plugin_name);
> +      else
> +        warning (0, G_("Plugin %s was specified multiple times"), plugin_name);

Do we really need to warn in this case?  We generally don't warn
when the same option is given more than once.

> +  if (!key_start)
> +    {
> +      error (G_("Misformed option -fplugin-arg-%s (missing -<key>[=<value>])"),

s/Misformed/Malformed/

> +          /* Insert the new pass instance based on the positioning op.  */
> +          switch (plugin_pass_info->pos_op)
> +            {
> +              case PASS_POS_INSERT_AFTER:
> +                new_pass->next = pass->next;
> +                pass->next = new_pass;
> +                break;
> +              case PASS_POS_INSERT_BEFORE:
> +                new_pass->next = pass;
> +                if (prev_pass)
> +                  prev_pass->next = new_pass;
> +                else
> +                  *pass_list = new_pass;
> +                break;
> +              case PASS_POS_REPLACE:
> +                new_pass->next = pass->next;
> +                if (prev_pass)
> +                  prev_pass->next = new_pass;
> +                else
> +                  *pass_list = new_pass;
> +                new_pass->sub = pass->sub;
> +                new_pass->tv_id = pass->tv_id;
> +                pass = new_pass;
> +                break;
> +              default:
> +                error (G_("Invalid pass positioning operation"));
> +                return false;
> +            }

We should also check that the new pass is of the appropriate
type.  It should match that type of all the passes in PASS_LIST.

> +#define PTR_UNION_TYPE(TOTYPE) union { void *_q; TOTYPE _nq; }
> +#define PTR_UNION_AS_VOID_PTR(NAME) (NAME._q)
> +#define PTR_UNION_AS_CAST_PTR(NAME) (NAME._nq)
> +
> +/* Routine to dlopen and initialize one plugin. This function is passed to
> +   (and called by) the hash table traverse routine.  */

Arguments and return value need documenting.

> +
> +static int
> +init_one_plugin (void **slot, void * ARG_UNUSED (info))
> +{
> +  struct plugin_name_args *plugin = (struct plugin_name_args *) *slot;
> +  void *dl_handle;
> +  plugin_init_func plugin_init;
> +  char *err;
> +  PTR_UNION_TYPE (plugin_init_func) plugin_init_union;
> +
> +  dl_handle = dlopen (plugin->full_name, RTLD_NOW);
> +  if (!dl_handle)
> +    {
> +      error (G_("Cannot load plugin %s\n%s"), plugin->full_name, dlerror());

Space before '(' in call to dlerror.

> +      return 1;
> +    }
> +
> +  /* Clear any existing error.  */
> +  dlerror();

Likewise.

> +
> +  PTR_UNION_AS_VOID_PTR (plugin_init_union) =
> +      dlsym (dl_handle, str_plugin_init_func_name);
> +  plugin_init = PTR_UNION_AS_CAST_PTR(plugin_init_union);

Space before '('.

> +
> +  if ((err = dlerror()) != NULL)

Likewise.

> +  /* Call the plugin-provided initialization routine with the arguments.  */
> +  if ((*plugin_init) (plugin->base_name, plugin->argc, plugin->argv))
> +    {
> +      error (G_("Fail to initialize plugin %s"), plugin->full_name);
> +      return 1;
> +    }

We should also check that the plugin was compiled for the same
version of GCC.  If there is a mismatch, emit a warning or an
error.  We could decided whether to warn or error based on some
command line flag or internal mechanism (like having a table of
compatible versions for plugin ABIs), but that can be done later.


Diego.