Better default for MinGW setjmp/longjmp

[svaarala]

Proposal from IRC: mamod/JavaScript-Duktape@91f0ef2.

Changes:

• For Windows target, when compiling with MinGW, use _builtin{setjmp,longjmp}() to avoid a potential crash issue
• Identify Cygwin target as "cygwin", not "windows", for clarity
• Releases entry

[Corion]

I guess that will work, but note that MinGW is not Cygwin. The MinGW compiler is used without and outside of the Cygwin framework and, for example, uses native filenames instead of /cygdrive/whatever.

I don't know what your configuration framework requires and how it makes the distinction between MinGW and Cygwin.

[svaarala]

It needs a way of detecting the compiler/platform from preprocessor defines (without #including files) and then whatever goes.

[svaarala]

@Corion Would setjmp() be a better default than __builtin_setjmp()? Is your environment missing both _setjmp() and setjmp() (that would seem odd)?

[svaarala]

Ok, so based on the link there's a potential crash issue in longjmp() while __builtin_longjmp() would work.

[Corion]

According to Stackoverflow and a Sourceforge project collecting #defines , the following should work to identify MinGW:

#ifdef MINGW32

On 64bit, the following also is enabled:

#ifdef MINGW64

Unfortunately, I don't know much about the differences between longjmp, _longjmp, __longjmp and __builtin_longjmp - I just took a solution I found and applied it here ;)

[denisdemaisbr]

C and C++ (lang/compillers) exceptions handlers differs, i.e., https://msdn.microsoft.com/pt-br/library/yz2ez4as(v=vs.110).aspx

Lua uses a tiny approach to handle it.

NOTE: i know, duktape are written in ANSI C, but, can be compilled using c++ [same lua]

[svaarala]

@denisdemaisbr The easiest approach would be to rely on ANSI setjmp()/longjmp() and any platform missing it would need to supply it separately. That was pretty much the original approach but there are various reasons (performance, signal handling) to use different variants on each platform. So ultimately I can only rely on the input from people working on each platform as to what the appropriate default (that is, suitable for the majority of users on that platform) would be.

Duktape does support C++ exceptions which allows proper C++ unwinding, -DDUK_USE_CPP_EXCEPTIONS. It's not the default because C++ environments don't always have exception support enabled.

[svaarala]

Ok, I've updated the pull so that MinGW with POSIX (Cygwin) and Windows targets should now use __builtin_setjmp/longjmp(). I don't actively use Windows so I'm not sure if the checks are correct so if anyone can test, that would be nice. But in principle:

• When compiling for Cygwin POSIX target, __CYGWIN__ should be set and _WIN32 etc not. Due to the ordering in platforms.yaml, this should be handled by config/platforms/platform_cygwin.h.in which has the custom setjmp/longjmp now (it's #ifdef'd for MinGW which should be pretty pointless but also doesn't harm).
• When compiling for Windows target, __CYGWIN__ should not be set, and _WIN32 should be set. This should be handled by config/platforms/platform_windows.h.in, which now has a MinGW specific setjmp/longjmp check.

If anyone can confirm whether this is correct, that'd be nice.

[svaarala]

Also one thing to consider: should config/platforms/platform_cygwin.h.in define the OS string as "cygwin" rather than "windows"? If so, a POSIX/Cygwin build would identify as "mingw cygwin".

[svaarala]

I happened to have an unused Windows 10 VM around so I quickly checked that Cygwin GCC doesn't identify as MinGW (= doesn't trigger DUK_F_MINGW) so if the setjmp() crash is confined to MinGW only, then it should only be necessary to add the MinGW check for config/platforms/platform_windows.h.in.

I've updated the diff to reflect this; config/platforms/platform_cygwin.h.in uses _setjmp()/_longjmp() as before, and now sets the OS string to "cygwin" and not "windows".

[svaarala]

So, as things stand now this pull should only affect Win32 + MinGW w.r.t. setjmp/longjmp. It'd be great if @Corion or @mamod could confirm that the pull works in practice for that combination - I'll merge once there is some confirmation of this.

[mamod]

@svaarala I've been checking this yesterday and I came to a conclusion that this may happened because of a bad design from my side, or maybe perl itself :P

I've been compiling Duktape along side with perl headers, perl redefines setjmp
# define setjmp PerlProc_setjmp and PerlProc_setjmp is perl's own Sigsetjmp implementation.

This is already confusing to me so I think this exact case is a major header pollution, separate the build process and linking duktape fixed the problem, at least for me, I'll check with @Corion and I'll try to test duktape compiling under MingW, I'll be commenting here if I find anything.