[WIN32] 64-bit long double build crashes in op/pack.t (was #125863)

[p5pRT]

Migrated from rt.perl.org#125924 (status was 'open')

Searchable as RT125924$

[p5pRT]

From @sisyphus

Hi,

Tony Cook noted in #125863 that op/pack.t crashes because of a problem with
modfl()​:

It's crashing inside the libraries modfl() implementation​:

J​:\dev\perl\git\perl>gcc -D__USE_MINGW_ANSI_STDIO -otest.exe -O0 -g test.c

J​:\dev\perl\git\perl>test (this crashed)

J​:\dev\perl\git\perl>gdb test
GNU gdb (GDB) 7.8.1
Copyright (C) 2014 Free Software Foundation, Inc.
License GPLv3+​: GNU GPL version 3 or later
<http​://gnu.org/licenses/gpl.html>
This is free software​: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-w64-mingw32".
Type "show configuration" for configuration details.
For bug reporting instructions, please see​:
<http​://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at​:
<http​://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from test...done.
(gdb) r
Starting program​: J​:\dev\perl\git\perl\test.exe
[New Thread 1224.0x220c]

Program received signal SIGSEGV, Segmentation fault.
0x0000000000402be5 in modfl ()
(gdb) bt
#0 0x0000000000402be5 in modfl ()
#1 0x000000000040156c in main () at test.c​:11
(gdb) q
A debugging session is active.

    Inferior 1 \[process 1224\] will be killed\.

Quit anyway? (y or n) y

J​:\dev\perl\git\perl>type test.c
#include <stdio.h>
#include <math.h>

long double d = 100.0;

long double anv = 36893488147419103232.0F;
long double cdouble = 36893488147419103232.0F;

int main() {
long double y;
long double x = modfl(cdouble / anv, &y); // line 11
printf("x %Lg y %Lg\n", x, y);
return 0;
}

The code in modfl() is using %rax to store the address passed in then uses
it for an intermediate result.

I suspect the inline assembly found​:

http​://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-crt/math/modfl.c

needs a clobber entry to mark %eax as being modified by the code.

However, this issue is not related to the original subject of #125863, so
I'm opening a separate ticket for it.

It's a problem that affects 64-bit gcc-4.9.2 -Duselongdouble builds of
perl-5.23.2 on Windows, where the mingw runtime version is 4.0 (which is the
current mingw runtime version).

As a workaround, I've successfully used the attached patch to pp_pack.c
(applied to the pp_pack.c that shipped with perl-5.23.2).

With this patch in place, there are no crashes in op/pack.t.
Tests 13177 and 13180 are the only tests that fail (as is also the case with
32-bit mingw -Duselongdouble builds).

The patch applies only when nvtype is 'long double' && the mingw compiler is
64-bit && mingw runtime version is 4.x.
I believe these are the appropriate conditions, but this has not been
rigorously proven. (For example, I haven't built with a 64-bit gcc-4.9.2
that uses the older 3.1 mingw runtime.)

I'm not sure whether we should be specifying "mingw runtime version is 4.x"
or "mingw runtime version is 4.0" .... the main problem being that, at this
stage, we don't know if this problem will be fixed in version 4.1.
If someone wanted to add the condition "&& __MINGW64_VERSION_MINOR == 0"
that would be fine by me.

Any thoughts/concerns re the patch ?

Cheers,
Rob

[p5pRT]

From @sisyphus

pp_pack.c.patch

--- pp_pack.c_orig	2015-08-27 21:36:40 +1000
+++ pp_pack.c	2015-08-27 22:10:52 +1000
@@ -1767,7 +1767,15 @@
 		}
 		while (cdouble < 0.0)
 		    cdouble += anv;
+#if defined(USE_LONG_DOUBLE) && defined(__MINGW64__) && __MINGW64_VERSION_MAJOR == 4
+   /* modfl() segfaults for -Duselongdouble && 64-bit mingw && mingw runtime version 4 */
+                cdouble /= anv;
+                if(cdouble >= 0) trouble = floorl(cdouble);
+                else trouble = ceill(cdouble);
+                cdouble -= trouble;
+#else
 		cdouble = Perl_modf(cdouble / anv, &trouble);
