Re: patch PLUGIN_HEADER_FILE event for tracing of header inclusions.

Re: patch PLUGIN_HEADER_FILE event for tracing of header inclusions.

[Dominique Dhumieres]

> Thanks: Committed revision 205038.

This seems to break several g++ tests: see http://gcc.gnu.org/ml/gcc-testresults/2013-11/msg01482.html
On x86_64-apple-darwin13, the ICE is

cc1: error: cannot load plugin ./one_time_plugin.so
dlopen(./one_time_plugin.so, 10): image not found
/opt/gcc/work/gcc/testsuite/gcc.dg/plugin/one_time-test-1.c:1:0: internal compiler error: in invoke_plugin_callbacks_full, at plugin.c:509
 /* Test that pass is inserted and invoked once. */
 ^

/opt/gcc/work/gcc/testsuite/gcc.dg/plugin/one_time-test-1.c:1:0: internal compiler error: Abort trap: 6
gcc49: internal compiler error: Abort trap: 6 (program cc1)
Abort

TIA

Dominique

Re: patch PLUGIN_HEADER_FILE event for tracing of header inclusions.

[Basile Starynkevitch]

On Tue, 2013-11-19 at 20:33 +0100, Dominique Dhumieres wrote:
> > Thanks: Committed revision 205038.
> 
> This seems to break several g++ tests: see http://gcc.gnu.org/ml/gcc-testresults/2013-11/msg01482.html
> On x86_64-apple-darwin13, the ICE is
> 
> cc1: error: cannot load plugin ./one_time_plugin.so
> dlopen(./one_time_plugin.so, 10): image not found
> /opt/gcc/work/gcc/testsuite/gcc.dg/plugin/one_time-test-1.c:1:0: internal compiler error: in invoke_plugin_callbacks_full, at plugin.c:509
>  /* Test that pass is inserted and invoked once. */
>  ^

I don't have an Apple machine, but I cannot understand the logic (in
particular, did you make clean before recompiling and running the test).

The patch only added a new event. How can just adding a new event break
the thing?

I'm beginning more tests now, but I only have access to Gnu Linux x86-64
systems...

Or is it some MacOSX specific bug? (BTW, I thought that GCC plugins
never worked on MacOSX or on Windows)

Cheers.

-- 
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 mine, sont seulement les miennes} ***

Re: patch PLUGIN_HEADER_FILE event for tracing of header inclusions.

[Basile Starynkevitch]

On Tue, 2013-11-19 at 20:33 +0100, Dominique Dhumieres wrote:
> > Thanks: Committed revision 205038.
> 
> This seems to break several g++ tests: see http://gcc.gnu.org/ml/gcc-testresults/2013-11/msg01482.html
> On x86_64-apple-darwin13, the ICE is

I would guess that the following trivial patch should correct that:

