Re: plugin directory [ping^3]

[Basile STARYNKEVITCH <basile at starynkevitch dot net>]

Basile STARYNKEVITCH wrote:
> Hello All
>
> I am pinging again the patch  
> http://gcc.gnu.org/ml/gcc-patches/2009-12/msg00253.html which is a 
> minuscule improvement (comments & typos) over 
> http://gcc.gnu.org/ml/gcc-patches/2009-12/msg00069.html
>
> The http://gcc.gnu.org/ml/gcc-patches/2009-12/msg00253.html patch 
> provide the following stuff:

I am attaching a slightly improved patch to trunk rev155790. The only improvement (w.r.t previous submission) is a 
comment saying that the wired-in ".so" suffix should be fixed when non-ELF plugins becomes possible.


One can pass -fplugin=treehydra instead of -fplugin=/some/distribution/dependent/path/to/treehydra.so since there is a 
default location for commonly used plugins. Yes, this defines an additional policy (but GCC already have policies for 
location of crt0.o, or system includes like limits.h files). The main reason for this patch (which does not break 
existing plugin behavior) is to avoid the nightmare situation of commonly available plugins to be invoked in a different 
way on e.g. Debian & Redhat. I believe trying to help users to be able to easily (that is, in the same way) invoke 
common existing plugins is important (and is a minor change, stage 3 or 4 compatible - since no previous release of GCC 
had plugins.).

> The gcc driver is slightly enhanced to pass -iplugindir <plugindirname> 
> to cc1 (etc..) when having -fplugin
>
> The common.opt has a new (internal) -iplugindir option.
>
> The gcc-plugin.h has a new plugin_directory_name() public function 
> trivially implemented in plugin.c


While some (e.g. Rafael) apparently believe that defining a tiny policy for common plugins file location might not be 
desirable, I think that permitting a small patch which define a minuscule policy for the new feature of plugins is good. 
Apparently Davek also believes that would help the user (of course, as of today, no commonly used plugins exists, 
because no plugins are distributed, because no GCC release supports plugins yet, except the soon to be released 4.5.0. 
This is a typical chicken & egg situation.). Since I've some positive opinion and some negative one, I am resubmitting 
my patch.

If you feel I should not try hard to help future plugins newbie users and assume that a set of commonly used plugins 
will appear, please tell. But defining such a very minor policy is trivial today (and my patch also permits -fplugin=foo 
in addition of -fplugin=/some/path/to/foo.so thus making GCC command invocation a bit shorter so does not break any 
"existing" plugin.). It would be quite hard when each (e.g. Linux) distribution will implement a slightly incompatible 
one (and such a state would be a nuisance to users; their compilation flags & Makefile will be even more 
distribution-dependent).

BTW, we should perhaps document that (in addition of other few shell variables) the LD_LIBRARY_PATH environment variable 
now impacts the behavior of GCC, since GCC will use dlopen whose behavior is affected by LD_LIBRARY_PATH etc. And 
perhaps without this patch some people might be tempted to set LD_LIBRARY_PATH to be able to pass -fplugin=foo.so 
instead of some much longer program argument (I would recommend against such a practice, and I am proposing this patch 
to make that practice less useful).


I am attaching the patch file (to trunk rev rev155790.) plugindir-rev155790.diff and the gcc/ChangeLog entry.

Ok for trunk (ie future GCC 4.5) if it bootstraps [it did pass stage1 of the bootstrap]? Other comments are welcome.

Regards.

PS. Of course, such a patch would help MELT, by permitting shorter program arguments and by suggesting a policy for 
installation of MELT specific files.

--
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mines, sont seulement les miennes} ***

Index: gcc/doc/plugins.texi
===================================================================
--- gcc/doc/plugins.texi	(revision 155790)
+++ gcc/doc/plugins.texi	(working copy)
@@ -22,7 +22,14 @@ The plugin arguments are parsed by GCC and passed
 plugins as key-value pairs. Multiple plugins can be invoked by
 specifying multiple @option{-fplugin} arguments.
 
+A plugin can be simply given by its short name (no dots or
+slashes). When simply passing @option{-fplugin=NAME}, the plugin is
+loaded from @file{libexec/} inside the @file{plugin} directory, so
+@option{-fplugin=NAME} is the same as @option{-fplugin=`gcc
+-print-file-name=plugin`/libexec/NAME.so}, using backquote shell
+syntax to query the @file{plugin} directory.
 
+
 @section Plugin API
 
 Plugins are activated by the compiler at specific events as defined in
@@ -407,6 +414,9 @@ On most systems, you can query this @code{plugin}
 invoking @command{gcc -print-file-name=plugin} (replace if needed
 @command{gcc} with the appropriate program path).
 