+#endif
 #ifdef LONGDOUBLE_DOUBLEDOUBLE
                 /* Workaround for powerpc doubledouble modfl bug:
                  * close to 1.0L and -1.0L cdouble is 0, and trouble

[p5pRT]

From @bulk88

On Thu Aug 27 06​:27​:16 2015, sisyphus wrote​:

However, this issue is not related to the original subject of #125863,
so
I'm opening a separate ticket for it.

It's a problem that affects 64-bit gcc-4.9.2 -Duselongdouble builds of
perl-5.23.2 on Windows, where the mingw runtime version is 4.0 (which
is the
current mingw runtime version).

As a workaround, I've successfully used the attached patch to
pp_pack.c
(applied to the pp_pack.c that shipped with perl-5.23.2).

With this patch in place, there are no crashes in op/pack.t.
Tests 13177 and 13180 are the only tests that fail (as is also the
case with
32-bit mingw -Duselongdouble builds).

The patch applies only when nvtype is 'long double' && the mingw
compiler is
64-bit && mingw runtime version is 4.x.
I believe these are the appropriate conditions, but this has not been
rigorously proven. (For example, I haven't built with a 64-bit gcc-
4.9.2
that uses the older 3.1 mingw runtime.)

I'm not sure whether we should be specifying "mingw runtime version is
4.x"
or "mingw runtime version is 4.0" .... the main problem being that, at
this
stage, we don't know if this problem will be fixed in version 4.1.
If someone wanted to add the condition "&& __MINGW64_VERSION_MINOR ==
0"
that would be fine by me.

Any thoughts/concerns re the patch ?

Cheers,
Rob

What about mingw.org CCs? is there a bug ticket filed with mingw64 about this? The important thing about a version check is, when will W64 fix this and will they ever bump __MINGW64_VERSION_MAJOR? You don't want a deoptimized version in perl core, to stay forever in the code since it was forgotten about, even though mingw64 fixed the bug years ago. I will mention mingw64 changed the calling convention of libstdc++-6.dll between 4.6 and 4.8 from cdecl to thiscall, but never renamed the DLL, making it impossible to have 4.6 and 4.8 GCC DLLs in the same process, so I presume they are very conservative about version bumps.

--
bulk88 ~ bulk88 at hotmail.com

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From @sisyphus

-----Original Message-----
From​: bulk88 via RT
Sent​: Friday, August 28, 2015 1​:30 PM
To​: OtherRecipients of perl Ticket #125924​:
Cc​: perl5-porters@​perl.org
Subject​: [perl #125924] [WIN32] 64-bit long double build crashes in
op/pack.t (was #125863)

Any thoughts/concerns re the patch ?

What about mingw.org CCs?

If there are mingw.org compilers that have this bug, then the patch will not
provide a fix for them.
I think it likely that it's only the 64-bit mingw64.sf compilers running
mingw runtime version 4 that suffer this bug.
But if that's not so, then we'll obviously need to alter the conditions when
(if) the other reports come through. (I'm not about to go hunting through
mingw.org compilers to see if there are any problems there - I'll leave that
to those that care about mingw.org compilers.)

The compiler/runtime combo's I have checked are​:
a) 32-bit mingw64 gcc-4.8.2, runtime version 3.1;
b) 64-bit mingw64 gcc-4.8.2, runtime version 3.1;
c) 32-bit mingw64 gcc-4.9.2, runtime version 4.0;
d) 64-bit mingw64 gcc-4.9.2, runtime version 4.0.

The only one of those that needs the patch is d).

I probably ought to at least verify that the patch is *not* needed for
64-bit mingw64 gcc-4.9.2, runtime version 3.1.
I'll check that tonight.

is there a bug ticket filed with mingw64 about this?

I don't know. I did mention in the #125863 discussion that I would
investigate this and file a bug report there if none was already present.
And I will do that tonight - and report back.
(I don't expect those beginning with *this* bug report to have read that
discussion - so it's my bad that I didn't mention this in my previous post.)

The important thing about a version check is, when will W64 fix this and
will they ever bump __MINGW64_VERSION_MAJOR? You don't want a deoptimized
version in perl core, to stay forever in the code since it was forgotten
about

Exactly. This bothers me a bit.
Perhaps we *should* add the condition "&& __MINGW64_VERSION_MINOR == 0".
Worst thing that could then happen with that is if the next mingw runtime
version hasn't fixed the problem - in which case we just have to alter the
conditions again.

But that's certainly better than having de-optimised code being run when
it's no longer needed. (And, in any case, one's hunch would be that this
problem *will* be fixed in the next runtime.)
So .... yeah, I'm now more comfortable with the addition of the "&&
__MINGW64_VERSION_MINOR == 0" condition.
Unless there's a better alternative ?

Cheers,
Rob

[p5pRT]

From @sisyphus

-----Original Message-----
From​: sisyphus1@​optusnet.com.au
Sent​: Friday, August 28, 2015 2​:32 PM
To​: perlbug-followup@​perl.org
Cc​: perl5-porters@​perl.org
Subject​: Re​: [perl #125924] [WIN32] 64-bit long double build crashes in
op/pack.t (was #125863)

-----Original Message-----
From​: bulk88 via RT
Sent​: Friday, August 28, 2015 1​:30 PM
To​: OtherRecipients of perl Ticket #125924​:
Cc​: perl5-porters@​perl.org
Subject​: [perl #125924] [WIN32] 64-bit long double build crashes in
op/pack.t (was #125863)

The compiler/runtime combo's I have checked are​:
a) 32-bit mingw64 gcc-4.8.2, runtime version 3.1;
b) 64-bit mingw64 gcc-4.8.2, runtime version 3.1;
c) 32-bit mingw64 gcc-4.9.2, runtime version 4.0;
d) 64-bit mingw64 gcc-4.9.2, runtime version 4.0.

The only one of those that needs the patch is d).

I probably ought to at least verify that the patch is *not* needed for
64-bit mingw64 gcc-4.9.2, runtime version 3.1.
I'll check that tonight.

Well, I don't think there's much doubt that the problem arises only with
mingw runtime version 4.0 - but I tried gcc-4.9.2, runtime version 3.4 (as I
have it on hand).
No crash (or any other problems) running Tony's test script using that
compiler.

is there a bug ticket filed with mingw64 about this?

I don't know. I did mention in the #125863 discussion that I would
investigate this and file a bug report there if none was already present.

There's a bug report for this (created 2015-05-07) at
http​://sourceforge.net/p/mingw-w64/bugs/478/

A patch to the mingw runtime is also available​:
http​://sourceforge.net/p/mingw-w64/bugs/_discuss/thread/6ff78738/608c/attachment/mingw-w64-v4.0.2.patch

Cheers,
Rob

[p5pRT]

From @sisyphus

-----Original Message-----
From​: sisyphus1@​optusnet.com.au
Sent​: Friday, August 28, 2015 2​:32 PM
To​: perlbug-followup@​perl.org
Cc​: perl5-porters@​perl.org
Subject​: Re​: [perl #125924] [WIN32] 64-bit long double build crashes in
op/pack.t (was #125863)

-----Original Message-----
From​: bulk88 via RT
Sent​: Friday, August 28, 2015 1​:30 PM
To​: OtherRecipients of perl Ticket #125924​:
Cc​: perl5-porters@​perl.org
Subject​: [perl #125924] [WIN32] 64-bit long double build crashes in
op/pack.t (was #125863)

The important thing about a version check is, when will W64 fix this and
will they ever bump __MINGW64_VERSION_MAJOR? You don't want a deoptimized
version in perl core, to stay forever in the code since it was forgotten
about

Exactly. This bothers me a bit.
Perhaps we *should* add the condition "&& __MINGW64_VERSION_MINOR == 0".

Ok ... so I've rewritten that patch to include the condition "&&
__MINGW64_VERSION_MINOR == 0".
The rewrite is attached as pp_pack.c.patch2. (Works fine for me.)

But *then* I found that the crash in ext/POSIX/t/math.t occurs for the very
same reason - so I applied the same changes to ext/POSIX/POSIX.xs.
This patch is attached as POSIX.xs.patch. (Also works fine for me.)

But *now* I'm wondering whether I should instead be attacking this issue in
perl.h, where Perl_modf is defined.

Thoughts ??

I now get no crashes during the test suite, and I think the 2 attached
patches cover all issues with Perl_modf for the moment .... but that's no
guarantee for the future (when it's quite possible, I guess, that additional
Perl_modf calls could be added to the perl source).

Cheers,
Rob

[p5pRT]

From @sisyphus

pp_pack.c.patch2

[p5pRT]

From @sisyphus

POSIX.xs.patch

--- POSIX.xs_orig	2015-09-01 20:41:42 +1000
+++ POSIX.xs	2015-09-01 21:21:53 +1000
@@ -2864,8 +2864,17 @@
 	NV		x
     PPCODE:
 	NV intvar;
+#if defined(USE_LONG_DOUBLE) && defined(__MINGW64__) \
+    && __MINGW64_VERSION_MAJOR == 4 && __MINGW64_VERSION_MINOR == 0
+   /* modfl() segfaults for -Duselongdouble && 64-bit mingw64 && mingw runtime version 4.0 */
+        if(x >= 0) intvar = floorl(x);
+        else intvar = ceill(x);
+        x -= intvar;
 	/* (We already know stack is long enough.) */
