Patchwork [bootstrap] Tentative fix for PR 54281

login
register
mail settings
Submitter Diego Novillo
Date Aug. 16, 2012, 11:55 a.m.
Message ID <20120816115551.GA13453@google.com>
Download mbox | patch
Permalink /patch/177965/
State New
Headers show

Comments

Diego Novillo - Aug. 16, 2012, 11:55 a.m.
Richi, this implements your idea for fixing PR 54281.  I don't
have an old enough compiler.  Could you please test it in your
system?

I debated whether to remove the GENERATOR_FILE predicate from the
inclusion (some files include gmp.h unconditionally).  I think it
could be removed, but only a minority of files build with
GENERATOR_FILE set, so it didn't seem harmful.

OK if tests pass?


Diego.


2012-08-16  Diego Novillo  <dnovillo@google.com>
            Richard Guenther  <rguenther@suse.de>

	PR bootstrap/54281
	* double-int.h: Move including of gmp.h ...
	* system.h: ... here.
	* realmpfr.h: Do not include gmp.h.
	* tree-ssa-loop-niter.c: Do not include gmp.h.

fortran/ChangeLog
	* gfortran.h: Do not include gmp.h.
Richard Guenther - Aug. 16, 2012, 1:08 p.m.
On Thu, 16 Aug 2012, Diego Novillo wrote:

> Richi, this implements your idea for fixing PR 54281.  I don't
> have an old enough compiler.  Could you please test it in your
> system?
> 
> I debated whether to remove the GENERATOR_FILE predicate from the
> inclusion (some files include gmp.h unconditionally).  I think it
> could be removed, but only a minority of files build with
> GENERATOR_FILE set, so it didn't seem harmful.
> 
> OK if tests pass?

It fixes it.

Thus, ok from my side (if your testing succeeds).

Thanks,
Richard.

> 
> Diego.
> 
> 
> 2012-08-16  Diego Novillo  <dnovillo@google.com>
>             Richard Guenther  <rguenther@suse.de>
> 
> 	PR bootstrap/54281
> 	* double-int.h: Move including of gmp.h ...
> 	* system.h: ... here.
> 	* realmpfr.h: Do not include gmp.h.
> 	* tree-ssa-loop-niter.c: Do not include gmp.h.
> 
> fortran/ChangeLog
> 	* gfortran.h: Do not include gmp.h.
> 
> diff --git a/gcc/double-int.h b/gcc/double-int.h
> index 3d9aa2c..7ea0528 100644
> --- a/gcc/double-int.h
> +++ b/gcc/double-int.h
> @@ -20,10 +20,6 @@ along with GCC; see the file COPYING3.  If not see
>  #ifndef DOUBLE_INT_H
>  #define DOUBLE_INT_H
>  
> -#ifndef GENERATOR_FILE
> -#include <gmp.h>
> -#endif
> -
>  /* A large integer is currently represented as a pair of HOST_WIDE_INTs.
>     It therefore represents a number with precision of
>     2 * HOST_BITS_PER_WIDE_INT bits (it is however possible that the
> diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
> index 7c4c0a4..611d16d 100644
> --- a/gcc/fortran/gfortran.h
> +++ b/gcc/fortran/gfortran.h
> @@ -1681,7 +1681,6 @@ gfc_intrinsic_sym;
>     EXPR_COMPCALL   Function (or subroutine) call of a procedure pointer
>  		   component or type-bound procedure.  */
>  
> -#include <gmp.h>
>  #include <mpfr.h>
>  #include <mpc.h>
>  #define GFC_RND_MODE GMP_RNDN
> diff --git a/gcc/realmpfr.h b/gcc/realmpfr.h
> index ab234e9..ada876e 100644
> --- a/gcc/realmpfr.h
> +++ b/gcc/realmpfr.h
> @@ -22,7 +22,10 @@
>  #ifndef GCC_REALGMP_H
>  #define GCC_REALGMP_H
>  
> -#include <gmp.h>
> +/* Note that we do not include gmp.h.  It is included in system.h
> +   because it wrecks intl.h when compiling in C++ mode.
> +   See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54281 for details.  */
> +
>  #include <mpfr.h>
>  #include <mpc.h>
>  #include "real.h"
> diff --git a/gcc/system.h b/gcc/system.h
> index 9e7d503..0ccd991 100644
> --- a/gcc/system.h
> +++ b/gcc/system.h
> @@ -1037,4 +1037,8 @@ helper_const_non_const_cast (const char *p)
>  #define DEBUG_VARIABLE
>  #endif
>  
> +#ifndef GENERATOR_FILE
> +#include <gmp.h>
> +#endif
> +
>  #endif /* ! GCC_SYSTEM_H */
> diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
> index c719a74..4c67c26 100644
> --- a/gcc/tree-ssa-loop-niter.c
> +++ b/gcc/tree-ssa-loop-niter.c
> @@ -38,7 +38,6 @@ along with GCC; see the file COPYING3.  If not see
>  #include "flags.h"
>  #include "diagnostic-core.h"
>  #include "tree-inline.h"
> -#include "gmp.h"
>  
>  #define SWAP(X, Y) do { affine_iv *tmp = (X); (X) = (Y); (Y) = tmp; } while (0)
>  
> 
>
Diego Novillo - Aug. 16, 2012, 1:28 p.m.
On 12-08-16 09:08 , Richard Guenther wrote:

