| Summary: | [4.8 Regression] Bootstrap with --disable-nls broken under Windows | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Tobias Burnus <burnus> |
| Component: | bootstrap | Assignee: | Diego Novillo <dnovillo> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | burnus, dnovillo, ebotcazou, fragabr, jakub, ktietz, nightstrike, roland |
| Priority: | P1 | Keywords: | build |
| Version: | 4.8.0 | ||
| Target Milestone: | 4.8.1 | ||
| Host: | Target: | windows | |
| Build: | Known to work: | ||
| Known to fail: | Last reconfirmed: | 2012-12-21 00:00:00 | |
| Attachments: |
patch
Proposed fix. Revert trunk commit r190487 |
||
From PR 54281: | With --disable-nls intl.h does | | #ifdef ENABLE_NLS | #include <libintl.h> | extern void gcc_init_libintl (void); | extern size_t gcc_gettext_width (const char *); | #else | /* Stubs. */ | # undef textdomain | # define textdomain(domain) (domain) | # undef bindtextdomain | # define bindtextdomain(domain, directory) (domain) | # undef gettext | # define gettext(msgid) (msgid) | | which wrecks an included libintl.h: | We end up including libintl.h through | | gcc/double-int.h | #include <gmp.h> | #include <iosfwd> (here from GCC 4.1) | #include <bits/c++locale.h> | #include <libintl.h> Side note: On my system only GCC 4.1's c++locale.h includes libintl.h, GCC 4.3/4.4/4.5/4.6/4.7 don't. And: | Possibly by looking for and | including libintl.h before re-defining those macros? It's the toplevel | intl.h btw. | Another fix is to include all system headers (and thus gmp.h) from system.h | which always comes before includes of intl.h. The patch for PR 54281 comment 10 (Rev. 190487) changed it to: #if defined(HAVE_LIBINTL_H) || defined(ENABLE_NLS) # include <libintl.h> #endif #ifdef ENABLE_NLS ... #else /* Stubs. */ # undef textdomain # define textdomain(domain) (domain) ... As written in comment 0, that breaks MinGW builds as libintl.h re-defined fprint on non-POSIX-printf systems. If one now tries to undo the effect by using: #if defined(HAVE_LIBINTL_H) || defined(ENABLE_NLS) # include <libintl.h> # if !defined(ENABLE_NLS) # undef fprintf # undef sprintf # undef snprintf # undef vfprintf # endif #endif One ends up with: libbackend.a(ipa-pure-const.o): In function `check_decl': C:\MinGW\msys\1.0\home\brad\gfortran\ibin\gcc/../../gcc-trunk/gcc/ipa-pure-const.c:253: undefined reference to `_libintl _fprintf' ... libcommon-target.a(opts.o): In function `wrap_help': C:\MinGW\msys\1.0\home\brad\gfortran\ibin\gcc/../../gcc-trunk/gcc/opts.c:884: undefined reference to `___printf__' Without having tested with neither MinGW nor with GCC 4.1/Linux, I wonder whether one could do something like: #if defined(HAVE_LIBINTL_H) && !defined(ENABLE_NLS) # define _LIBINTL_H 1 #end if #ifdef (ENABLE_NLS) # include <libintl.h> #endif Or is that too evil or won't it work? On Fri, 26 Oct 2012, burnus at gcc dot gnu.org wrote:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54659
>
> Tobias Burnus <burnus at gcc dot gnu.org> changed:
>
> What |Removed |Added
> ----------------------------------------------------------------------------
> CC| |burnus at gcc dot gnu.org,
> | |rguenther at suse dot de
>
> --- Comment #1 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-10-26 10:36:30 UTC ---
> From PR 54281:
>
> | With --disable-nls intl.h does
> |
> | #ifdef ENABLE_NLS
> | #include <libintl.h>
> | extern void gcc_init_libintl (void);
> | extern size_t gcc_gettext_width (const char *);
> | #else
> | /* Stubs. */
> | # undef textdomain
> | # define textdomain(domain) (domain)
> | # undef bindtextdomain
> | # define bindtextdomain(domain, directory) (domain)
> | # undef gettext
> | # define gettext(msgid) (msgid)
> |
> | which wrecks an included libintl.h:
> | We end up including libintl.h through
> |
> | gcc/double-int.h
> | #include <gmp.h>
> | #include <iosfwd> (here from GCC 4.1)
> | #include <bits/c++locale.h>
> | #include <libintl.h>
>
> Side note: On my system only GCC 4.1's c++locale.h includes libintl.h, GCC
> 4.3/4.4/4.5/4.6/4.7 don't.
>
> And:
> | Possibly by looking for and
> | including libintl.h before re-defining those macros? It's the toplevel
> | intl.h btw.
>
> | Another fix is to include all system headers (and thus gmp.h) from system.h
> | which always comes before includes of intl.h.
>
>
>
> The patch for PR 54281 comment 10 (Rev. 190487) changed it to:
>
> #if defined(HAVE_LIBINTL_H) || defined(ENABLE_NLS)
> # include <libintl.h>
> #endif
>
> #ifdef ENABLE_NLS
> ...
> #else
> /* Stubs. */
> # undef textdomain
> # define textdomain(domain) (domain)
> ...
>
>
>
> As written in comment 0, that breaks MinGW builds as libintl.h re-defined
> fprint on non-POSIX-printf systems. If one now tries to undo the effect by
> using:
>
> #if defined(HAVE_LIBINTL_H) || defined(ENABLE_NLS)
> # include <libintl.h>
> # if !defined(ENABLE_NLS)
> # undef fprintf
> # undef sprintf
> # undef snprintf
> # undef vfprintf
> # endif
> #endif
>
>
> One ends up with:
>
> libbackend.a(ipa-pure-const.o): In function `check_decl':
> C:\MinGW\msys\1.0\home\brad\gfortran\ibin\gcc/../../gcc-trunk/gcc/ipa-pure-const.c:253:
> undefined reference to `_libintl
> _fprintf'
> ...
> libcommon-target.a(opts.o): In function `wrap_help':
> C:\MinGW\msys\1.0\home\brad\gfortran\ibin\gcc/../../gcc-trunk/gcc/opts.c:884:
> undefined reference to `___printf__'
>
>
>
> Without having tested with neither MinGW nor with GCC 4.1/Linux, I wonder
> whether one could do something like:
>
> #if defined(HAVE_LIBINTL_H) && !defined(ENABLE_NLS)
> # define _LIBINTL_H 1
> #end if
>
> #ifdef (ENABLE_NLS)
> # include <libintl.h>
> #endif
>
> Or is that too evil or won't it work?
Fact is that all this stuff happens because gmp.h is not included
from system.h ...
On Fri, Oct 26, 2012 at 8:05 AM, rguenther at suse dot de <gcc-bugzilla@gcc.gnu.org> wrote: > Fact is that all this stuff happens because gmp.h is not included > from system.h ... I broke Ada when I tried it. I don't remember the details, but it seemed tedious to fix. Diego. On Fri, 26 Oct 2012, dnovillo at google dot com wrote:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54659
>
> --- Comment #3 from dnovillo at google dot com <dnovillo at google dot com> 2012-10-26 12:34:53 UTC ---
> On Fri, Oct 26, 2012 at 8:05 AM, rguenther at suse dot de
> <gcc-bugzilla@gcc.gnu.org> wrote:
>
> > Fact is that all this stuff happens because gmp.h is not included
> > from system.h ...
>
> I broke Ada when I tried it. I don't remember the details, but it
> seemed tedious to fix.
I know ... but it's the only way that is designed to avoid this
kind of issues.
Richard.
1(In reply to comment #3) > On Fri, Oct 26, 2012 at 8:05 AM, rguenther at suse dot de > <gcc-bugzilla@gcc.gnu.org> wrote: > > > Fact is that all this stuff happens because gmp.h is not included > > from system.h ... > > I broke Ada when I tried it. I don't remember the details, but it > seemed tedious to fix. > > > Diego. So instead you broke Windows? Tobias, what is your full configure line alongside --disable-nls? (In reply to comment #4) > On Fri, 26 Oct 2012, dnovillo at google dot com wrote: > > > > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54659 > > > > --- Comment #3 from dnovillo at google dot com <dnovillo at google dot com> 2012-10-26 12:34:53 UTC --- > > On Fri, Oct 26, 2012 at 8:05 AM, rguenther at suse dot de > > <gcc-bugzilla@gcc.gnu.org> wrote: > > > > > Fact is that all this stuff happens because gmp.h is not included > > > from system.h ... > > > > I broke Ada when I tried it. I don't remember the details, but it > > seemed tedious to fix. > > I know ... but it's the only way that is designed to avoid this > kind of issues. Trying to see what happens ... The issue is r176210 wrapping everything in #ifdef __cplusplus extern "C" { #endif ... #ifdef __cplusplus } #endif _including_ #include statements, including the inclusion of system.h. That of course is totally bogus. What it wants is to make sure exports have C linkage (I assume). A fix is to undo this ... like with the following hack: Index: gcc/system.h =================================================================== --- gcc/system.h (revision 194658) +++ gcc/system.h (working copy) @@ -24,6 +24,11 @@ along with GCC; see the file COPYING3. #ifndef GCC_SYSTEM_H #define GCC_SYSTEM_H +/* Undo extern "C" wrappings done by including files (Ada). */ +#ifdef __cplusplus +extern "C++" { +#endif + /* We must include stdarg.h before stdio.h. */ #include <stdarg.h> @@ -1051,4 +1056,8 @@ helper_const_non_const_cast (const char #define DEBUG_VARIABLE #endif +#ifdef __cplusplus +} +#endif + #endif /* ! GCC_SYSTEM_H */ but of course even better would be to fix the reason for this hack - why are those Ada files built with a C++ compiler in the first place?! Why does it need to wrap _everything_ inside extern "C"?? Ada people? Created attachment 29020 [details]
patch
Thus, try this patch. Does it fix the windows issue with --disable-nls?
(do you need to revert the original --disable-nls fix?)
> but of course even better would be to fix the reason for this hack - why > are those Ada files built with a C++ compiler in the first place?! Probably because it would be too complex to invoke 2 compilers in the Makefile. But nobody tried if I recall correctly and volunteers are always welcome. > Why does it need to wrap _everything_ inside extern "C"?? See prefix.h: because the foreign language interface of the Ada part is hardcoded for the C language. This cannot really be changed for the time being, but if the directives are misplaced, feel free to move them around. Author: rguenth Date: Fri Dec 21 14:33:13 2012 New Revision: 194665 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=194665 Log: 2012-12-21 Richard Biener <rguenther@suse.de> PR bootstrap/54659 * system.h: Include gmp.h. * tree-ssa-loop-niter.c: Do not include gmp.h here. * double-int.h: Likewise. * realmpfr.h: Likewise. fortran/ * gfortran.h: Do not include gmp.h here. Modified: trunk/gcc/ChangeLog trunk/gcc/double-int.h trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/gfortran.h trunk/gcc/realmpfr.h trunk/gcc/system.h trunk/gcc/tree-ssa-loop-niter.c Should be fixed. Please check if it works with or without reverting
2012-08-17 Diego Novillo <dnovillo@google.com>
PR bootstrap/54281
* configure.ac: Add libintl.h to AC_CHECK_HEADERS list.
* config.in: Regenerate.
* configure: Regenerate.
* intl.h: Always include libintl.h if HAVE_LIBINTL_H is
set.
Author: ian Date: Fri Dec 21 15:59:27 2012 New Revision: 194669 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=194669 Log: PR bootstrap/54659 compiler: Don't include <gmp.h>, now included by go-system.h. * go-system.h: Don't include <cstdio>. Modified: trunk/gcc/go/ChangeLog trunk/gcc/go/go-system.h trunk/gcc/go/gofrontend/expressions.cc trunk/gcc/go/gofrontend/expressions.h trunk/gcc/go/gofrontend/gogo-tree.cc trunk/gcc/go/gofrontend/lex.h trunk/gcc/go/gofrontend/runtime.cc trunk/gcc/go/gofrontend/statements.cc trunk/gcc/go/gofrontend/types.cc I assume it is indeed fixed. Please re-open if not. Not fixed. Still reproducible with final 4.8.0 See also http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56644 Confirmed still broken on MinGW with --disable-nls. Confirmed that reverting Diego's 2012-08-17 change fixes it. I don't seem to be able to reopen this bug. Re-opening on request from Roland. Diego, can you please revert your change then on the trunk for now? Sure. But I'm not quite sure which change you are referring to. I see 2 related patches (well, 3 but one is the reversal of the first one). I suppose we are referring to rev 190487?
commit 4b9beb88f5d49452a1fb25826c00cd81b7461b04
Author: dnovillo <dnovillo@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Fri Aug 17 15:37:57 2012 +0000
2012-08-17 Diego Novillo <dnovillo@google.com>
PR bootstrap/54281
* configure.ac: Add libintl.h to AC_CHECK_HEADERS list.
* config.in: Regenerate.
* configure: Regenerate.
* intl.h: Always include libintl.h if HAVE_LIBINTL_H is
set. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@190487 138bc75d-0d04-0410-961f-82ee72b054a4
commit 2a02178fdd2f016404c835abe988c427207b3f5aAuthor: dnovillo <dnovillo@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Thu Aug 16 18:24:22 2012 +0000
2012-08-16 Diego Novillo <dnovillo@google.com>
Revert
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.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@190449 138bc75d-0d04-0410-961f-82ee72b054a4
commit f5d9dd2ea4fdacef34156ebc853cfeba4e6f301c
Author: dnovillo <dnovillo@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Thu Aug 16 13:28:13 2012 +0000
2012-08-16 Diego Novillo <dnovillo@google.com>
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.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@190444 138bc75d-0d04-0410-961f-82ee72b054a4
r190487 is the breaking change. I tested reverting that (relative to 4.8 branch) and it solved the problem. Note I'm more concerned with having this fixed on the 4.8 branch than on the trunk. Any progress with this? We'd like to do 4.8.1-rc1 in mid-May, would be nice to have this resolved till then. On 2013-04-29 11:25 , jakub at gcc dot gnu.org wrote:
> Any progress with this? We'd like to do 4.8.1-rc1 in mid-May, would be nice to
> have this resolved till then.
>
None at all, sorry. I will try to work on this after I get back (early
next week).
Diego.
Looking at this today. Created attachment 30046 [details] Proposed fix. Revert trunk commit r190487 The patch I just attached reverts the commit that caused this problem, but I am unable to test it properly. I need help with: 1- Making sure this reversal does not re-introduce http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54281 2- Testing it on MinGW. I have no access to this system. I have been using a straightforward revert of r190487 to build on mingw with --disable-nls. It works. On 2013-05-07 13:06 , roland at gnu dot org wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54659 > > --- Comment #25 from roland at gnu dot org 2013-05-07 17:06:56 UTC --- > I have been using a straightforward revert of r190487 to build on mingw with > --disable-nls. It works. > Thanks. Then I just need to confirm that this doesn't re-introduce http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54281. Richi, could you check that? Do you still have access to 4.1 host compilers? On Tue, 7 May 2013, Diego Novillo wrote: > On 2013-05-07 13:06 , roland at gnu dot org wrote: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54659 > > > > --- Comment #25 from roland at gnu dot org 2013-05-07 17:06:56 UTC --- > > I have been using a straightforward revert of r190487 to build on mingw with > > --disable-nls. It works. > > > > > Thanks. Then I just need to confirm that this doesn't re-introduce > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54281. Richi, could you check > that? Do you still have access to 4.1 host compilers? I've verified that reverting r190487 on the 4.8 branch does not re-introduce the issue on the originally affected host system. Richard. On 2013-05-08 06:05 , Richard Biener wrote: > On Tue, 7 May 2013, Diego Novillo wrote: > >> On 2013-05-07 13:06 , roland at gnu dot org wrote: >>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54659 >>> >>> --- Comment #25 from roland at gnu dot org 2013-05-07 17:06:56 UTC --- >>> I have been using a straightforward revert of r190487 to build on mingw with >>> --disable-nls. It works. >>> >> >> Thanks. Then I just need to confirm that this doesn't re-introduce >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54281. Richi, could you check >> that? Do you still have access to 4.1 host compilers? > I've verified that reverting r190487 on the 4.8 branch does not > re-introduce the issue on the originally affected host system. > > Richard. Thanks, folks. I've applied the patch to trunk and gcc-4_8-branch. Diego. Fixed in gcc-4_8-branch at rev 198708 and trunk at rev 198711. (In reply to Diego Novillo from comment #29) > Fixed in gcc-4_8-branch at rev 198708 and trunk at rev 198711. Diego, I'm using gcc 4.8.2 20130905 snapshot and it's generating libstdc++ with those symbols: nm /usr/local/lib64/libstdc++.so.6.0.18|grep libintl U libintl_bindtextdomain U libintl_gettext U libintl_textdomain And I get the same problem reported in this bug. It started with gcc 4.8.0 and until now my workaround was to add "LDFLAGS=-lintl" when I compile some software. So I'm afraid this bug still exist or it was reintroduced at some point. Is there any test I can do for you? Thank you. |
The following commit includes libintl.h even with --disable-nls: 2012-08-17 Diego Novillo <dnovillo@google.com> PR bootstrap/54281 * configure.ac: Add libintl.h to AC_CHECK_HEADERS list. * config.in: Regenerate. * configure: Regenerate. * intl.h: Always include libintl.h if HAVE_LIBINTL_H is set. That breaks bootstrap on Windows: C:\MinGW\msys\1.0\home\brad\gfortran\ibin\gcc/../../gcc-trunk/gcc/c-family/c-lex.c:152: undefined reference to `_libintl _fprintf' The reason is that libintl.h re-defines fprint on #if !@HAVE_POSIX_PRINTF@ systems, see: http://git.savannah.gnu.org/cgit/gettext.git/tree/gettext-runtime/intl/libgnuintl.in.h