Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Win64: Only use __builtin_setjmp/__builtin_longjmp when compiling with GCC #331

Merged
merged 1 commit into from
Oct 8, 2023

Conversation

qmfrederik
Copy link
Contributor

Using __builtin_setjmp when compiling with Clang on MSYS2 will result in compiler warnings and runtime crashes. It seems to work on GCC, though.

Compiler warning on Clang:

warning: incompatible pointer types passing 'jmp_buf' (aka 'struct _SETJMP_FLOAT128[16]') to parameter of type 'void **' [-Wincompatible-pointer-types]
                NS_DURING
                ^~~~~~~~~
note: expanded from macro 'NS_DURING'
                    if (!setjmp(NSLocalHandler.jumpState)) {
                                ^~~~~~~~~~~~~~~~~~~~~~~~
note: expanded from macro 'setjmp'
#define setjmp(X)       __builtin_setjmp(X)
                                         ^
1 warning generated.

@qmfrederik
Copy link
Contributor Author

Actually, I think the best approach may be to remove this alltogether? setjmp and __builtin_setjmp have slightly different semantics. Using __builtin_setjmp as a replacement for setjmp apparently breaks interoperability between GNUstep compiled using gcc and other software compiled with clang.

@rfm
Copy link
Contributor

rfm commented Oct 7, 2023

Yes, I see this is a hack specifically for the state of mingw64 as it was in 2016 ... so I agree that if current mingw64 is working without the hack we should simply remove it.

Using `__builtin_setjmp` when compiling with Clang on MSYS2 will result in compiler warnings and runtime crashes.
Additionally, all tests seem to pass on recent versions of MSYS2 without this workaround in place.

Compiler warning on Clang:

warning: incompatible pointer types passing 'jmp_buf' (aka 'struct _SETJMP_FLOAT128[16]') to parameter of type 'void **' [-Wincompatible-pointer-types]
                NS_DURING
                ^~~~~~~~~
note: expanded from macro 'NS_DURING'
                    if (!setjmp(NSLocalHandler.jumpState)) {
                                ^~~~~~~~~~~~~~~~~~~~~~~~
note: expanded from macro 'setjmp'
                                         ^
1 warning generated.
@qmfrederik
Copy link
Contributor Author

@rfm I compiled libs-base on the UCRT64 environment of MSYS2, using gcc as the compiler, and using the 'traditional'/setjmp-based exceptions (e.g. without -fexceptions and fojbc-exceptions). All tests (make check) passed. I ran the test suite of one of our applications, which passed as well.

That seems to be an indication that it's OK to remove this workaround, 7 years after it was introduced.

@rfm rfm merged commit 0d9e456 into gnustep:master Oct 8, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants