diff mbox series

[v2] fix Ada bootstrap on Cygwin64 (PR bootstrap/94918)

Message ID CAM43=SMDKywAdrWVQDfYakPyfCgSjP3+TjT1gWpSXOWrrg6qoQ@mail.gmail.com
State New
Headers show
Series [v2] fix Ada bootstrap on Cygwin64 (PR bootstrap/94918) | expand

Commit Message

Mikael Pettersson Jan. 10, 2021, 10:44 a.m. UTC
This fixes a compilation error preventing bootstrap with Ada on
x86_64-pc-cygwin. See PR bootstrap/94918 for details.

Compared to the initial patch sent in May 2020, this v2 patch places
the fix in Ada's raise-gcc.c instead of the shared unwind-generic.h,
which should hopefully simplify getting it applied.

Tested by bootstrapping this and the preliminary workaround for
PR98590 on x86_64-pc-cygwin.

Ok for master and branches?

(Patch also attached to protect it against gmail formatting.)

gcc/ada/

2021-01-10  Mikael Pettersson  <mikpelinux@gmail.com>

        PR bootstrap/94918
        * raise-gcc.c: (__SEH__): Prevent windows.h from including
        x86intrin.h or emmintrin.h on Cygwin64.

--- gcc-11-20210103/gcc/ada/raise-gcc.c.~1~     2021-01-03
23:32:14.000000000 +0100
+++ gcc-11-20210103/gcc/ada/raise-gcc.c 2021-01-10 11:13:07.878685936 +0100
@@ -79,6 +79,15 @@ typedef char bool;
    (SJLJ or DWARF). We need a consistently named interface to import from
    a-except, so wrappers are defined here.  */

+#if defined (__SEH__) && !defined (__USING_SJLJ_EXCEPTIONS__) && \
+    defined (__CYGWIN__) && !defined (__CYGWIN32__) && !defined (IN_RTS)
+/* Note: windows.h (via unwind-generic.h) on cygwin-64 includes x86intrin.h
+   which uses malloc. That fails to compile if malloc is poisoned, i.e. if
+   !IN_RTS.  */
+#define _X86INTRIN_H_INCLUDED
+#define _EMMINTRIN_H_INCLUDED
+#endif
+
 #ifndef IN_RTS
   /* For gnat1/gnatbind compilation: cannot use unwind.h, as it is for the
      target. So mimic configure...

Comments

Arnaud Charlet Jan. 10, 2021, 10:57 a.m. UTC | #1
> This fixes a compilation error preventing bootstrap with Ada on
> x86_64-pc-cygwin. See PR bootstrap/94918 for details.
> 
> Compared to the initial patch sent in May 2020, this v2 patch places
> the fix in Ada's raise-gcc.c instead of the shared unwind-generic.h,
> which should hopefully simplify getting it applied.

Not sure why. Applying it there looks incomplete and kludgy, don't you
agree?

Arno

> gcc/ada/
> 
> 2021-01-10  Mikael Pettersson  <mikpelinux@gmail.com>
> 
>         PR bootstrap/94918
>         * raise-gcc.c: (__SEH__): Prevent windows.h from including
>         x86intrin.h or emmintrin.h on Cygwin64.
> 
> --- gcc-11-20210103/gcc/ada/raise-gcc.c.~1~     2021-01-03
> 23:32:14.000000000 +0100
> +++ gcc-11-20210103/gcc/ada/raise-gcc.c 2021-01-10 11:13:07.878685936 +0100
> @@ -79,6 +79,15 @@ typedef char bool;
>     (SJLJ or DWARF). We need a consistently named interface to import from
>     a-except, so wrappers are defined here.  */
> 
> +#if defined (__SEH__) && !defined (__USING_SJLJ_EXCEPTIONS__) && \
> +    defined (__CYGWIN__) && !defined (__CYGWIN32__) && !defined (IN_RTS)
> +/* Note: windows.h (via unwind-generic.h) on cygwin-64 includes x86intrin.h
> +   which uses malloc. That fails to compile if malloc is poisoned, i.e. if
> +   !IN_RTS.  */
> +#define _X86INTRIN_H_INCLUDED
> +#define _EMMINTRIN_H_INCLUDED
> +#endif
> +
>  #ifndef IN_RTS
>    /* For gnat1/gnatbind compilation: cannot use unwind.h, as it is for the
>       target. So mimic configure...
Mikael Pettersson Jan. 10, 2021, 1:04 p.m. UTC | #2
On Sun, Jan 10, 2021 at 11:57 AM Arnaud Charlet <charlet@adacore.com> wrote:
>
> > This fixes a compilation error preventing bootstrap with Ada on
> > x86_64-pc-cygwin. See PR bootstrap/94918 for details.
> >
> > Compared to the initial patch sent in May 2020, this v2 patch places
> > the fix in Ada's raise-gcc.c instead of the shared unwind-generic.h,
> > which should hopefully simplify getting it applied.
>
> Not sure why. Applying it there looks incomplete and kludgy, don't you
> agree?

Ok, then here's v3 which places the fix in unwind-generic.h. The fix
matches what Ada's mingw32.h does.

Tested by bootstrapping this and the preliminary workaround for
PR98590 on x86_64-pc-cygwin.

Ok for master and branches?

libgcc/

2021-01-10  Mikael Pettersson  <mikpelinux@gmail.com>

        PR bootstrap/94918
        * unwind-generic.h (__SEH__): Prevent windows.h from including
        x86intrin.h or emmintrin.h on Cygwin64.

--- gcc-11-20210103/libgcc/unwind-generic.h.~1~ 2021-01-03
23:32:20.000000000 +0100
+++ gcc-11-20210103/libgcc/unwind-generic.h     2021-01-09
14:51:16.262378715 +0100
@@ -30,6 +30,12 @@

 #if defined (__SEH__) && !defined (__USING_SJLJ_EXCEPTIONS__)
 /* Only for _GCC_specific_handler.  */