+Inside plugins, this @code{plugin} directory name can be queried by
+calling @code{plugin_directory_name ()}.
+
 The following GNU Makefile excerpt shows how to build a simple plugin:
 
 @smallexample
Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c	(revision 155790)
+++ gcc/gcc.c	(working copy)
@@ -403,6 +403,7 @@ static const char *if_exists_else_spec_function (i
 static const char *replace_outfile_spec_function (int, const char **);
 static const char *version_compare_spec_function (int, const char **);
 static const char *include_spec_function (int, const char **);
+static const char *find_file_spec_function (int, const char **);
 static const char *print_asm_header_spec_function (int, const char **);
 static const char *compare_debug_dump_opt_spec_function (int, const char **);
 static const char *compare_debug_self_opt_spec_function (int, const char **);
@@ -905,6 +906,7 @@ static const char *cc1_options =
  %{!fsyntax-only:%{S:%W{o*}%{!o*:-o %b.s}}}\
  %{fsyntax-only:-o %j} %{-param*}\
  %{fmudflap|fmudflapth:-fno-builtin -fno-merge-constants}\
+ %{fplugin*:-iplugindir %:find-file(plugin) -fplugin%*}\
  %{coverage:-fprofile-arcs -ftest-coverage}";
 
 static const char *asm_options =
@@ -1726,6 +1728,7 @@ static const struct spec_function static_spec_func
   { "replace-outfile",		replace_outfile_spec_function },
   { "version-compare",		version_compare_spec_function },
   { "include",			include_spec_function },
+  { "find-file",		find_file_spec_function },
   { "print-asm-header",		print_asm_header_spec_function },
   { "compare-debug-dump-opt",	compare_debug_dump_opt_spec_function },
   { "compare-debug-self-opt",	compare_debug_self_opt_spec_function },
@@ -8779,6 +8782,22 @@ include_spec_function (int argc, const char **argv
   return NULL;
 }
 
+/* %:find-file spec function.  This function replace its argument by
+    the file found thru find_file, that is the -print-file-name gcc
+    program option. */
+static const char *
+find_file_spec_function (int argc, const char**argv)
+{
+  const char *file;
+
+  if (argc != 1)
+    abort ();
+
+  file = find_file (argv[0]);
+  return file;
+}
+
+
 /* %:print-asm-header spec function.  Print a banner to say that the
    following output is from the assembler.  */
 
Index: gcc/testsuite/gcc.dg/plugin/plugdir-test-1.c
===================================================================
--- gcc/testsuite/gcc.dg/plugin/plugdir-test-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/plugin/plugdir-test-1.c	(revision 0)
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-O" } */
+
+int main (int argc, char **argv) 
+{ 
+  return 0;  
+}
Index: gcc/testsuite/gcc.dg/plugin/plugdir_plugin.c
===================================================================
--- gcc/testsuite/gcc.dg/plugin/plugdir_plugin.c	(revision 0)
+++ gcc/testsuite/gcc.dg/plugin/plugdir_plugin.c	(revision 0)
@@ -0,0 +1,42 @@
+/* This test plugin just displays the plugin directory. */
+#include "gcc-plugin.h"
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tm.h"
+#include "toplev.h"
+#include "basic-block.h"
+#include "gimple.h"
+#include "tree.h"
+#include "tree-pass.h"
+#include "intl.h"
+#include "plugin-version.h"
+
+int plugin_is_GPL_compatible;
+
+
+static void 
+start_callback (void *gcc_data, void *user_data)
+{
+  inform (UNKNOWN_LOCATION, "plugin directory is %s", 
+	  plugin_directory_name ());
+}
+
+/* The initialization routine exposed to and called by GCC. The spec of this
+   function is defined in gcc/gcc-plugin.h.
+
+   PLUGIN_NAME - name of the plugin (useful for error reporting)
+   ARGC        - the size of the ARGV array
+   ARGV        - an array of key-value argument pair
+
+   Returns 0 if initialization finishes successfully.
+
+   Note that this function needs to be named exactly "plugin_init".  */
+
+int
+plugin_init (struct plugin_name_args *plugin_info,
+             struct plugin_gcc_version *version)
+{
+  register_callback ("plugdir", PLUGIN_START_UNIT, &start_callback, NULL);
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/plugin/plugin.exp
===================================================================
--- gcc/testsuite/gcc.dg/plugin/plugin.exp	(revision 155790)
+++ gcc/testsuite/gcc.dg/plugin/plugin.exp	(working copy)
@@ -51,6 +51,7 @@ set plugin_test_list [list \
     { ggcplug.c ggcplug-test-1.c } \
     { one_time_plugin.c one_time-test-1.c } \
     { start_unit_plugin.c start_unit-test-1.c } \
+    { plugdir_plugin.c plugdir-test-1.c } \
     { finish_unit_plugin.c finish_unit-test-1.c } \
 ]
 
Index: gcc/gcc-plugin.h
===================================================================
--- gcc/gcc-plugin.h	(revision 155790)
+++ gcc/gcc-plugin.h	(working copy)
@@ -135,4 +135,10 @@ extern void register_callback (const char *plugin_
 
 extern int unregister_callback (const char *plugin_name, int event);
 
+
+/* Retrieve the plugin directory name, as returned by the
+   -fprint-file-name=plugin argument to the gcc program, which is the
+   -iplugindir program argument to cc1.  */
+extern const char* plugin_directory_name (void);
+
 #endif /* GCC_PLUGIN_H */
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 155790)
+++ gcc/common.opt	(working copy)
@@ -1529,6 +1529,11 @@ gxcoff+
 Common JoinedOrMissing Negative(gcoff)
 Generate debug information in extended XCOFF format
 
+
+iplugindir
+Common Joined Separate Var(plugindir_string)
+-iplugindir <dir>	Set <dir> to be the plugin directory
+
 o
 Common Joined Separate
 -o <file>	Place output into <file>
Index: gcc/plugin.c
===================================================================
--- gcc/plugin.c	(revision 155790)
+++ gcc/plugin.c	(working copy)
@@ -1,5 +1,5 @@
 /* Support for GCC plugin mechanism.
-   Copyright (C) 2009 Free Software Foundation, Inc.
+   Copyright (C) 2009, 2010 Free Software Foundation, Inc.
 
 This file is part of GCC.
 
@@ -124,17 +124,40 @@ get_plugin_base_name (const char *full_name)
 }
 
 
-/* Create a plugin_name_args object for the give plugin and insert it to
-   the hash table. This function is called when -fplugin=/path/to/NAME.so
-   option is processed.  */
+/* Create a plugin_name_args object for the give plugin and insert it
+   to the hash table. This function is called when
+   -fplugin=/path/to/NAME.so or -fplugin=NAME option is processed.  */
 
 void
 add_new_plugin (const char* plugin_name)
 {
   struct plugin_name_args *plugin;
   void **slot;
-  char *base_name = get_plugin_base_name (plugin_name);
+  char *base_name;
+  bool name_is_short = true;
+  const char *pc;
 
+  /* Replace short names by their full path when relevant.  */
+  for (pc = plugin_name; *pc && name_is_short; pc++)
+    if (*pc == '.' || *pc == '/')
+      name_is_short = false;
+
+  if (name_is_short)
+    {
+      base_name = CONST_CAST (char*, plugin_name);
+      /* FIXME: the ".so" suffix is currently builtin.  When plugins
+	 shall be available on non ELF systems, this should be
+	 improved.  */
+      plugin_name = concat (plugin_directory_name (), "/libexec/",
+			    plugin_name, ".so", NULL);
+      if (access (plugin_name, R_OK))
+	fatal_error
+	  ("inacessible file %s expanded from plugin short name %s: %m",
+	   plugin_name, base_name);
+    }
+  else
+    base_name = get_plugin_base_name (plugin_name);
+
   /* If this is the first -fplugin= option we encounter, create
      'plugin_name_args_tab' hash table.  */
   if (!plugin_name_args_tab)
@@ -809,6 +832,7 @@ plugin_default_version_check (struct plugin_gcc_ve
   return true;
 }
 
+
 /* Return the current value of event_last, so that plugins which provide
    additional functionality for events for the benefit of high-level plugins
    know how many valid entries plugin_event_name holds.  */
@@ -818,3 +842,12 @@ get_event_last (void)
 {
   return event_last;
 }
+
+/* Retrieve the plugin directory.  The gcc driver should have passed it
+   as -iplugindir <dir> to the cc1 program, and it is queriable thru the
+   -print-file-name=plugin option to gcc.  */
+const char*
+plugin_directory_name (void)
+{
+  return plugindir_string;
+}

2010-01-10  Basile Starynkevitch  <basile@starynkevitch.net>

	* doc/plugins.texi (Loading Plugins): Added short invocation
	-fplugin=NAME. (Building GCC plugins): Mention
	plugin_directory_name function.

	* gcc.c (find_file_spec_function): added declaration & function.
	(cc1_options): added -fplugin* stuff.
	(static_spec_func): added "find-file" entry.

	* gcc-plugin.h (plugin_directory_name): added declaration.

	* common.opt (iplugindir): added new internal option.

	* plugin.c: updated copyright year.
	(add_new_plugin): handle short form -fplugin=NAME.
	(plugin_directory_name): added new public function.

	* testsuite/gcc.dg/plugin/plugdir-test-1.c: added file.
	* testsuite/gcc.dg/plugin/plugdir_plugin.c: added file.
	* testsuite/gcc.dg/plugin/plugin.exp: added plugdir_* test.