+        PUSHs(sv_2mortal(newSVnv(x)));
+#else
 	PUSHs(sv_2mortal(newSVnv(Perl_modf(x,&intvar)))); /* C89 math */
+#endif
 	PUSHs(sv_2mortal(newSVnv(intvar)));
 
 void

[p5pRT]

From @tonycoz

On Tue Sep 01 05​:12​:53 2015, sisyphus wrote​:

+ if(cdouble >= 0) trouble = floorl(cdouble);
+ else trouble = ceill(cdouble);

could be​:

  trouble = truncl(cdouble);

As to providing our own modfl() implementation, the main issue is we'd need to make sure it's exported so POSIX (or other XS modules) could see it.

Though we could add it to inline.h

Tony

[p5pRT]

From @sisyphus

-----Original Message-----
From​: Tony Cook via RT
Sent​: Wednesday, September 02, 2015 10​:59 AM
To​: OtherRecipients of perl Ticket #125924​:
Cc​: perl5-porters@​perl.org
Subject​: [perl #125924] [WIN32] 64-bit long double build crashes in
op/pack.t (was #125863)

On Tue Sep 01 05​:12​:53 2015, sisyphus wrote​:

+ if(cdouble >= 0) trouble = floorl(cdouble);
+ else trouble = ceill(cdouble);

could be​:

trouble = truncl(cdouble);

Yes, that is better.

As to providing our own modfl() implementation, the main issue is we'd
need to make sure it's exported so POSIX (or other XS modules) could see
it.

I later tried altering perl.h so that, for the particular conditions,
Perl_modf was defined to Perl_my_modfl (numeric.c), which uses truncl and
copysignl - and which, I assume, would give us the result we're after.
I *think* I got that part done as I intended, but the POSIX build then
croaked on the undefined reference to 'Perl_my_modfl' - so there was
obviously something that I didn't get right.

Here's the "proof-of-concept" patch to perl.h that I was using​:

Inline Patch

--- perl.h_orig    2015-09-01 21:16:45 +1000
+++ perl.h    2015-09-02 00:54:07 +1000
@@ -1915,6 +1915,23 @@

# define Perl_tan tanl \# define Perl\_tanh tanhl \# endif \+\#if defined\(USE\_LONG\_DOUBLE\) && defined\(\_\_MINGW64\_\_\) \\ \+ && \_\_MINGW64\_VERSION\_MAJOR == 4 && \_\_MINGW64\_VERSION\_MINOR == 0 \+ /\* modfl\(\) segfaults for \-Duselongdouble && 64\-bit mingw64 && mingw runtime version 4\.0 \*/ \+\#undef HAS\_MODFL \+\#ifndef HAS\_TRUNCL \+\#define HAS\_TRUNCL \+\#endif \+\#ifndef HAS\_COPYSIGNL \+\#define HAS\_COPYSIGNL \+\#endif \+\#ifndef HAS\_MODFL\_PROTO \+\#define HAS\_MODFL\_PROTO \+\#endif \+\#ifdef Perl\_modf \+\#undef Perl\_modf \+\#endif \+\#endif /\* e\.g\. libsunmath doesn't have modfl and frexpl as of mid\-March 2000 \*/ \# ifndef Perl\_modf \# ifdef HAS\_MODFL

I might have another go at it tonight.

Though we could add it to inline.h

I'm thinking that Perl_my_modfl() is already the "our own modfl()
implementation" that we're looking for - in which case we just need to
switch to using it (whenn faced with the mingw64 modfl bug).
But if it's better to add an implementation to inline.h, then that's fine by
me.

Cheers,
Rob

[p5pRT]

From @tonycoz

On Wed, Sep 02, 2015 at 12​:11​:40PM +1000, sisyphus1@​optusnet.com.au wrote​:

-----Original Message----- From​: Tony Cook via RT
Sent​: Wednesday, September 02, 2015 10​:59 AM

Though we could add it to inline.h

I'm thinking that Perl_my_modfl() is already the "our own modfl()
implementation" that we're looking for - in which case we just need to
switch to using it (whenn faced with the mingw64 modfl bug).
But if it's better to add an implementation to inline.h, then that's fine by
me.

Mostly so we don't need to add Perl_my_modfl to the Win32 exports on a
condition that might be hard to work out in makedef.pl.

Or just wait for mingw-64 4.1 with a fixed modfl() -- though the fix
in the ticket hasn't been applied yet.

Tony

[p5pRT]

From @sisyphus

-----Original Message-----
From​: Tony Cook
Sent​: Wednesday, September 02, 2015 12​:18 PM
To​: sisyphus1@​optusnet.com.au
Cc​: perlbug-followup@​perl.org ; perl5-porters@​perl.org
Subject​: Re​: [perl #125924] [WIN32] 64-bit long double build crashes in
op/pack.t (was #125863)

But if it's better to add an implementation to inline.h, then that's fine
by me.

Mostly so we don't need to add Perl_my_modfl to the Win32 exports on a
condition that might be hard to work out in makedef.pl.

Aaah - so that's the reason I was getting the undefined reference to
Perl_my_modfl. (I now understand.)

Or just wait for mingw-64 4.1 with a fixed modfl() -- though the fix in
the ticket hasn't been applied yet.

That option is fine by me - this modfl bug isn't creating any problems for
me that I haven't already worked around.

Let me know if there's anything else needs testing. Otherwise, I'm quite
happy to just let things sit as they are.

Cheers,
Rob

[p5pRT]

From @sisyphus

The mingw64-w64 bug in modfl() that caused this issue has finally been fixed in runtime version 5.0.3, and there's now no need for 64-bit long double builds of perl to patch pp_pack.c and POSIX.c so long as the runtime version used does not fall in the range 4.0 to 5.0.2 (inclusive).

Cheers,
Rob

[p5pRT]

From @xsawyerx

Are you suggesting to close the ticket?

On 11/23/2017 10​:57 AM, Sisyphus via RT wrote​:

The mingw64-w64 bug in modfl() that caused this issue has finally been fixed in runtime version 5.0.3, and there's now no need for 64-bit long double builds of perl to patch pp_pack.c and POSIX.c so long as the runtime version used does not fall in the range 4.0 to 5.0.2 (inclusive).

Cheers,
Rob

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=125924

[p5pRT]

From @sisyphus

-----Original Message-----
From​: Sawyer X via RT
Sent​: Tuesday, November 28, 2017 7​:19 AM
To​: sisyphus1@​optusnet.com.au
Subject​: Re​: [perl #125924] [WIN32] 64-bit long double build crashes in
op/pack.t (was #125863)

Are you suggesting to close the ticket?

No - I just wanted to update the thread to reflect the current state of
play.

With the compilers used by all recent builds of 64-bit Strawberry Perl (and
probably 64-bit ActiveState Perl, also) modfl() crashes - so the bug is
still very much alive and active.

The correct thing to do would be to fix the perl source, then close the
ticket - but that's messy.

The condition I use to determine whether modfl needs to be worked around (in
both pp_pack.c and POSIX.xs) is​:

#if defined(USE_LONG_DOUBLE) && defined(__MINGW64__) &&
(__MINGW64_VERSION_MAJOR == 4 || __MINGW64_VERSION_MAJOR == 5) &&
__MINGW64_VERSION_MINOR == 0
/* workaround to avoid Perl_modf */
#else
/* use existing code that uses Perl_modf /
#endif

And, of course, that will still perform the workaround when runtime version
is 5.0.3, even though 5.0.3 doesn't need the workaround.
Unfortunately, there's currently no way for the C pre-processor to determine
the "patchlevel" - though I believe this will be addressed in the next
runtime release when, in addition to __MINGW64_VERSION_MAJOR and
__MINGW64_VERSION_MINOR, they will provide a __MINGW64_VERSION_BUGFIX
value.

Having said all that, if you think the ticket should be closed, I've no
objection to that happening.

Cheers,
Rob

[tonycoz]

This was fixed upstream in https://sourceforge.net/p/mingw-w64/mingw-w64/ci/9a2479858b3bb7df36a09c4472fa1adca7e84f19/ I don't think we need to do anything further. Closing.