+#if defined (__CYGWIN__) && !defined (__CYGWIN32__) && !defined (IN_RTS)
+/* Note: windows.h on cygwin-64 includes x86intrin.h which uses malloc.
+   That fails to compile, if malloc is poisoned, i.e. if !IN_RTS.  */
+#define _X86INTRIN_H_INCLUDED
+#define _EMMINTRIN_H_INCLUDED
+#endif
 #include <windows.h>
 #endif
Mikael Pettersson March 7, 2021, 5:16 p.m. UTC | #3
On Sun, Jan 10, 2021 at 2:04 PM Mikael Pettersson <mikpelinux@gmail.com> wrote:
>
> On Sun, Jan 10, 2021 at 11:57 AM Arnaud Charlet <charlet@adacore.com> wrote:
> >
> > > This fixes a compilation error preventing bootstrap with Ada on
> > > x86_64-pc-cygwin. See PR bootstrap/94918 for details.
> > >
> > > Compared to the initial patch sent in May 2020, this v2 patch places
> > > the fix in Ada's raise-gcc.c instead of the shared unwind-generic.h,
> > > which should hopefully simplify getting it applied.
> >
> > Not sure why. Applying it there looks incomplete and kludgy, don't you
> > agree?
>
> Ok, then here's v3 which places the fix in unwind-generic.h. The fix
> matches what Ada's mingw32.h does.
>
> Tested by bootstrapping this and the preliminary workaround for
> PR98590 on x86_64-pc-cygwin.
>
> Ok for master and branches?

Ping. This is currently the only blocker for bootstrapping gcc-11 with
Ada on Cygwin64.

>
> libgcc/
>
> 2021-01-10  Mikael Pettersson  <mikpelinux@gmail.com>
>
>         PR bootstrap/94918
>         * unwind-generic.h (__SEH__): Prevent windows.h from including
>         x86intrin.h or emmintrin.h on Cygwin64.
>
> --- gcc-11-20210103/libgcc/unwind-generic.h.~1~ 2021-01-03
> 23:32:20.000000000 +0100
> +++ gcc-11-20210103/libgcc/unwind-generic.h     2021-01-09
> 14:51:16.262378715 +0100
> @@ -30,6 +30,12 @@
>
>  #if defined (__SEH__) && !defined (__USING_SJLJ_EXCEPTIONS__)
>  /* Only for _GCC_specific_handler.  */
> +#if defined (__CYGWIN__) && !defined (__CYGWIN32__) && !defined (IN_RTS)
> +/* Note: windows.h on cygwin-64 includes x86intrin.h which uses malloc.
> +   That fails to compile, if malloc is poisoned, i.e. if !IN_RTS.  */
> +#define _X86INTRIN_H_INCLUDED
> +#define _EMMINTRIN_H_INCLUDED
> +#endif
>  #include <windows.h>
>  #endif
Arnaud Charlet March 7, 2021, 6:20 p.m. UTC | #4
> > > > This fixes a compilation error preventing bootstrap with Ada on
> > > > x86_64-pc-cygwin. See PR bootstrap/94918 for details.
> > > >
> > > > Compared to the initial patch sent in May 2020, this v2 patch places
> > > > the fix in Ada's raise-gcc.c instead of the shared unwind-generic.h,
> > > > which should hopefully simplify getting it applied.
> > >
> > > Not sure why. Applying it there looks incomplete and kludgy, don't you
> > > agree?
> >
> > Ok, then here's v3 which places the fix in unwind-generic.h. The fix
> > matches what Ada's mingw32.h does.
> >
> > Tested by bootstrapping this and the preliminary workaround for
> > PR98590 on x86_64-pc-cygwin.
> >
> > Ok for master and branches?
> 
> Ping. This is currently the only blocker for bootstrapping gcc-11 with
> Ada on Cygwin64.

