[Bug bootstrap/108300] New: `abort()` macro cause bootstrap failure on *-w64-mingw32

[Bug bootstrap/108300] New: `abort()` macro cause bootstrap failure on *-w64-mingw32

[lh_mouse at 126 dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

            Bug ID: 108300
           Summary: `abort()` macro cause bootstrap failure on
                    *-w64-mingw32
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: bootstrap
          Assignee: unassigned at gcc dot gnu.org
          Reporter: lh_mouse at 126 dot com
  Target Milestone: ---

Recently, mingw-w64 has got updated <msxml.h> from Wine. GCC then ceases to
build:

```
../../gcc/gcc/system.h:791:30: error: expected identifier before string
constant
  791 | #define abort() fancy_abort (__FILE__, __LINE__, __FUNCTION__)
      |                              ^~~~~~~~
```

The reason is elaborated in this piece of log of #gcc on OFTC:

```
[16:51:28] <lh_ideapad> on Windows hosts, <windows.h> now involves <msxml.h>,
which brings a struct that has `abort()` as a member function, so if the macro
above is expanded there it will become nonsense.
[16:52:34] <lh_ideapad> if GCC does not use that struct (and a plenty more) the
standard way to prevent them from being included is to define
`WIN32_LEAN_AND_MEAN` before inclusion of <windows.h>.  unfortunately, not all
GCC source files do so.
[16:53:31] <lh_ideapad> there are about thirty to forty files that include
<windows.h> without defining `WIN32_LEAN_AND_MEAN`.  do you think it is a good
idea to patch them all?
[16:54:11] <lh_ideapad> this can be generated by an sed command, but it's gonna
be a large patch, touching files that I am not familiar with.
[16:55:03] <lh_ideapad> or, we can patch mingw-w64 msxml.h to `#undef abort`.
[16:57:22] <iains> can you fixincludes windows.h to add a wrapperr that defines
WIN32_LEAN_AND_MEAN and then include_next windows.h?
[16:58:11] <iains> (that means you do not have to patch the original file ..
you could also use fixincludes to do that too - but it seems maybe more
complicated than the wrapper)
[17:06:14] <iains> .. of course, that does not help if the files are sources
that need to be built by the bootstrap compiler - it’s only going to succeed if
they are all target-libs related.
[17:23:52] <iains> .. hmm but if they are sources in the gcc tree, I’d suppose
that windows.h should not be included directly - but via system.h where you
could place the #define before the inclusion.
[17:34:14] <Arsen> ugh, the wonders of macros
[17:34:25] <Arsen> maybe that can be a static inline fn instead?
[17:34:41] <Arsen> (or even just regular ol inline)
[18:08:19] <jwakely> yeah, GCC should not #define abort like that, it's
disgusting
[18:08:40] <jwakely> "but we've always done it like that" - time to stop
```

I create this PR for the record. In my opinion, including <windows.h> without
`WIN32_LEAN_AND_MEAN` pulls in tons of unnecessary stuff and is almost never
desired. This can be fixed by

```
sed -Ei 's,^( *)#( *)include <windows\.h>,\1#\2define WIN32_LEAN_AND_MEAN
1\n&,'  \
  $(grep --inc "*.[hc]" --inc "*.cc" -Flr "<windows.h>")
```

(Some source files already have it.)

[Bug bootstrap/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-01-05

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to LIU Hao from comment #0)
>   791 | #define abort() fancy_abort (__FILE__, __LINE__, __FUNCTION__)

The C++ standard says this is undefined.

We should do something like:

#ifdef __has_builtin
# define HAS_BUILTIN(X) __has_builtin(X)
#else
# define HAS_BUILTIN(X) 0
#endif

namespace gcc {
#if HAS_BUILTIN(__builtin_FILE) && HAS_BUILTIN(__builtin_LINE) \
  && HAS_BUILTIN(__builtin_FUNCTION)
__attribute__((noreturn))
inline void
abort(const char* file = __builtin_FILE(),
      long line = __builtin_LINE(),
      const char* func = __builtin_FUNCTION())
{ fancy_abort(file, line, func); }
#else
__attribute__((noreturn))
inline void
abort()
{ fancy_abort("<unavailable>", 0, "<unavailable>"); }
#endif
}

[Bug bootstrap/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[lh_mouse at 126 dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

--- Comment #2 from LIU Hao <lh_mouse at 126 dot com> ---
(In reply to Jonathan Wakely from comment #1)
> (In reply to LIU Hao from comment #0)
> >   791 | #define abort() fancy_abort (__FILE__, __LINE__, __FUNCTION__)
> 
> The C++ standard says this is undefined.
> 
> We should do something like:
> 

I was almost going to submit a patch for mingw-w64 headers. If you think this
can be fixed for GCC 13, that's very kind of you, and please consider
backporting.

[Bug middle-end/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|bootstrap                   |middle-end
           Keywords|                            |build

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This is remnant of when GCC was C code ...
I suspect there are other code like this too in system.h.

[Bug middle-end/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |i.nixman at autistici dot org

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 108313 has been marked as a duplicate of this bug. ***

[Bug middle-end/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[lh_mouse at 126 dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

--- Comment #5 from LIU Hao <lh_mouse at 126 dot com> ---
A quick and obvious fix is to add `CPPFLAGS='-DWIN32_LEAN_AND_MEAN'` when
configuring. Bootstrapped successfully on {i686,x86_64}-w64-mingw32.

I still think GCC source files should be patched.

[Bug middle-end/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Yong <jyong@gcc.gnu.org>:

https://gcc.gnu.org/g:902c755930326cb4405672aa3ea13c35c653cbff

commit r13-5055-g902c755930326cb4405672aa3ea13c35c653cbff
Author: LIU Hao <lh_mouse@126.com>
Date:   Fri Jan 6 23:18:15 2023 +0800

    Always define `WIN32_LEAN_AND_MEAN` before <windows.h>

    Recently, mingw-w64 has got updated <msxml.h> from Wine which is included
    indirectly by <windows.h> if `WIN32_LEAN_AND_MEAN` is not defined. The
    `IXMLDOMDocument` class has a member function named `abort()`, which gets
    affected by our `abort()` macro in "system.h".

    `WIN32_LEAN_AND_MEAN` should, nevertheless, always be defined. This
    can exclude 'APIs such as Cryptography, DDE, RPC, Shell, and Windows
    Sockets' [1], and speed up compilation of these files a bit.

    [1]
https://learn.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers

    gcc/

            PR middle-end/108300
            * config/xtensa/xtensa-dynconfig.c: Define `WIN32_LEAN_AND_MEAN`
            before <windows.h>.
            * diagnostic-color.cc: Likewise.
            * plugin.cc: Likewise.
            * prefix.cc: Likewise.

    gcc/ada/

            PR middle-end/108300
            * adaint.c: Define `WIN32_LEAN_AND_MEAN` before `#include
            <windows.h>`.
            * cio.c: Likewise.
            * ctrl_c.c: Likewise.
            * expect.c: Likewise.
            * gsocket.h: Likewise.
            * mingw32.h: Likewise.
            * mkdir.c: Likewise.
            * rtfinal.c: Likewise.
            * rtinit.c: Likewise.
            * seh_init.c: Likewise.
            * sysdep.c: Likewise.
            * terminals.c: Likewise.
            * tracebak.c: Likewise.

    gcc/jit/

            PR middle-end/108300
            * jit-w32.h: Define `WIN32_LEAN_AND_MEAN` before <windows.h>.

    libatomic/

            PR middle-end/108300
            * config/mingw/lock.c: Define `WIN32_LEAN_AND_MEAN` before
            <windows.h>.

    libffi/

            PR middle-end/108300
            * src/aarch64/ffi.c: Define `WIN32_LEAN_AND_MEAN` before
            <windows.h>.

    libgcc/

            PR middle-end/108300
            * config/i386/enable-execute-stack-mingw32.c: Define
            `WIN32_LEAN_AND_MEAN` before <windows.h>.
            * libgcc2.c: Likewise.
            * unwind-generic.h: Likewise.

    libgfortran/

            PR middle-end/108300
            * intrinsics/sleep.c: Define `WIN32_LEAN_AND_MEAN` before
            <windows.h>.

    libgomp/

            PR middle-end/108300
            * config/mingw32/proc.c: Define `WIN32_LEAN_AND_MEAN` before
            <windows.h>.

    libiberty/

            PR middle-end/108300
            * make-temp-file.c: Define `WIN32_LEAN_AND_MEAN` before
<windows.h>.
            * pex-win32.c: Likewise.

    libssp/

            PR middle-end/108300
            * ssp.c: Define `WIN32_LEAN_AND_MEAN` before <windows.h>.

    libstdc++-v3/

            PR middle-end/108300
            * src/c++11/system_error.cc: Define `WIN32_LEAN_AND_MEAN` before
            <windows.h>.
            * src/c++11/thread.cc: Likewise.
            * src/c++17/fs_ops.cc: Likewise.
            * src/filesystem/ops.cc: Likewise.

    libvtv/

            PR middle-end/108300
            * vtv_malloc.cc: Define `WIN32_LEAN_AND_MEAN` before <windows.h>.
            * vtv_rts.cc: Likewise.
            * vtv_utils.cc: Likewise.

[Bug middle-end/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[i.nixman at autistici dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

--- Comment #7 from niXman <i.nixman at autistici dot org> ---
(In reply to CVS Commits from comment #6)
> The master branch has been updated by Jonathan Yong <jyong@gcc.gnu.org>:
> 
> https://gcc.gnu.org/g:902c755930326cb4405672aa3ea13c35c653cbff
> 
> commit r13-5055-g902c755930326cb4405672aa3ea13c35c653cbff
> Author: LIU Hao <lh_mouse@126.com>
> Date:   Fri Jan 6 23:18:15 2023 +0800
> 
>     Always define `WIN32_LEAN_AND_MEAN` before <windows.h>
>     
>     Recently, mingw-w64 has got updated <msxml.h> from Wine which is included
>     indirectly by <windows.h> if `WIN32_LEAN_AND_MEAN` is not defined. The
>     `IXMLDOMDocument` class has a member function named `abort()`, which gets
>     affected by our `abort()` macro in "system.h".
>     
>     `WIN32_LEAN_AND_MEAN` should, nevertheless, always be defined. This
>     can exclude 'APIs such as Cryptography, DDE, RPC, Shell, and Windows
>     Sockets' [1], and speed up compilation of these files a bit.
>     
>     [1]
> https://learn.microsoft.com/en-us/windows/win32/winprog/using-the-windows-
> headers
>     
>     gcc/
>     
>             PR middle-end/108300
>             * config/xtensa/xtensa-dynconfig.c: Define `WIN32_LEAN_AND_MEAN`
>             before <windows.h>.
>             * diagnostic-color.cc: Likewise.
>             * plugin.cc: Likewise.
>             * prefix.cc: Likewise.
>     
>     gcc/ada/
>     
>             PR middle-end/108300
>             * adaint.c: Define `WIN32_LEAN_AND_MEAN` before `#include
>             <windows.h>`.
>             * cio.c: Likewise.
>             * ctrl_c.c: Likewise.
>             * expect.c: Likewise.
>             * gsocket.h: Likewise.
>             * mingw32.h: Likewise.
>             * mkdir.c: Likewise.
>             * rtfinal.c: Likewise.
>             * rtinit.c: Likewise.
>             * seh_init.c: Likewise.
>             * sysdep.c: Likewise.
>             * terminals.c: Likewise.
>             * tracebak.c: Likewise.
>     
>     gcc/jit/
>     
>             PR middle-end/108300
>             * jit-w32.h: Define `WIN32_LEAN_AND_MEAN` before <windows.h>.
>     
>     libatomic/
>     
>             PR middle-end/108300
>             * config/mingw/lock.c: Define `WIN32_LEAN_AND_MEAN` before
>             <windows.h>.
>     
>     libffi/
>     
>             PR middle-end/108300
>             * src/aarch64/ffi.c: Define `WIN32_LEAN_AND_MEAN` before
>             <windows.h>.
>     
>     libgcc/
>     
>             PR middle-end/108300
>             * config/i386/enable-execute-stack-mingw32.c: Define
>             `WIN32_LEAN_AND_MEAN` before <windows.h>.
>             * libgcc2.c: Likewise.
>             * unwind-generic.h: Likewise.
>     
>     libgfortran/
>     
>             PR middle-end/108300
>             * intrinsics/sleep.c: Define `WIN32_LEAN_AND_MEAN` before
>             <windows.h>.
>     
>     libgomp/
>     
>             PR middle-end/108300
>             * config/mingw32/proc.c: Define `WIN32_LEAN_AND_MEAN` before
>             <windows.h>.
>     
>     libiberty/
>     
>             PR middle-end/108300
>             * make-temp-file.c: Define `WIN32_LEAN_AND_MEAN` before
> <windows.h>.
>             * pex-win32.c: Likewise.
>     
>     libssp/
>     
>             PR middle-end/108300
>             * ssp.c: Define `WIN32_LEAN_AND_MEAN` before <windows.h>.
>     
>     libstdc++-v3/
>     
>             PR middle-end/108300
>             * src/c++11/system_error.cc: Define `WIN32_LEAN_AND_MEAN` before
>             <windows.h>.
>             * src/c++11/thread.cc: Likewise.
>             * src/c++17/fs_ops.cc: Likewise.
>             * src/filesystem/ops.cc: Likewise.
>     
>     libvtv/
>     
>             PR middle-end/108300
>             * vtv_malloc.cc: Define `WIN32_LEAN_AND_MEAN` before <windows.h>.
>             * vtv_rts.cc: Likewise.
>             * vtv_utils.cc: Likewise.

now it bootstrapped OK, thanks!

[Bug middle-end/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[lh_mouse at 126 dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

--- Comment #8 from LIU Hao <lh_mouse at 126 dot com> ---
The commit above just lets GCC bootstrap on Windows. The cause of this issue is
still there.

Maybe it's possible to replace all direct calls to `abort()` in gcc and libcpp
with `fancy_abort (__FILE__, __LINE__, __FUNCTION__)`, and eventually get rid
of that macro.

[Bug middle-end/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[arsen at aarsen dot me]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

--- Comment #9 from Arsen Arsenović <arsen at aarsen dot me> ---
(In reply to LIU Hao from comment #8)
> The commit above just lets GCC bootstrap on Windows. The cause of this issue
> is still there.
> 
> Maybe it's possible to replace all direct calls to `abort()` in gcc and
> libcpp with `fancy_abort (__FILE__, __LINE__, __FUNCTION__)`, and eventually
> get rid of that macro.

See Jonathans comment above.  I'll do that refactor this week, likely.

[Bug middle-end/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[lh_mouse at 126 dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

--- Comment #10 from LIU Hao <lh_mouse at 126 dot com> ---
(In reply to Arsen Arsenović from comment #9)
> (In reply to LIU Hao from comment #8)
> > The commit above just lets GCC bootstrap on Windows. The cause of this issue
> > is still there.
> > 
> > Maybe it's possible to replace all direct calls to `abort()` in gcc and
> > libcpp with `fancy_abort (__FILE__, __LINE__, __FUNCTION__)`, and eventually
> > get rid of that macro.
> 
> See Jonathans comment above.  I'll do that refactor this week, likely.

I think Jonathan's proposal makes it unnecessarily complex. Renaming `abort` to
`gcc_fancy_abort`, as well as all invocations accordingly, should be much
simpler than those inline functions and `__builtin_*` stuff.

[Bug middle-end/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[arsen at aarsen dot me]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

--- Comment #11 from Arsen Arsenović <arsen at aarsen dot me> ---
(In reply to LIU Hao from comment #10)
> I think Jonathan's proposal makes it unnecessarily complex. Renaming `abort`
> to `gcc_fancy_abort`, as well as all invocations accordingly, should be much
> simpler than those inline functions and `__builtin_*` stuff.

This feels like an increase in development overhead and potential for error. 
The approach described by Jonathan is pretty standard, and it doesn't really
have a cost at all.

[Bug middle-end/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[i.nixman at autistici dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

--- Comment #12 from niXman <i.nixman at autistici dot org> ---
guys, does anyone have an idea why I faced with the error on gcc-12.2.0 ?

```
../../../../src/gcc-12.2.0/libcpp/system.h:404:30: error: expected identifier
before string constant
  404 | #define abort() fancy_abort (__FILE__, __LINE__, __FUNCTION__)
      |                              ^~~~~~~~
../../../../src/gcc-12.2.0/libcpp/system.h:404:30: error: expected ',' or '...'
before string constant
../../../../src/gcc-12.2.0/libcpp/system.h:404:30: error: expected identifier
before string constant
  404 | #define abort() fancy_abort (__FILE__, __LINE__, __FUNCTION__)
      |                              ^~~~~~~~
../../../../src/gcc-12.2.0/libcpp/system.h:404:30: error: expected ',' or '...'
before string constant
make[3]: *** [Makefile:227: files.o] Error 1
```

until recently I successfully built this version, but now for some reason I get
that error...

[Bug middle-end/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[i.nixman at autistici dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

--- Comment #13 from niXman <i.nixman at autistici dot org> ---
could it be because now as host toolchain used a version which contains the
changes LIU Hao is talked about?

[Bug middle-end/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Yes. Your mingw headers have changed, adding <msxml.h> which is incompatible
with the 'abort' macro in the GCC sources.

[Bug middle-end/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[nightstrike at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

nightstrike <nightstrike at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nightstrike at gmail dot com

--- Comment #15 from nightstrike <nightstrike at gmail dot com> ---
Someone on irc (jakub?) suggested just changing all of the aborts to
gcc_unreachable. Is that a viable option?

[Bug middle-end/108300] `abort()` macro cause bootstrap failure on *-w64-mingw32

[i.nixman at autistici dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108300

--- Comment #16 from niXman <i.nixman at autistici dot org> ---
(In reply to nightstrike from comment #15)
> Someone on irc (jakub?) suggested just changing all of the aborts to
> gcc_unreachable. Is that a viable option?

I like that idea, I'm not sure it will be accepted...