[PATCH] Fix build on MINGW on ARM64

Started by Lars Kanis11 months ago9 messages
#1Lars Kanis
lars@greiz-reinsdorf.de
1 attachment(s)

This patch limits the workaround of using __buildin_setjmp on the
Windows MINGW platform. This workaround is only necessary for legacy
MSVCRT based toolchain, but not for UCRT based. It is not available at
all on clang on ARM64 resulting in the following compiler error:

error: __builtin_longjmp is not supported for the current target

This patch is used since years in MSYS2 packages:
https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-postgresql/postgresql-14.0-use-mingw-setjmp-on-ucrt.patch

It is also used in ruby-pg to allow compiling for
aarch64-w64-windows-gnu: https://github.com/ged/ruby-pg/pull/626/files

It would be nice if this patch could be merged upstream.

--
Regards, Lars

Attachments:

0001-Use-workaround-of-__builtin_setjmp-only-on-MINGW-on-.patchapplication/octet-stream; name=0001-Use-workaround-of-__builtin_setjmp-only-on-MINGW-on-.patchDownload
From 746e8e250b265c40d9706f26560e02e8623f123f Mon Sep 17 00:00:00 2001
From: Lars Kanis <lars@greiz-reinsdorf.de>
Date: Fri, 31 Jan 2025 21:58:00 +0100
Subject: [PATCH] Use workaround of __builtin_setjmp only on MINGW on MSVCRT

Because it is not present on ARM64 on Windows and not necessary on any UCRT based toolchain.
---
 src/include/c.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/include/c.h b/src/include/c.h
index a14c631516..33792c860c 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -1312,19 +1312,19 @@ extern int	fdatasync(int fildes);
 /*
  * When there is no sigsetjmp, its functionality is provided by plain
  * setjmp.  We now support the case only on Windows.  However, it seems
- * that MinGW-64 has some longstanding issues in its setjmp support,
- * so on that toolchain we cheat and use gcc's builtins.
+ * that MinGW-64 on x86_64 has some longstanding issues in its setjmp
+ * support, so on that toolchain we cheat and use gcc's builtins.
  */
 #ifdef WIN32
-#ifdef __MINGW64__
+#if defined(__MINGW64__) && !defined(_UCRT)
 typedef intptr_t sigjmp_buf[5];
 #define sigsetjmp(x,y) __builtin_setjmp(x)
 #define siglongjmp __builtin_longjmp
-#else							/* !__MINGW64__ */
+#else							/* !defined(__MINGW64__) || defined(_UCRT) */
 #define sigjmp_buf jmp_buf
 #define sigsetjmp(x,y) setjmp(x)
 #define siglongjmp longjmp
-#endif							/* __MINGW64__ */
+#endif							/* defined(__MINGW64__) && !defined(_UCRT) */
 #endif							/* WIN32 */
 
 /* /port compatibility functions */
-- 
2.43.0