Unfortunately changes in libgcc are outside my area, you'll need to find
another maintainer for this one.

> > libgcc/
> >
> > 2021-01-10  Mikael Pettersson  <mikpelinux@gmail.com>
> >
> >         PR bootstrap/94918
> >         * unwind-generic.h (__SEH__): Prevent windows.h from including
> >         x86intrin.h or emmintrin.h on Cygwin64.
> >
> > --- gcc-11-20210103/libgcc/unwind-generic.h.~1~ 2021-01-03
> > 23:32:20.000000000 +0100
> > +++ gcc-11-20210103/libgcc/unwind-generic.h     2021-01-09
> > 14:51:16.262378715 +0100
> > @@ -30,6 +30,12 @@
> >
> >  #if defined (__SEH__) && !defined (__USING_SJLJ_EXCEPTIONS__)
> >  /* Only for _GCC_specific_handler.  */
> > +#if defined (__CYGWIN__) && !defined (__CYGWIN32__) && !defined (IN_RTS)
> > +/* Note: windows.h on cygwin-64 includes x86intrin.h which uses malloc.
> > +   That fails to compile, if malloc is poisoned, i.e. if !IN_RTS.  */
> > +#define _X86INTRIN_H_INCLUDED
> > +#define _EMMINTRIN_H_INCLUDED
> > +#endif
> >  #include <windows.h>
> >  #endif
>
Richard Biener March 8, 2021, 10:21 a.m. UTC | #5
On Sun, Jan 10, 2021 at 2:05 PM Mikael Pettersson via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Sun, Jan 10, 2021 at 11:57 AM Arnaud Charlet <charlet@adacore.com> wrote:
> >
> > > This fixes a compilation error preventing bootstrap with Ada on
> > > x86_64-pc-cygwin. See PR bootstrap/94918 for details.
> > >
> > > Compared to the initial patch sent in May 2020, this v2 patch places
> > > the fix in Ada's raise-gcc.c instead of the shared unwind-generic.h,
> > > which should hopefully simplify getting it applied.
> >
> > Not sure why. Applying it there looks incomplete and kludgy, don't you
> > agree?
>
> Ok, then here's v3 which places the fix in unwind-generic.h. The fix
> matches what Ada's mingw32.h does.
>
> Tested by bootstrapping this and the preliminary workaround for
> PR98590 on x86_64-pc-cygwin.
>
> Ok for master and branches?

I wonder why we include <windows.h> from this file at all,
and why it is not included from {t,}system.h instead which
is where system header specific fixups should be made
(and this one could be avoided because system headers
are included _before_ poisoning anything).

The include was added by Tristan, well - probably just merged by him.

Eric?

> libgcc/
>
> 2021-01-10  Mikael Pettersson  <mikpelinux@gmail.com>
>
>         PR bootstrap/94918
>         * unwind-generic.h (__SEH__): Prevent windows.h from including
>         x86intrin.h or emmintrin.h on Cygwin64.
>
> --- gcc-11-20210103/libgcc/unwind-generic.h.~1~ 2021-01-03
> 23:32:20.000000000 +0100
> +++ gcc-11-20210103/libgcc/unwind-generic.h     2021-01-09
> 14:51:16.262378715 +0100
> @@ -30,6 +30,12 @@
>
>  #if defined (__SEH__) && !defined (__USING_SJLJ_EXCEPTIONS__)
>  /* Only for _GCC_specific_handler.  */
> +#if defined (__CYGWIN__) && !defined (__CYGWIN32__) && !defined (IN_RTS)
> +/* Note: windows.h on cygwin-64 includes x86intrin.h which uses malloc.
> +   That fails to compile, if malloc is poisoned, i.e. if !IN_RTS.  */
> +#define _X86INTRIN_H_INCLUDED
> +#define _EMMINTRIN_H_INCLUDED
> +#endif
>  #include <windows.h>
>  #endif
Eric Botcazou March 8, 2021, 11:07 a.m. UTC | #6
> I wonder why we include <windows.h> from this file at all,
> and why it is not included from {t,}system.h instead which
> is where system header specific fixups should be made
> (and this one could be avoided because system headers
> are included _before_ poisoning anything).

