User account creation filtered due to spam.

Bug 54659 - [4.8 Regression] Bootstrap with --disable-nls broken under Windows
Summary: [4.8 Regression] Bootstrap with --disable-nls broken under Windows
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: bootstrap (show other bugs)
Version: 4.8.0
: P1 normal
Target Milestone: 4.8.1
Assignee: Diego Novillo
URL:
Keywords: build
Depends on:
Blocks:
 
Reported: 2012-09-21 16:54 UTC by Tobias Burnus
Modified: 2013-09-08 00:54 UTC (History)
10 users (show)

See Also:
Host:
Target: windows
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-12-21 00:00:00


Attachments
patch (982 bytes, patch)
2012-12-21 11:42 UTC, Richard Biener
Details | Diff
Proposed fix. Revert trunk commit r190487 (882 bytes, patch)
2013-05-07 16:55 UTC, Diego Novillo
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2012-09-21 16:54:33 UTC
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
Comment 1 Tobias Burnus 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?
Comment 2 rguenther@suse.de 2012-10-26 12:05:14 UTC
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 ...
Comment 3 dnovillo@google.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.


Diego.
Comment 4 rguenther@suse.de 2012-10-26 12:36:30 UTC
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.
Comment 5 nightstrike 2012-12-02 05:51:25 UTC
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?
Comment 6 nightstrike 2012-12-11 14:14:47 UTC
Tobias, what is your full configure line alongside --disable-nls?
Comment 7 Richard Biener 2012-12-21 11:35:46 UTC
(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?
Comment 8 Richard Biener 2012-12-21 11:42:21 UTC
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?)
Comment 9 Eric Botcazou 2012-12-21 11:57:41 UTC
> 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.
Comment 10 Richard Biener 2012-12-21 14:33:19 UTC
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
Comment 11 Richard Biener 2012-12-21 14:34:57 UTC
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.
Comment 12 ian@gcc.gnu.org 2012-12-21 15:59:36 UTC
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
Comment 13 Richard Biener 2013-01-02 14:40:17 UTC
I assume it is indeed fixed.  Please re-open if not.
Comment 14 Josue Andrade Gomes 2013-03-25 18:30:40 UTC
Not fixed. Still reproducible with final 4.8.0
See also http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56644
Comment 15 roland 2013-03-25 21:52:59 UTC
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.
Comment 16 Diego Novillo 2013-03-25 22:18:06 UTC
Re-opening on request from Roland.
Comment 17 Richard Biener 2013-03-26 08:52:38 UTC
Diego, can you please revert your change then on the trunk for now?
Comment 18 Diego Novillo 2013-03-26 10:48:18 UTC
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
Comment 19 roland 2013-03-26 22:03:44 UTC
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.
Comment 20 Jakub Jelinek 2013-04-29 09:25:29 UTC
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.
Comment 21 dnovillo@google.com 2013-04-29 16:46:27 UTC
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.
Comment 22 Diego Novillo 2013-05-07 15:07:54 UTC
Looking at this today.
Comment 23 Diego Novillo 2013-05-07 16:55:07 UTC
Created attachment 30046 [details]
Proposed fix.  Revert trunk commit r190487
Comment 24 Diego Novillo 2013-05-07 16:56:49 UTC
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.
Comment 25 roland 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.
Comment 26 dnovillo@google.com 2013-05-07 17:10:07 UTC
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?
Comment 27 rguenther@suse.de 2013-05-08 10:06:30 UTC
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.
Comment 28 dnovillo@google.com 2013-05-08 13:23:22 UTC
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.
Comment 29 Diego Novillo 2013-05-08 13:42:49 UTC
Fixed in gcc-4_8-branch at rev 198708 and trunk at rev 198711.
Comment 30 Dâniel Fraga 2013-09-08 00:54:54 UTC
(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.