#2vignesh C
vignesh21@gmail.com
In reply to: Lars Kanis (#1)
Re: [PATCH] Fix build on MINGW on ARM64

On Sun, 2 Feb 2025 at 00:52, Lars Kanis <lars@greiz-reinsdorf.de> wrote:

This patch limits the workaround of using __buildin_setjmp on the
Windows MINGW platform. This workaround is only necessary for legacy
MSVCRT based toolchain, but not for UCRT based. It is not available at
all on clang on ARM64 resulting in the following compiler error:

error: __builtin_longjmp is not supported for the current target

This patch is used since years in MSYS2 packages:
https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-postgresql/postgresql-14.0-use-mingw-setjmp-on-ucrt.patch

It is also used in ruby-pg to allow compiling for
aarch64-w64-windows-gnu: https://github.com/ged/ruby-pg/pull/626/files

It would be nice if this patch could be merged upstream.

Are there any known issues with using __builtin_setjmp? I'm asking
because the comment mentions about the long standing issues in its
setjmp "However, it seems that MinGW-64 has some longstanding issues
in its setjmp support, so on that toolchain we cheat and use gcc's
builtins. Also few users have reported segfaults when using setjmp
with MinGW as in [1]https://stackoverflow.com/questions/53709069/setjmp-longjmp-in-x86-64-w64-mingw32.
[1]: https://stackoverflow.com/questions/53709069/setjmp-longjmp-in-x86-64-w64-mingw32

Regards,
Vignesh

#3Andrew Dunstan
andrew@dunslane.net
In reply to: vignesh C (#2)
Re: [PATCH] Fix build on MINGW on ARM64

On 2025-04-01 Tu 5:16 AM, vignesh C wrote:

On Sun, 2 Feb 2025 at 00:52, Lars Kanis <lars@greiz-reinsdorf.de> wrote:

This patch limits the workaround of using __buildin_setjmp on the
Windows MINGW platform. This workaround is only necessary for legacy
MSVCRT based toolchain, but not for UCRT based. It is not available at
all on clang on ARM64 resulting in the following compiler error:

error: __builtin_longjmp is not supported for the current target

This patch is used since years in MSYS2 packages:
https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-postgresql/postgresql-14.0-use-mingw-setjmp-on-ucrt.patch

It is also used in ruby-pg to allow compiling for
aarch64-w64-windows-gnu: https://github.com/ged/ruby-pg/pull/626/files

It would be nice if this patch could be merged upstream.

Are there any known issues with using __builtin_setjmp? I'm asking
because the comment mentions about the long standing issues in its
setjmp "However, it seems that MinGW-64 has some longstanding issues
in its setjmp support, so on that toolchain we cheat and use gcc's
builtins. Also few users have reported segfaults when using setjmp
with MinGW as in [1].
[1] - https://stackoverflow.com/questions/53709069/setjmp-longjmp-in-x86-64-w64-mingw32

That report is from quite a few years ago, so I'm not sure it really helps.

If one of you would add this to the next CF we could see how the CFbot
reacts to it. In general it looks sane.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

#4vignesh C
vignesh21@gmail.com
In reply to: Andrew Dunstan (#3)
Re: [PATCH] Fix build on MINGW on ARM64

On Tue, 1 Apr 2025 at 16:02, Andrew Dunstan <andrew@dunslane.net> wrote:

On 2025-04-01 Tu 5:16 AM, vignesh C wrote:

On Sun, 2 Feb 2025 at 00:52, Lars Kanis <lars@greiz-reinsdorf.de> wrote:

This patch limits the workaround of using __buildin_setjmp on the
Windows MINGW platform. This workaround is only necessary for legacy
MSVCRT based toolchain, but not for UCRT based. It is not available at
all on clang on ARM64 resulting in the following compiler error:

error: __builtin_longjmp is not supported for the current target

This patch is used since years in MSYS2 packages:
https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-postgresql/postgresql-14.0-use-mingw-setjmp-on-ucrt.patch

It is also used in ruby-pg to allow compiling for
aarch64-w64-windows-gnu: https://github.com/ged/ruby-pg/pull/626/files

It would be nice if this patch could be merged upstream.

Are there any known issues with using __builtin_setjmp? I'm asking
because the comment mentions about the long standing issues in its
setjmp "However, it seems that MinGW-64 has some longstanding issues
in its setjmp support, so on that toolchain we cheat and use gcc's
builtins. Also few users have reported segfaults when using setjmp
with MinGW as in [1].
[1] - https://stackoverflow.com/questions/53709069/setjmp-longjmp-in-x86-64-w64-mingw32

That report is from quite a few years ago, so I'm not sure it really helps.

If one of you would add this to the next CF we could see how the CFbot
reacts to it. In general it looks sane.

There is an existing CF entry for this at [1]https://commitfest.postgresql.org/patch/5610/. If no one picks this
till the end of this CF, we can move it to next CF.
[1]: https://commitfest.postgresql.org/patch/5610/

Regards,
Vignesh

#5Andrew Dunstan
andrew@dunslane.net
In reply to: vignesh C (#4)
Re: [PATCH] Fix build on MINGW on ARM64

On 2025-04-01 Tu 8:47 AM, vignesh C wrote:

On Tue, 1 Apr 2025 at 16:02, Andrew Dunstan <andrew@dunslane.net> wrote:

On 2025-04-01 Tu 5:16 AM, vignesh C wrote:

On Sun, 2 Feb 2025 at 00:52, Lars Kanis <lars@greiz-reinsdorf.de> wrote:

This patch limits the workaround of using __buildin_setjmp on the
Windows MINGW platform. This workaround is only necessary for legacy
MSVCRT based toolchain, but not for UCRT based. It is not available at
all on clang on ARM64 resulting in the following compiler error:

error: __builtin_longjmp is not supported for the current target

This patch is used since years in MSYS2 packages:
https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-postgresql/postgresql-14.0-use-mingw-setjmp-on-ucrt.patch

It is also used in ruby-pg to allow compiling for
aarch64-w64-windows-gnu: https://github.com/ged/ruby-pg/pull/626/files

It would be nice if this patch could be merged upstream.

Are there any known issues with using __builtin_setjmp? I'm asking
because the comment mentions about the long standing issues in its
setjmp "However, it seems that MinGW-64 has some longstanding issues
in its setjmp support, so on that toolchain we cheat and use gcc's
builtins. Also few users have reported segfaults when using setjmp
with MinGW as in [1].
[1] - https://stackoverflow.com/questions/53709069/setjmp-longjmp-in-x86-64-w64-mingw32

That report is from quite a few years ago, so I'm not sure it really helps.

If one of you would add this to the next CF we could see how the CFbot
reacts to it. In general it looks sane.

There is an existing CF entry for this at [1]. If no one picks this
till the end of this CF, we can move it to next CF.
[1] - https://commitfest.postgresql.org/patch/5610/

Somehow I missed that. OK, looks good, will commit shortly.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

#6Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#5)
Re: [PATCH] Fix build on MINGW on ARM64

On 2025-04-01 Tu 11:15 AM, Andrew Dunstan wrote:

On 2025-04-01 Tu 8:47 AM, vignesh C wrote:

On Tue, 1 Apr 2025 at 16:02, Andrew Dunstan <andrew@dunslane.net> wrote:

On 2025-04-01 Tu 5:16 AM, vignesh C wrote:

On Sun, 2 Feb 2025 at 00:52, Lars Kanis <lars@greiz-reinsdorf.de>
wrote:

This patch limits the workaround of using __buildin_setjmp on the
Windows MINGW platform. This workaround is only necessary for legacy
MSVCRT based toolchain, but not for UCRT based. It is not
available at
all on clang on ARM64 resulting in the following compiler error:

    error: __builtin_longjmp is not supported for the current target

This patch is used since years in MSYS2 packages:
https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-postgresql/postgresql-14.0-use-mingw-setjmp-on-ucrt.patch

It is also used in ruby-pg to allow compiling for
aarch64-w64-windows-gnu:
https://github.com/ged/ruby-pg/pull/626/files

It would be nice if this patch could be merged upstream.

Are there any known issues with using __builtin_setjmp? I'm asking
because the comment mentions about the long standing issues in its
setjmp "However, it seems that MinGW-64 has some longstanding issues
in its setjmp support,  so on that toolchain we cheat and use gcc's
builtins. Also few users have reported segfaults when using setjmp
with MinGW as in [1].
[1] -
https://stackoverflow.com/questions/53709069/setjmp-longjmp-in-x86-64-w64-mingw32

That report is from quite a few years ago, so I'm not sure it really
helps.

If one of you would add this to the next CF we could see how the CFbot
reacts to it. In general it looks sane.

There is an existing CF entry for this at [1]. If no one picks this
till the end of this CF, we can move it to next CF.
[1] - https://commitfest.postgresql.org/patch/5610/

Somehow I missed that. OK, looks good, will commit shortly.

Done

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#6)
Re: [PATCH] Fix build on MINGW on ARM64

Andrew Dunstan <andrew@dunslane.net> writes:

On 2025-04-01 Tu 11:15 AM, Andrew Dunstan wrote:

On 2025-04-01 Tu 8:47 AM, vignesh C wrote:

There is an existing CF entry for this at [1]. If no one picks this
till the end of this CF, we can move it to next CF.
[1] - https://commitfest.postgresql.org/patch/5610/

Somehow I missed that. OK, looks good, will commit shortly.

Done

fairywren has been failing rather horribly since this went in [1]https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=fairywren&amp;dt=2025-04-02%2007%3A06%3A04.
It's not certain of course that this commit broke it and not one
of the other ones that first appeared in that build -- but none
of the other ones look plausibly related.

regards, tom lane

[1]: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=fairywren&amp;dt=2025-04-02%2007%3A06%3A04

#8Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#7)
Re: [PATCH] Fix build on MINGW on ARM64

On 2025-04-07 Mo 12:58 AM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 2025-04-01 Tu 11:15 AM, Andrew Dunstan wrote:

On 2025-04-01 Tu 8:47 AM, vignesh C wrote:

There is an existing CF entry for this at [1]. If no one picks this
till the end of this CF, we can move it to next CF.
[1] - https://commitfest.postgresql.org/patch/5610/

Somehow I missed that. OK, looks good, will commit shortly.

Done

fairywren has been failing rather horribly since this went in [1].
It's not certain of course that this commit broke it and not one
of the other ones that first appeared in that build -- but none
of the other ones look plausibly related.

regards, tom lane

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=fairywren&amp;dt=2025-04-02%2007%3A06%3A04

Yes, that was indeed the problem. I have reverted the patch.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

#9Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#8)
Re: [PATCH] Fix build on MINGW on ARM64

On 2025-04-07 Mo 11:05 AM, Andrew Dunstan wrote:

On 2025-04-07 Mo 12:58 AM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 2025-04-01 Tu 11:15 AM, Andrew Dunstan wrote:

On 2025-04-01 Tu 8:47 AM, vignesh C wrote:

There is an existing CF entry for this at [1]. If no one picks this
till the end of this CF, we can move it to next CF.
[1] - https://commitfest.postgresql.org/patch/5610/

Somehow I missed that. OK, looks good, will commit shortly.

Done

fairywren has been failing rather horribly since this went in [1].
It's not certain of course that this commit broke it and not one
of the other ones that first appeared in that build -- but none
of the other ones look plausibly related.

            regards, tom lane

[1]
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=fairywren&amp;dt=2025-04-02%2007%3A06%3A04

Yes, that was indeed the problem. I have reverted the patch.

Lars,

I have marked the CF entry as Returned with Feedback - you can submit
again, but will need to show that this does not cause breakage on x86_64
using any msys2 toolchain.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com