<windows.h> is the Mother of All Things on the platform so you don't want to 
include it liberally (although it can be tamed e.g. with WIN32_LEAN_AND_MEAN).
Therefore including it from tsystem.h might be worse than the actual disease.

Mikael, can you work around the problem by adding

#ifdef __CYGWIN__
#include "mingw32.h"
#endif

at the appropriate spot in raise-gcc.c instead?
Mikael Pettersson March 8, 2021, 9:43 p.m. UTC | #7
On Mon, Mar 8, 2021 at 12:07 PM Eric Botcazou <botcazou@adacore.com> wrote:
>
> > I wonder why we include <windows.h> from this file at all,
> > and why it is not included from {t,}system.h instead which
> > is where system header specific fixups should be made
> > (and this one could be avoided because system headers
> > are included _before_ poisoning anything).
>
> <windows.h> is the Mother of All Things on the platform so you don't want to
> include it liberally (although it can be tamed e.g. with WIN32_LEAN_AND_MEAN).
> Therefore including it from tsystem.h might be worse than the actual disease.
>
> Mikael, can you work around the problem by adding
>
> #ifdef __CYGWIN__
> #include "mingw32.h"
> #endif
>
> at the appropriate spot in raise-gcc.c instead?

This one worked. Is that what you had in mind?

        * raise-gcc.c: On Cygwin include mingw32.h to prevent
        windows.h from including x86intrin.h or emmintrin.h.
---
 gcc/ada/raise-gcc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gcc/ada/raise-gcc.c b/gcc/ada/raise-gcc.c
index 1446bfaaeb7..b096eba1b75 100644
--- a/gcc/ada/raise-gcc.c
+++ b/gcc/ada/raise-gcc.c
@@ -79,6 +79,12 @@ typedef char bool;
    (SJLJ or DWARF). We need a consistently named interface to import from
    a-except, so wrappers are defined here.  */

+#ifdef __CYGWIN__
+/* Prevent compile error due to unwind-generic.h including <windows.h>,
+   see comment above #include <windows.h> in mingw32.h.  */
+#include "mingw32.h"
+#endif
+
 #ifndef IN_RTS
   /* For gnat1/gnatbind compilation: cannot use unwind.h, as it is for the
      target. So mimic configure...
Eric Botcazou March 8, 2021, 10:21 p.m. UTC | #8
> This one worked. Is that what you had in mind?
> 
>         * raise-gcc.c: On Cygwin include mingw32.h to prevent
>         windows.h from including x86intrin.h or emmintrin.h.

Yep, exactly, thanks, you may put it on whichever branch you need.
Jeff Law March 9, 2021, 4 p.m. UTC | #9
On 3/8/21 3:21 PM, Eric Botcazou wrote:
>> This one worked. Is that what you had in mind?
>>
>>         * raise-gcc.c: On Cygwin include mingw32.h to prevent
>>         windows.h from including x86intrin.h or emmintrin.h.
> Yep, exactly, thanks, you may put it on whichever branch you need.
I've pushed it to the trunk.
jeff
diff mbox series

Patch

gcc/ada/

2021-01-10  Mikael Pettersson  <mikpelinux@gmail.com>

	PR bootstrap/94918
	* raise-gcc.c: (__SEH__): Prevent windows.h from including
	x86intrin.h or emmintrin.h on Cygwin64.

--- gcc-11-20210103/gcc/ada/raise-gcc.c.~1~	2021-01-03 23:32:14.000000000 +0100
+++ gcc-11-20210103/gcc/ada/raise-gcc.c	2021-01-10 11:13:07.878685936 +0100
@@ -79,6 +79,15 @@  typedef char bool;
    (SJLJ or DWARF). We need a consistently named interface to import from
    a-except, so wrappers are defined here.  */
 
+#if defined (__SEH__) && !defined (__USING_SJLJ_EXCEPTIONS__) && \
+    defined (__CYGWIN__) && !defined (__CYGWIN32__) && !defined (IN_RTS)
+/* Note: windows.h (via unwind-generic.h) on cygwin-64 includes x86intrin.h
+   which uses malloc. That fails to compile if malloc is poisoned, i.e. if
+   !IN_RTS.  */
+#define _X86INTRIN_H_INCLUDED
+#define _EMMINTRIN_H_INCLUDED
+#endif
+
 #ifndef IN_RTS
   /* For gnat1/gnatbind compilation: cannot use unwind.h, as it is for the
      target. So mimic configure...