> It fixes it.
>
> Thus, ok from my side (if your testing succeeds).

Worked here too.  Committed to trunk.


Diego.
Diego Novillo - Aug. 16, 2012, 1:47 p.m.
On 12-08-16 09:08 , Richard Guenther wrote:
> On Thu, 16 Aug 2012, Diego Novillo wrote:
>
>> Richi, this implements your idea for fixing PR 54281.  I don't
>> have an old enough compiler.  Could you please test it in your
>> system?
>>
>> I debated whether to remove the GENERATOR_FILE predicate from the
>> inclusion (some files include gmp.h unconditionally).  I think it
>> could be removed, but only a minority of files build with
>> GENERATOR_FILE set, so it didn't seem harmful.
>>
>> OK if tests pass?
>
> It fixes it.
>
> Thus, ok from my side (if your testing succeeds).

So, I had failed to include Ada in my testing and my patch breaks it. 
In several ada/*.c files, we forcefully include system.h as a C header:

#ifdef __cplusplus
extern "C" {
#endif
...
#include "system.h"
...
#ifdef __cplusplus
}
#endif


Eric, is this really needed now?  We are including gmp.h from system.h 
due to PR 54281.  This is causing Ada builds to fail.

Would it be OK to remove all the '#ifdef __cplusplus' conditionals from 
ada/*.c or is there something I'm missing?


Thanks.  Diego.
Magnus Fromreide - Aug. 16, 2012, 5:50 p.m.
On Thu, Aug 16, 2012 at 07:55:51AM -0400, Diego Novillo wrote:
> Richi, this implements your idea for fixing PR 54281.  I don't
> have an old enough compiler.  Could you please test it in your
> system?
> 
> I debated whether to remove the GENERATOR_FILE predicate from the
> inclusion (some files include gmp.h unconditionally).  I think it
> could be removed, but only a minority of files build with
> GENERATOR_FILE set, so it didn't seem harmful.
> 
> OK if tests pass?

This patch breaks building with gmp, cloog and isl in subdirectories of the
source tree.


echo timestamp > s-options
gawk -f ../../trunk/gcc/opt-functions.awk -f ../../trunk/gcc/opt-read.awk \
       -f ../../trunk/gcc/opth-gen.awk \
       < optionlist > tmp-options.h
/bin/sh ../../trunk/gcc/../move-if-change tmp-options.h options.h
echo timestamp > s-options-h
TARGET_CPU_DEFAULT="" \
HEADERS="auto-host.h ansidecl.h" DEFINES="" \
/bin/sh ../../trunk/gcc/mkconfig.sh bconfig.h
g++ -c   -g -DIN_GCC   -fno-exceptions -fno-rtti -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common  -DHAVE_CONFIG_H -DGENERATOR_FILE -I. -Ibuild -I../../trunk/gcc -I../../trunk/gcc/build -I../../trunk/gcc/../include -I../../trunk/gcc/../libcpp/include -I/home/magfr/src/gcc/build/./gmp -I/home/magfr/src/gcc/trunk/gmp -I/home/magfr/src/gcc/build/./mpfr -I/home/magfr/src/gcc/trunk/mpfr -I/home/magfr/src/gcc/trunk/mpc/src  -I../../trunk/gcc/../libdecnumber -I../../trunk/gcc/../libdecnumber/bid -I../libdecnumber -DCLOOG_INT_GMP -I/home/magfr/src/gcc/build/./cloog/include -I/home/magfr/src/gcc/trunk/cloog/include -I../trunk/cloog/include  -I/home/magfr/src/gcc/build/./isl/include -I/home/magfr/src/gcc/trunk/isl/include  \
        -o build/genconstants.o ../../trunk/gcc/genconstants.c
In file included from /usr/include/sys/resource.h:25:0,
                 from /usr/include/sys/wait.h:32,
                 from ../../trunk/gcc/system.h:351,
                 from ../../trunk/gcc/genconstants.c:29:
/usr/include/bits/resource.h:133:18: error: declaration does not declare anything [-fpermissive]
In file included from ../../trunk/gcc/genconstants.c:29:0:
../../trunk/gcc/system.h:443:23: error: declaration of C function `void* sbrk(int)' conflicts with
In file included from ../../trunk/gcc/system.h:253:0,
                 from ../../trunk/gcc/genconstants.c:29:
/usr/include/unistd.h:1067:14: error: previous declaration `void* sbrk(intptr_t)' here
In file included from ../../trunk/gcc/genconstants.c:29:0:
../../trunk/gcc/system.h:447:48: error: new declaration `char* strstr(const char*, const char*)'
In file included from /usr/lib/gcc/x86_64-redhat-linux/4.7.0/../../../../include/c++/4.7.0/cstring:44:0,
                 from ../../trunk/gcc/system.h:207,
                 from ../../trunk/gcc/genconstants.c:29:
/usr/include/string.h:323:22: error: ambiguates old declaration `const char* strstr(const char*, const char*)'
In file included from ../../trunk/gcc/genconstants.c:29:0:
../../trunk/gcc/system.h:499:34: error: declaration of C function `const char* strsignal(int)' conflicts with
In file included from /usr/lib/gcc/x86_64-redhat-linux/4.7.0/../../../../include/c++/4.7.0/cstring:44:0,
                 from ../../trunk/gcc/system.h:207,
                 from ../../trunk/gcc/genconstants.c:29:
/usr/include/string.h:566:14: error: previous declaration `char* strsignal(int)' here
In file included from ../../trunk/gcc/system.h:639:0,
                 from ../../trunk/gcc/genconstants.c:29:
../../trunk/gcc/../include/libiberty.h:110:36: error: new declaration `char* basename(const char*)'
In file included from /usr/lib/gcc/x86_64-redhat-linux/4.7.0/../../../../include/c++/4.7.0/cstring:44:0,
                 from ../../trunk/gcc/system.h:207,
                 from ../../trunk/gcc/genconstants.c:29:
/usr/include/string.h:603:28: error: ambiguates old declaration `const char* basename(const char*)'
make[3]: *** [build/genconstants.o] Error 1
make[3]: Leaving directory `/home/magfr/src/gcc/build/gcc'
make[2]: *** [all-stage1-gcc] Error 2
make[2]: Leaving directory `/home/magfr/src/gcc/build'
make[1]: *** [stage1-bubble] Error 2
make[1]: Leaving directory `/home/magfr/src/gcc/build'
make: *** [all] Error 2


The problem seems to be that during configure time the test scripts include
system.h, but it fails to add the gmp include directory to the compiler flags
so the checks for sbrk, strstr and so on fails with


configure:10294: checking whether sbrk is declared
configure:10317: gcc -c -g -I../../trunk/gcc -I../../trunk/gcc/../include  conftest.c >&5
In file included from conftest.c:125:0:
../../trunk/gcc/system.h:1041:17: fatal error: gmp.h: No such file or directory
compilation terminated.
configure:10317: $? = 1


Reverting this patch makes the compiler build.

/MF
Diego Novillo - Aug. 16, 2012, 5:53 p.m.
On Thu, Aug 16, 2012 at 1:50 PM, Magnus Fromreide <magfr@lysator.liu.se> wrote:
> On Thu, Aug 16, 2012 at 07:55:51AM -0400, Diego Novillo wrote:
>> Richi, this implements your idea for fixing PR 54281.  I don't
>> have an old enough compiler.  Could you please test it in your
>> system?
>>
>> I debated whether to remove the GENERATOR_FILE predicate from the
>> inclusion (some files include gmp.h unconditionally).  I think it
>> could be removed, but only a minority of files build with
>> GENERATOR_FILE set, so it didn't seem harmful.
>>
>> OK if tests pass?
>
> This patch breaks building with gmp, cloog and isl in subdirectories of the
> source tree.

Working on a fix.  It also exposed problems in Ada.


Diego.
Eric Botcazou - Aug. 19, 2012, 9:34 a.m.
[Sorry for the delay]

> So, I had failed to include Ada in my testing and my patch breaks it.
> In several ada/*.c files, we forcefully include system.h as a C header:
> 
> #ifdef __cplusplus
> extern "C" {
> #endif
> ...
> #include "system.h"
> ...
> #ifdef __cplusplus
> }
> #endif
> 
> 
> Eric, is this really needed now?  We are including gmp.h from system.h
> due to PR 54281.  This is causing Ada builds to fail.
> 
> Would it be OK to remove all the '#ifdef __cplusplus' conditionals from
> ada/*.c or is there something I'm missing?

The conditionals cannot be removed for the time being because the foreign 
language interface of the Ada part of the compiler is hardcoded for C.

Barring a massive switch to the Ada language for the GNU project, we would need 
to switch the foreign language interface to C++, which might introduce various 
maintenance issues in the short term (Arno CCed).
Arnaud Charlet - Aug. 19, 2012, 11:18 a.m.
> The conditionals cannot be removed for the time being because the foreign 
> language interface of the Ada part of the compiler is hardcoded for C.
> 
> Barring a massive switch to the Ada language for the GNU project, we would need 
> to switch the foreign language interface to C++, which might introduce various 
> maintenance issues in the short term (Arno CCed).

Yes, that would be a large hearthquake for both the compiler and the run-time, which is unwanted at this stage. I guess the solution for now is to more selectively export the various symbols as C symbols and include header files as is.

Arno
Diego Novillo - Aug. 20, 2012, 1:27 p.m.
On 2012-08-19 07:18 , Arnaud Charlet wrote:
>
>> The conditionals cannot be removed for the time being because the
>> foreign language interface of the Ada part of the compiler is
>> hardcoded for C.
>>
>> Barring a massive switch to the Ada language for the GNU project,
>> we would need to switch the foreign language interface to C++,
>> which might introduce various maintenance issues in the short term
>> (Arno CCed).
>
> Yes, that would be a large hearthquake for both the compiler and the
> run-time, which is unwanted at this stage. I guess the solution for
> now is to more selectively export the various symbols as C symbols
> and include header files as is.

Thanks.

One potential issue I see here is that as we start using more C++ 
headers (and more C++ in existing headers), we will reach a point in 
which including something like system.h inside an extern "C" {} block 
will fail.

None of the GCC headers should be included as C code anymore.


Diego.

Patch

diff --git a/gcc/double-int.h b/gcc/double-int.h
index 3d9aa2c..7ea0528 100644
--- a/gcc/double-int.h
+++ b/gcc/double-int.h
@@ -20,10 +20,6 @@  along with GCC; see the file COPYING3.  If not see
 #ifndef DOUBLE_INT_H
 #define DOUBLE_INT_H
 
-#ifndef GENERATOR_FILE
-#include <gmp.h>
-#endif
-
 /* A large integer is currently represented as a pair of HOST_WIDE_INTs.
    It therefore represents a number with precision of
    2 * HOST_BITS_PER_WIDE_INT bits (it is however possible that the
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 7c4c0a4..611d16d 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -1681,7 +1681,6 @@  gfc_intrinsic_sym;
    EXPR_COMPCALL   Function (or subroutine) call of a procedure pointer
 		   component or type-bound procedure.  */
 
