diff mbox

Fix PR46916/46902

Message ID E00DD7AD-F729-425D-BFDE-950828D1C6D8@sandoe-acoustics.co.uk
State New
Headers show

Commit Message

Iain Sandoe Dec. 17, 2010, 9:55 a.m. UTC
The two PRs turned out to be manifestations of the same issue.

Basically,
gcc/system.h redefines bool => unsigned char.

this causes a conflict with any ABI where bool/_Bool is defined to  
something different (e.g. int).

(I don't think PPC/Darwin is the only case where this was done...  but  
it might be the only modern case).

anyway the "gotcha" is that all {external} system headers _must_ be  
included before gcc/system.h (noted in that header).

OK for trunk?
Iain

gcc:

	PR gcc/46902
	PR testsuite/46912
	* plugin.c: Ensure system headers are included before
	gcc/system.h

Comments

Mike Stump Dec. 17, 2010, 9:11 p.m. UTC | #1
On Dec 17, 2010, at 1:55 AM, IainS <developer@sandoe-acoustics.co.uk> wrote:
> The two PRs turned out to be manifestations of the same issue.
> 
> Basically,
> gcc/system.h redefines bool => unsigned char.
> 
> this causes a conflict with any ABI where bool/_Bool is defined to something different (e.g. int).
> 
> (I don't think PPC/Darwin is the only case where this was done...  but it might be the only modern case).
> 
> anyway the "gotcha" is that all {external} system headers _must_ be included before gcc/system.h (noted in that header).

I'll say what I said last time, this came up, but I'll be more blunt this round, this is totally wrong, it should either have it's own unique name (not bool), or it should use _Bool, everything else is wrong.  That said, the below patch is in line with the current code's design, as awful as it is.
>
Iain Sandoe Jan. 7, 2011, 2:58 p.m. UTC | #2
Apologies for the wrong PR # in the original subject,

Approved from the Darwin perspective in  http://gcc.gnu.org/ml/gcc-patches/2010-12/msg01425.html

since there is no specific maintainer listed for the plugin  
infrastructure, and this is verging on obvious,  perhaps an RM could  
approve?

Iain

On 17 Dec 2010, at 09:55, IainS wrote:

> The two PRs turned out to be manifestations of the same issue.
>
> Basically,
> gcc/system.h redefines bool => unsigned char.
>
> this causes a conflict with any ABI where bool/_Bool is defined to  
> something different (e.g. int).
>
> (I don't think PPC/Darwin is the only case where this was done...   
> but it might be the only modern case).
>
> anyway the "gotcha" is that all {external} system headers _must_ be  
> included before gcc/system.h (noted in that header).
>
> OK for trunk?
> Iain
>
> gcc:
>
> 	PR gcc/46902
> 	PR testsuite/46912
> 	* plugin.c: Ensure system headers are included before
> 	gcc/system.h
>
> Index: gcc/plugin.c
> ===================================================================
> --- gcc/plugin.c	(revision 167973)
> +++ gcc/plugin.c	(working copy)
> @@ -21,16 +21,19 @@ along with GCC; see the file COPYING3.  If not see
>    APIs described in doc/plugin.texi.  */
>
> #include "config.h"
> -#include "system.h"
>
> /* If plugin support is not enabled, do not try to execute any code
>    that may reference libdl.  The generic code is still compiled in to
>    avoid including too many conditional compilation paths in the rest
> -   of the compiler.  */
> +   of the compiler.
> +
> +   We must include system headers before "system.h" or the override
> +   for bool might be upset. */
> #ifdef ENABLE_PLUGIN
> #include <dlfcn.h>
> #endif
>
> +#include "system.h"
> #include "coretypes.h"
> #include "diagnostic-core.h"
> #include "tree.h"
>
>
Richard Biener Jan. 7, 2011, 6:08 p.m. UTC | #3
On Fri, Jan 7, 2011 at 3:58 PM, IainS <developer@sandoe-acoustics.co.uk> wrote:
> Apologies for the wrong PR # in the original subject,
>
> Approved from the Darwin perspective in
>  http://gcc.gnu.org/ml/gcc-patches/2010-12/msg01425.html
>
> since there is no specific maintainer listed for the plugin infrastructure,
> and this is verging on obvious,  perhaps an RM could approve?
>
> Iain
>
> On 17 Dec 2010, at 09:55, IainS wrote:
>
>> The two PRs turned out to be manifestations of the same issue.
>>
>> Basically,
>> gcc/system.h redefines bool => unsigned char.
>>
>> this causes a conflict with any ABI where bool/_Bool is defined to
>> something different (e.g. int).
>>
>> (I don't think PPC/Darwin is the only case where this was done...  but it
>> might be the only modern case).
>>
>> anyway the "gotcha" is that all {external} system headers _must_ be
>> included before gcc/system.h (noted in that header).
>>
>> OK for trunk?
>> Iain
>>
>> gcc:
>>
>>        PR gcc/46902
>>        PR testsuite/46912
>>        * plugin.c: Ensure system headers are included before
>>        gcc/system.h
>>
>> Index: gcc/plugin.c
>> ===================================================================
>> --- gcc/plugin.c        (revision 167973)
>> +++ gcc/plugin.c        (working copy)
>> @@ -21,16 +21,19 @@ along with GCC; see the file COPYING3.  If not see
>>   APIs described in doc/plugin.texi.  */
>>
>> #include "config.h"
>> -#include "system.h"
>>
>> /* If plugin support is not enabled, do not try to execute any code
>>   that may reference libdl.  The generic code is still compiled in to
>>   avoid including too many conditional compilation paths in the rest
>> -   of the compiler.  */
>> +   of the compiler.
>> +
>> +   We must include system headers before "system.h" or the override
>> +   for bool might be upset. */
>> #ifdef ENABLE_PLUGIN
>> #include <dlfcn.h>
>> #endif
>>
>> +#include "system.h"

Hmm.  dlfcn should be included from system.h instead.

Richard.

>> #include "coretypes.h"
>> #include "diagnostic-core.h"
>> #include "tree.h"
>>
>>
>
>
diff mbox

Patch

Index: gcc/plugin.c
===================================================================
--- gcc/plugin.c	(revision 167973)
+++ gcc/plugin.c	(working copy)
@@ -21,16 +21,19 @@  along with GCC; see the file COPYING3.  If not see
     APIs described in doc/plugin.texi.  */

  #include "config.h"
-#include "system.h"

  /* If plugin support is not enabled, do not try to execute any code
     that may reference libdl.  The generic code is still compiled in to
     avoid including too many conditional compilation paths in the rest
-   of the compiler.  */
+   of the compiler.
+
+   We must include system headers before "system.h" or the override
+   for bool might be upset. */
  #ifdef ENABLE_PLUGIN
  #include <dlfcn.h>
  #endif

+#include "system.h"
  #include "coretypes.h"
  #include "diagnostic-core.h"
  #include "tree.h"