##### patch against svn rev 205052
Index: gcc/plugin.c
===================================================================
--- gcc/plugin.c	(revision 205052)
+++ gcc/plugin.c	(working copy)
@@ -529,6 +529,7 @@ invoke_plugin_callbacks_full (int event, void *gcc
       case PLUGIN_EARLY_GIMPLE_PASSES_START:
       case PLUGIN_EARLY_GIMPLE_PASSES_END:
       case PLUGIN_NEW_PASS:
+      case PLUGIN_INCLUDE_FILE:
         {
           /* Iterate over every callback registered with this event and
              call it.  */
#####

If that patch correct the bug, I guess that I could apply it under the
"obvious bugfix" rule. Could you check that it works on your MacOSX?

Apologies for my mistake.

Thanks

-- 
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 mine, sont seulement les miennes} ***

Re: patch PLUGIN_HEADER_FILE event for tracing of header inclusions.

[Dominique Dhumieres]

Basile,

http://gcc.gnu.org/ml/gcc-testresults/2013-11/msg01482.html is for
x86_64-unknown-linux-gnu. However I cannot infer from the pointer
what is the ICE. Nevertheless I'll try your patch.

Thanks for the answer.

Dominique

Re: patch PLUGIN_HEADER_FILE event for tracing of header inclusions.

[Basile Starynkevitch]

[-- Attachment #1: Type: text/plain, Size: 1818 bytes --]

On Tue, 2013-11-19 at 20:33 +0100, Dominique Dhumieres wrote:
> > Thanks: Committed revision 205038.
> 
> This seems to break several g++ tests: see http://gcc.gnu.org/ml/gcc-testresults/2013-11/msg01482.html
> On x86_64-apple-darwin13, the ICE is
> 
> cc1: error: cannot load plugin ./one_time_plugin.so

Actually, when we add hard coded plugin events we should never forget,
like I just did, to edit invoke_plugin_callbacks_full.

Diego and all, can I add the following patch to the trunk. One is just a
comment inside plugin.def to recall that invoke_plugin_callbacks_full
should be edited, and another is the tiny fix on that function in
plugin.c

### gcc/ChangeLog entry
2013-11-19  Basile Starynkevitch  <basile@starynkevitch.net>

        * plugin.def: Add comment about register_callback and
	  invoke_plugin_callbacks_full.

        * plugin.c (register_callback, invoke_plugin_callbacks_full): 
	  Handle PLUGIN_INCLUDE_FILE event.
### 

Apologies for my mistake. Hope this will be approved quickly before 4.9
stage 3! (I will later submit a documentation patch)


BTW, I am not very happy about some events in plugin.def not being real
events.  IMHO I would prefer that PLUGIN_PASS_MANAGER_SETUP,
PLUGIN_REGISTER_GGC_ROOTS, PLUGIN_REGISTER_GGC_CACHES be removed,
because they are not real events (and they should have their own public
functions in plugin.h, e.g. add functions like
plugin_pass_manager_setup, plugin_register_ggc_roots,
plugin_register_ggc_caches in gcc/gcc-plugin.h). Unfortunately, we don't
have time for such an improvement for 4.9
 
Regards.
-- 
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 mine, sont seulement les miennes} ***


[-- Attachment #2: improve-PLUGIN_HEADER_FILE-svn-r205052.diff --]
[-- Type: text/x-patch, Size: 1336 bytes --]

Index: gcc/plugin.c
===================================================================
--- gcc/plugin.c	(revision 205052)
+++ gcc/plugin.c	(working copy)
@@ -452,6 +452,7 @@ register_callback (const char *plugin_name,
       case PLUGIN_EARLY_GIMPLE_PASSES_START:
       case PLUGIN_EARLY_GIMPLE_PASSES_END:
       case PLUGIN_NEW_PASS:
+      case PLUGIN_INCLUDE_FILE:
         {
           struct callback_info *new_callback;
           if (!callback)
@@ -529,6 +530,7 @@ invoke_plugin_callbacks_full (int event, void *gcc
       case PLUGIN_EARLY_GIMPLE_PASSES_START:
       case PLUGIN_EARLY_GIMPLE_PASSES_END:
       case PLUGIN_NEW_PASS:
+      case PLUGIN_INCLUDE_FILE:
         {
           /* Iterate over every callback registered with this event and
              call it.  */
Index: gcc/plugin.def
===================================================================
--- gcc/plugin.def	(revision 205052)
+++ gcc/plugin.def	(working copy)
@@ -97,6 +97,9 @@ DEFEVENT (PLUGIN_NEW_PASS)
    as a const char* pointer.  */
 DEFEVENT (PLUGIN_INCLUDE_FILE)
 
+/* When adding a new hard-coded plugin event, don't forget to edit in
+   file plugin.c the functions register_callback and
+   invoke_plugin_callbacks_full accordingly!  */
 
 /* After the hard-coded events above, plugins can dynamically allocate events
    at run time.

Re: patch PLUGIN_HEADER_FILE event for tracing of header inclusions.

[Dominique Dhumieres]

Basile,

The patch fixes most of the plugin failures except:

FAIL: g++.dg/plugin/selfassign.c compilation

The error is

/opt/gcc/work/gcc/testsuite/g++.dg/plugin/selfassign.c: In function 'void warn_self_assign(gimple)':
/opt/gcc/work/gcc/testsuite/g++.dg/plugin/selfassign.c:227:75: error: 'maybe_get_identifier' was not declared in this scope
       if (fdecl && (DECL_NAME (fdecl) == maybe_get_identifier ("operator=")))

Note that the test fails also on linux.

TIA

Dominique

Re: patch PLUGIN_HEADER_FILE event for tracing of header inclusions.

[Richard Biener]

On Tue, Nov 19, 2013 at 10:57 PM, Basile Starynkevitch
<basile@starynkevitch.net> wrote:
> On Tue, 2013-11-19 at 20:33 +0100, Dominique Dhumieres wrote:
>> > Thanks: Committed revision 205038.
>>
>> This seems to break several g++ tests: see http://gcc.gnu.org/ml/gcc-testresults/2013-11/msg01482.html
>> On x86_64-apple-darwin13, the ICE is
>>
>> cc1: error: cannot load plugin ./one_time_plugin.so
>
> Actually, when we add hard coded plugin events we should never forget,
> like I just did, to edit invoke_plugin_callbacks_full.
>
> Diego and all, can I add the following patch to the trunk. One is just a
> comment inside plugin.def to recall that invoke_plugin_callbacks_full
> should be edited, and another is the tiny fix on that function in
> plugin.c
>
> ### gcc/ChangeLog entry
> 2013-11-19  Basile Starynkevitch  <basile@starynkevitch.net>
>
>         * plugin.def: Add comment about register_callback and
>           invoke_plugin_callbacks_full.
>
>         * plugin.c (register_callback, invoke_plugin_callbacks_full):
>           Handle PLUGIN_INCLUDE_FILE event.
> ###
>
> Apologies for my mistake. Hope this will be approved quickly before 4.9
> stage 3! (I will later submit a documentation patch)

Ok.

Thanks,
Richard.

>
> BTW, I am not very happy about some events in plugin.def not being real
> events.  IMHO I would prefer that PLUGIN_PASS_MANAGER_SETUP,
> PLUGIN_REGISTER_GGC_ROOTS, PLUGIN_REGISTER_GGC_CACHES be removed,
> because they are not real events (and they should have their own public
> functions in plugin.h, e.g. add functions like
> plugin_pass_manager_setup, plugin_register_ggc_roots,
> plugin_register_ggc_caches in gcc/gcc-plugin.h). Unfortunately, we don't
> have time for such an improvement for 4.9
>
> Regards.
> --
> 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 mine, sont seulement les miennes} ***
>

Re: patch PLUGIN_HEADER_FILE event for tracing of header inclusions.

[Basile Starynkevitch]

On Tue, 2013-11-19 at 16:16 +0000, Joseph S. Myers wrote:
> On Tue, 19 Nov 2013, Basile Starynkevitch wrote:
> 
> > Thanks for your attention. I am attaching a slightly improved patch 
> > against trunk svn rev. 305009 (the improvements are removing the spurious 
> > diff hunk, and better comments.)
> 
> Still OK in the absence of plugin maintainer objections.

Thanks: Committed revision 205038.


-- 
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 mine, sont seulement les miennes} ***

Re: patch PLUGIN_HEADER_FILE event for tracing of header inclusions.

[Joseph S. Myers]

On Tue, 19 Nov 2013, Basile Starynkevitch wrote:

> Thanks for your attention. I am attaching a slightly improved patch 
> against trunk svn rev. 305009 (the improvements are removing the spurious 
> diff hunk, and better comments.)

Still OK in the absence of plugin maintainer objections.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: patch PLUGIN_HEADER_FILE event for tracing of header inclusions.

[Diego Novillo]

On Tue, Nov 19, 2013 at 11:16 AM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> On Tue, 19 Nov 2013, Basile Starynkevitch wrote:
>
>> Thanks for your attention. I am attaching a slightly improved patch
>> against trunk svn rev. 305009 (the improvements are removing the spurious
>> diff hunk, and better comments.)
>
> Still OK in the absence of plugin maintainer objections.

I'm OK with this patch.  Thanks, Basile.


Diego.

Re: patch PLUGIN_HEADER_FILE event for tracing of header inclusions.

[Basile Starynkevitch]

[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]

On Mon, Nov 18, 2013 at 10:50:10PM +0000, Joseph S. Myers wrote:
> On Mon, 18 Nov 2013, Basile Starynkevitch wrote:
> 
> > @@ -43,6 +44,7 @@
> >  				   TARGET_OPTF.  */
> >  #include "tm_p.h"		/* For C_COMMON_OVERRIDE_OPTIONS.  */
> >  
> > +
> >  #ifndef DOLLARS_IN_IDENTIFIERS
> >  # define DOLLARS_IN_IDENTIFIERS true
> >  #endif
> 
> This is a spurious diff hunk that should not be in this patch.
> 
> OK minus the spurious change in the absence of objections from the plugin 
> maintainers within 48 hours (or in the presence of approval from either of 
> them).

Thanks for your attention. I am attaching a slightly improved patch 
against trunk svn rev. 305009 (the improvements are removing the spurious 
diff hunk, and better comments.)

##### gcc/c-family/ChangeLog entry :
2013-11-19  Basile Starynkevitch  <basile@starynkevitch.net>

	* c-opts.c: Include plugin.h.
	(cb_file_change): Invoke plugin event PLUGIN_INCLUDE_FILE.


##### gcc/ChangeLog entry :
2013-11-19  Basile Starynkevitch  <basile@starynkevitch.net>

	* plugin.def (PLUGIN_INCLUDE_FILE): New event, invoked in 
	cb_file_change.

### 

Ok for trunk?

Regards.
-- 
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} ***

[-- Attachment #2: plugin-include-patch-trunk-r205009.diff --]
[-- Type: text/x-diff, Size: 1792 bytes --]

Index: gcc/c-family/c-opts.c
===================================================================
--- gcc/c-family/c-opts.c	(revision 205009)
+++ gcc/c-family/c-opts.c	(working copy)
@@ -34,6 +34,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "debug.h"		/* For debug_hooks.  */
 #include "opts.h"
 #include "options.h"
+#include "plugin.h"		/* For PLUGIN_INCLUDE_FILE event.  */
 #include "mkdeps.h"
 #include "c-target.h"
 #include "tm.h"			/* For BYTES_BIG_ENDIAN,
@@ -1397,6 +1398,17 @@ cb_file_change (cpp_reader * ARG_UNUSED (pfile),
   else
     fe_file_change (new_map);
 
+  if (new_map 
+      && (new_map->reason == LC_ENTER || new_map->reason == LC_RENAME))
+    {
+      /* Signal to plugins that a file is included.  This could happen
+	 several times with the same file path, e.g. because of
+	 several '#include' or '#line' directives...  */
+      invoke_plugin_callbacks 
+	(PLUGIN_INCLUDE_FILE,
+	 const_cast<char*> (ORDINARY_MAP_FILE_NAME (new_map)));
+    }
+
   if (new_map == 0 || (new_map->reason == LC_LEAVE && MAIN_FILE_P (new_map)))
     {
       pch_cpp_save_state ();
Index: gcc/plugin.def
===================================================================
--- gcc/plugin.def	(revision 205009)
+++ gcc/plugin.def	(working copy)
@@ -92,6 +92,12 @@ DEFEVENT (PLUGIN_EARLY_GIMPLE_PASSES_END)
 /* Called when a pass is first instantiated.  */
 DEFEVENT (PLUGIN_NEW_PASS)
 
+/* Called when a file is #include-d or given thru #line directive.
+   Could happen many times.  The event data is the included file path,
+   as a const char* pointer.  */
+DEFEVENT (PLUGIN_INCLUDE_FILE)
+
+
 /* After the hard-coded events above, plugins can dynamically allocate events
    at run time.
    PLUGIN_EVENT_FIRST_DYNAMIC only appears as last enum element.  */

Re: patch PLUGIN_HEADER_FILE event for tracing of header inclusions.

[Joseph S. Myers]

On Mon, 18 Nov 2013, Basile Starynkevitch wrote:

> @@ -43,6 +44,7 @@
>  				   TARGET_OPTF.  */
>  #include "tm_p.h"		/* For C_COMMON_OVERRIDE_OPTIONS.  */
>  
> +
>  #ifndef DOLLARS_IN_IDENTIFIERS
>  # define DOLLARS_IN_IDENTIFIERS true
>  #endif

This is a spurious diff hunk that should not be in this patch.

> Ok for trunk?

OK minus the spurious change in the absence of objections from the plugin 
maintainers within 48 hours (or in the presence of approval from either of 
them).

-- 
Joseph S. Myers
joseph@codesourcery.com

patch PLUGIN_HEADER_FILE event for tracing of header inclusions.

[Basile Starynkevitch]

Hello All

After http://gcc.gnu.org/ml/gcc/2013-11/msg00360.html &
http://gcc.gnu.org/ml/gcc/2013-11/msg00361.html  &
http://gcc.gnu.org/ml/gcc/2013-11/msg00347.html I am proposing the
following patch



############## patch
Index: gcc/plugin.def
===================================================================
--- gcc/plugin.def	(revision 204976)
+++ gcc/plugin.def	(working copy)
@@ -92,6 +92,10 @@
 /* Called when a pass is first instantiated.  */
 DEFEVENT (PLUGIN_NEW_PASS)
 
+/* Called when a file is #include-d or given thru #line directive.  */
+DEFEVENT (PLUGIN_INCLUDE_FILE)
+
+
 /* After the hard-coded events above, plugins can dynamically allocate
events
    at run time.
    PLUGIN_EVENT_FIRST_DYNAMIC only appears as last enum element.  */
Index: gcc/c-family/c-opts.c
===================================================================
--- gcc/c-family/c-opts.c	(revision 204976)
+++ gcc/c-family/c-opts.c	(working copy)
@@ -34,6 +34,7 @@
 #include "debug.h"		/* For debug_hooks.  */
 #include "opts.h"
 #include "options.h"
+#include "plugin.h"		/* For PLUGIN_INCLUDE_FILE event.  */
 #include "mkdeps.h"
 #include "c-target.h"
 #include "tm.h"			/* For BYTES_BIG_ENDIAN,
@@ -43,6 +44,7 @@
 				   TARGET_OPTF.  */
 #include "tm_p.h"		/* For C_COMMON_OVERRIDE_OPTIONS.  */
 
+
 #ifndef DOLLARS_IN_IDENTIFIERS
 # define DOLLARS_IN_IDENTIFIERS true
 #endif
@@ -1397,6 +1399,16 @@
   else
     fe_file_change (new_map);
 
+  if (new_map 
+      && (new_map->reason == LC_ENTER || new_map->reason == LC_RENAME))
+    {
+      /* Signal to plugins that a file is included.  This could happen
+	 several times.  */
+      invoke_plugin_callbacks 
+	(PLUGIN_INCLUDE_FILE,
+	 const_cast<char*> (ORDINARY_MAP_FILE_NAME (new_map)));
+    }
+
   if (new_map == 0 || (new_map->reason == LC_LEAVE && MAIN_FILE_P
(new_map)))
     {
       pch_cpp_save_state ();
########################################### gcc/ChangeLog entry

2013-11-18  Basile Starynkevitch  <basile@starynkevitch.net>
	* plugins.def (PLUGIN_INCLUDE_FILE): New event.

########################################### gcc/c-family/ChangeLog entry

2013-11-18  Basile Starynkevitch  <basile@starynkevitch.net>
	* c-opts.c: Include plugin.h header.
	(cb_file_change): Sent plugin event PLUGIN_INCLUDE_FILE.

########################################### 

Ok for trunk?


Regards.




-- 
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 mine, sont seulement les miennes} ***