-#include <gmp.h>
 #include <mpfr.h>
 #include <mpc.h>
 #define GFC_RND_MODE GMP_RNDN
diff --git a/gcc/realmpfr.h b/gcc/realmpfr.h
index ab234e9..ada876e 100644
--- a/gcc/realmpfr.h
+++ b/gcc/realmpfr.h
@@ -22,7 +22,10 @@ 
 #ifndef GCC_REALGMP_H
 #define GCC_REALGMP_H
 
-#include <gmp.h>
+/* Note that we do not include gmp.h.  It is included in system.h
+   because it wrecks intl.h when compiling in C++ mode.
+   See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54281 for details.  */
+
 #include <mpfr.h>
 #include <mpc.h>
 #include "real.h"
diff --git a/gcc/system.h b/gcc/system.h
index 9e7d503..0ccd991 100644
--- a/gcc/system.h
+++ b/gcc/system.h
@@ -1037,4 +1037,8 @@  helper_const_non_const_cast (const char *p)
 #define DEBUG_VARIABLE
 #endif
 
+#ifndef GENERATOR_FILE
+#include <gmp.h>
+#endif
+
 #endif /* ! GCC_SYSTEM_H */
diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
index c719a74..4c67c26 100644
--- a/gcc/tree-ssa-loop-niter.c
+++ b/gcc/tree-ssa-loop-niter.c
@@ -38,7 +38,6 @@  along with GCC; see the file COPYING3.  If not see
 #include "flags.h"
 #include "diagnostic-core.h"
 #include "tree-inline.h"
-#include "gmp.h"
 
 #define SWAP(X, Y) do { affine_iv *tmp = (X); (X) = (Y); (Y) = tmp; } while (0)