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

5.29.4 - re/subst.t crashes perl on 64-bit Windows #16729

Open
p5pRT opened this issue Oct 21, 2018 · 30 comments
Open

5.29.4 - re/subst.t crashes perl on 64-bit Windows #16729

p5pRT opened this issue Oct 21, 2018 · 30 comments

Comments

@p5pRT
Copy link

p5pRT commented Oct 21, 2018

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

Searchable as RT133603$

@p5pRT
Copy link
Author

p5pRT commented Oct 21, 2018

From @sisyphus

Hi,

On 64-bit builds, subst.t behaves perfectly up to and including test 236​:

ok 236 - vivifying stash elem in $that​::{elem} =~ s//.../e

At that point, however, perl immediately crashes - and I haven't yet found
a way of recovering from this crash in a way that enables the rest of the
test suite to be run.

The same crash occurs if I run the test script outside of Test​::Harness.

No problem with that test script on 32-bit builds.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Oct 21, 2018

From @tonycoz

On Sun, Oct 21, 2018 at 02​:03​:53AM -0700, sisyphus (via RT) wrote​:

# New Ticket Created by sisyphus
# Please include the string​: [perl #133603]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133603 >

Hi,

On 64-bit builds, subst.t behaves perfectly up to and including test 236​:

ok 236 - vivifying stash elem in $that​::{elem} =~ s//.../e

At that point, however, perl immediately crashes - and I haven't yet found
a way of recovering from this crash in a way that enables the rest of the
test suite to be run.

The same crash occurs if I run the test script outside of Test​::Harness.

No problem with that test script on 32-bit builds.

I didn't see it on 64-bit builds with CCTYPE=MSVC90.

Could you supply the -V information please?

Tony

@p5pRT
Copy link
Author

p5pRT commented Oct 21, 2018

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

@p5pRT
Copy link
Author

p5pRT commented Oct 21, 2018

From @sisyphus

On Sun, 21 Oct 2018 04​:19​:18 -0700, tonyc wrote​:

Could you supply the -V information please?

Summary of my perl5 (revision 5 version 29 subversion 4) configuration:

Platform:
osname=MSWin32
osvers=6.1.7601
archname=MSWin32-x64-multi-thread
uname=''
config_args='undef'
hint=recommended
useposix=true
d_sigaction=undef
useithreads=define
usemultiplicity=define
use64bitint=define
use64bitall=undef
uselongdouble=undef
usemymalloc=n
default_inc_excludes_dot=define
bincompat5005=undef
Compiler:
cc='gcc'
ccflags =' -s -O2 -DWIN32 -DWIN64 -DCONSERVATIVE -DPERL_TEXTMODE_SCRIPTS -D
PERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO -D__USE_MINGW_ANSI_STDIO
-fwrapv -fno-strict-aliasing -mms-bitfields'
optimize='-s -O2'
cppflags='-DWIN32'
ccversion=''
gccversion='8.1.0'
gccosandvers=''
intsize=4
longsize=4
ptrsize=8
doublesize=8
byteorder=12345678
doublekind=3
d_longlong=define
longlongsize=8
d_longdbl=define
longdblsize=16
longdblkind=3
ivtype='long long'
ivsize=8
nvtype='double'
nvsize=8
Off_t='long long'
lseeksize=8
alignbytes=8
prototype=define
Linker and Libraries:
ld='g++'
ldflags ='-s -L"C:\_64\blead-5.29.4\lib\CORE" -L"C:\_64\gcc-mingw-810\mingw6
4\lib"'
libpth=C:\_64\gcc-mingw-810\mingw64\lib C:\_64\gcc-mingw-810\mingw64\x86_64-
w64-mingw32\lib C:\_64\msys_810\1.0\local\lib C:\_64\gcc-mingw-810\mingw64\lib\g
cc\x86_64-w64-mingw32\8.1.0
libs= -lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi3
2 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lversio
n -lodbc32 -lodbccp32 -lcomctl32
perllibs= -lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladv
api32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lve
rsion -lodbc32 -lodbccp32 -lcomctl32
libc=
so=dll
useshrplib=true
libperl=libperl529.a
gnulibc_version=''
Dynamic Linking:
dlsrc=dl_win32.xs
dlext=dll
d_dlsymun=undef
ccdlflags=' '
cccdlflags=' '
lddlflags='-mdll -s -L"C:\_64\blead-5.29.4\lib\CORE" -L"C:\_64\gcc-mingw-810
\mingw64\lib"'


Characteristics of this binary (from libperl):
Compile-time options:
HAS_TIMES
HAVE_INTERP_INTERN
MULTIPLICITY
PERLIO_LAYERS
PERL_COPY_ON_WRITE
PERL_DONT_CREATE_GVSV
PERL_IMPLICIT_CONTEXT
PERL_IMPLICIT_SYS
PERL_MALLOC_WRAP
PERL_OP_PARENT
PERL_PRESERVE_IVUV
USE_64_BIT_INT
USE_ITHREADS
USE_LARGE_FILES
USE_LOCALE
USE_LOCALE_COLLATE
USE_LOCALE_CTYPE
USE_LOCALE_NUMERIC
USE_LOCALE_TIME
USE_PERLIO
USE_PERL_ATOF
Built under MSWin32
Compiled at Oct 21 2018 19:19:23
@INC:
C:/_64/blead-5.29.4/site/lib
C:/_64/blead-5.29.4/lib

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Oct 22, 2018

From @sisyphus

This bug might be specific to my mingw-w64 port of gcc-8.1.0 - though there was no such problem with 5.29.3.
When I build 5.29.4 with my mingw-w64 port of gcc-7.3.0, t/re/subst.t is fine.

If (using gcc-8.1.0) I skip the offending "read-only COW =~ s/does not match// should croak" test, that test script then runs ok and all tests except for IO/Socket.t (see #133604) pass.

However,I have now detected that t/run/fresh_perl.t is behaving strangely.
After the first 16 tests have passed, I get a pop-up box telling me that perl has stopped working.
But when I acknowledge that pop-up box, the test script continues on successfully and reports no failure.
I get​:

....
ok 15 - push(@​a, 1, 2, 3,)
ok 16 - quotemeta ""
<process hangs until I acknowledge the pop-up box>
ok 17 - for ("ABCDE") {
ok 18 - package FOO;sub new {bless {FOO => BAR}};
....

with all test passing.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Oct 22, 2018

From @sisyphus

On Sun, 21 Oct 2018 21​:54​:41 -0700, sisyphus@​cpan.org wrote​:

However,I have now detected that t/run/fresh_perl.t is behaving
strangely.
After the first 16 tests have passed, I get a pop-up box telling me
that perl has stopped working.
But when I acknowledge that pop-up box, the test script continues on
successfully and reports no failure.
I get​:

....
ok 15 - push(@​a, 1, 2, 3,)
ok 16 - quotemeta ""
<process hangs until I acknowledge the pop-up box>
ok 17 - for ("ABCDE") {
ok 18 - package FOO;sub new {bless {FOO => BAR}};
....

with all test passing.

Just a little bit more about this problem with t/run/fresh_perl.t on perl-5.29.4​:

1) It's happening only on x64 builds whose nvtype is double - there's no such issue if nvtype is long double;
2) It's happening when I use gcc-8.1.0, but I haven't yet checked to see what happens with earlier versions of gcc.
3) The problem did not arise when building 5.29.3 using gcc-8.1.0.

Needs some more poking.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Oct 23, 2018

From @tonycoz

On Sun, Oct 21, 2018 at 09​:54​:41PM -0700, sisyphus@​cpan.org via RT wrote​:

This bug might be specific to my mingw-w64 port of gcc-8.1.0 - though there was no such problem with 5.29.3.
When I build 5.29.4 with my mingw-w64 port of gcc-7.3.0, t/re/subst.t is fine.

If (using gcc-8.1.0) I skip the offending "read-only COW =~ s/does not match// should croak" test, that test script then runs ok and all tests except for IO/Socket.t (see #133604) pass.

However,I have now detected that t/run/fresh_perl.t is behaving strangely.
After the first 16 tests have passed, I get a pop-up box telling me that perl has stopped working.
But when I acknowledge that pop-up box, the test script continues on successfully and reports no failure.
I get​:

....
ok 15 - push(@​a, 1, 2, 3,)
ok 16 - quotemeta ""
<process hangs until I acknowledge the pop-up box>
ok 17 - for ("ABCDE") {
ok 18 - package FOO;sub new {bless {FOO => BAR}};
....

with all test passing.

I can't reproduce either failure​:

J​:\dev\perl\git\perl\t>.\perl -I..\lib -V​:gccversion
gccversion='8.1.0';

J​:\dev\perl\git\perl\t>.\perl harness run\fresh_perl.t
run/fresh_perl.t .. ok
All tests successful.
Files=1, Tests=92, 6 wallclock secs ( 0.03 usr + 0.03 sys = 0.06 CPU)
Result​: PASS

J​:\dev\perl\git\perl\t>.\perl -I..\lib -V​:ptrsize
ptrsize='8';

J​:\dev\perl\git\perl\t>.\perl harness re\subst.t
re/subst.t .. ok
All tests successful.
Files=1, Tests=276, 1 wallclock secs ( 0.02 usr + 0.01 sys = 0.03 CPU)
Result​: PASS

I didn't get a popup from fresh_perl.t

I started from a clean tree, and built with​:

  gmake -j2 CFG=Debug test-prep

Tony

@p5pRT
Copy link
Author

p5pRT commented Oct 23, 2018

From @sisyphus

On Mon, 22 Oct 2018 17​:20​:50 -0700, tonyc wrote​:
....

I can't reproduce either failure​:
....
gmake -j2 CFG=Debug test-prep

I've just now checked and "CFG=Debug" avoids both issues for me, too.
So that's yet another configuration option that has a bearing.

I'm still seeing these issues with a plain 'gmake -j2' (with no additional args), followed by​:

cd ..
perl -I./lib t/run/fresh_perl.t
perl -I./lib t/re/subst.t

I think there's a fair chance you'll see the same.

And I doubt that the '-j2' arg has any bearing on the issues as I don't normally specify any '-j*' arg when building.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Oct 23, 2018

From @tonycoz

Building with a default CFG, or by eliminating -DDEBUGGING I can
reproduce it​:

ok 236 - vivifying stash elem in $that::{elem} =~ s//.../e
gdb: unknown target exception 0xc0000028 at 0x77a28658

Program received signal ?, Unknown signal.
0x0000000077a28658 in ntdll!RtlRaiseStatus ()
from C:\Windows\SYSTEM32\ntdll.dll
(gdb) bt
#0 0x0000000077a28658 in ntdll!RtlRaiseStatus ()
from C:\Windows\SYSTEM32\ntdll.dll
#1 0x00000000779e7109 in ntdll!longjmp () from C:\Windows\SYSTEM32\ntdll.dll
#2 0x000007fefe39e5a3 in msvcrt!longjmp ()
from C:\Windows\system32\msvcrt.dll
#3 0x0000000069944310 in Perl_die_unwind (my_perl=my_perl@entry=0x2f6fe8,
msv=msv@entry=0x2807fd8) at ..\pp_ctl.c:1790
#4 0x00000000699cc74a in Perl_vcroak (my_perl=0x2f6fe8, pat=<optimized out>,
args=<optimized out>) at ..\util.c:1715
#5 0x00000000699cc826 in Perl_croak_nocontext (
pat=0x69b4ccd7 <details+1335> "%s") at ..\util.c:1749
#6 0x00000000699ccdd7 in Perl_croak_no_modify () at ..\util.c:1778
#7 0x0000000069968890 in Perl_pp_subst (my_perl=0x2f6fe8) at ..\pp_hot.c:4164
#8 0x00000000699c3506 in Perl_runops_standard (my_perl=0x2f6fe8)
at ..\run.c:42
#9 0x000000006997fc50 in S_run_body (oldscope=<optimized out>,
my_perl=<optimized out>) at perl.c:2683
#10 perl_run (my_perl=0x699d2e70 <xs_init(PerlInterpreter*)>,
my_perl@entry=0x2f6fe8) at perl.c:2611
#11 0x00000000699d6538 in RunPerl (argc=<optimized out>,
argv=<optimized out>, env=0x443b20) at perllib.c:251
#12 0x00000000004013c7 in __tmainCRTStartup ()
#13 0x00000000004014fb in mainCRTStartup ()

and the fresh_perl test:

tarting program: J:\dev\perl\git\perl\t\perl.exe ..\t.pl
New Thread 9312.0x234c]
odification of a read-only value attempted at ..\t.pl line 3.
db: unknown target exception 0xc0000028 at 0x77a28658

rogram received signal ?, Unknown signal.
x0000000077a28658 in ntdll!RtlRaiseStatus ()
from C:\Windows\SYSTEM32\ntdll.dll
gdb) bt
0 0x0000000077a28658 in ntdll!RtlRaiseStatus ()
from C:\Windows\SYSTEM32\ntdll.dll
1 0x00000000779e7109 in ntdll!longjmp () from C:\Windows\SYSTEM32\ntdll.dll
2 0x000007fefe39e5a3 in msvcrt!longjmp ()
from C:\Windows\system32\msvcrt.dll
3 0x000000006997707e in S_my_exit_jump (my_perl=my_perl@entry=0x496fc8)
at perl.c:5239
4 0x000000006997fe6c in Perl_my_failure_exit (
my_perl=my_perl@entry=0x496fc8) at perl.c:5221
5 0x0000000069944323 in Perl_die_unwind (my_perl=my_perl@entry=0x496fc8,
msv=msv@entry=0x49a628) at ..\pp_ctl.c:1796
6 0x00000000699cc74a in Perl_vcroak (my_perl=0x496fc8, pat=<optimized out>,
args=<optimized out>) at ..\util.c:1715
7 0x00000000699cc826 in Perl_croak_nocontext (
pat=0x69b4ccd7 <details+1335> "%s") at ..\util.c:1749
8 0x00000000699ccdd7 in Perl_croak_no_modify () at ..\util.c:1778
9 0x0000000069968890 in Perl_pp_subst (my_perl=0x496fc8) at ..\pp_hot.c:4164
10 0x00000000699c3506 in Perl_runops_standard (my_perl=0x496fc8)
at ..\run.c:42
11 0x000000006997fd47 in S_run_body (oldscope=<optimized out>,
my_perl=<optimized out>) at perl.c:2688
12 perl_run (my_perl=0x699d2e70 <xs_init(PerlInterpreter*)>,
my_perl@entry=0x496fc8) at perl.c:2611
13 0x00000000699d6538 in RunPerl (argc=<optimized out>,
argv=<optimized out>, env=0x1bd3b20) at perllib.c:251
14 0x00000000004013c7 in __tmainCRTStartup ()
15 0x00000000004014fb in mainCRTStartup ()

Tony

@p5pRT
Copy link
Author

p5pRT commented Dec 2, 2018

From @sisyphus

Haven't made any progress with this.
One correction to make - I stated above that "nvtype=long double" builds were unaffected, but that is incorrect.
They are affected in exactly the same way as "nvtype=double" builds.

Other than that, all I have is a couple of one-liners (based on the t/run/fresh_perl.t and t/re/subst.t failures) that demonstrate the problem​:

C​:\>perl -le "for('A') {s/b/c/}"
Modification of a read-only value attempted at -e line 1.

C​:\>perl -le "for(__PACKAGE__) {s/b/c/}"
Modification of a read-only value attempted at -e line 1.

C​:\>

In both cases a "perl.exe has stopped working" pop-up appears after the error message has been presented, and the process doesn't exit until the pop-up has been acknowledged.

One heisenbuggy aspect that I noticed was that if I preceded the above code with a Devel​::Peek​::Dump() of an arbitrary string, then there was no crash to contend with. That is, this one liner (for example) behaves exactly as it ought​:

C​:\>perl -MDevel​::Peek -le "Dump('x'); for('A') {s/b/c/}"
SV = PV(0x37c668) at 0x47c350
  REFCNT = 1
  FLAGS = (POK,IsCOW,READONLY,PROTECT,pPOK)
  PV = 0x486888 "x"\0
  CUR = 1
  LEN = 10
  COW_REFCNT = 0
Modification of a read-only value attempted at -e line 1.

C​:\>

Problem still persists in 5.29.5.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2018

From @tonycoz

On Sat, 01 Dec 2018 23​:46​:44 -0800, sisyphus@​cpan.org wrote​:

Problem still persists in 5.29.5.

After some fumbling, this bisected to​:

69aa5eb is the first bad commit
commit 69aa5eb
Author​: Tomasz Konojacki <me@​xenu.pl>
Date​: Mon Aug 13 01​:56​:47 2018 +0200

  win32​: define HAS_BUILTIN_EXPECT on MinGW
 
  According to perlwin32, the oldest officially supported version of
  gcc is 3.4.5. This built-in was introduced in 3.0, which means we
  don't have to do any version checks.
 
  [perl #133442]

:040000 040000 b4bfa77c7d830364d15c8004a70fb0e05bdff24e 6c7cd94c61bee93056f74b169328b169583428bd M win32
bisect run success

which implies a bug in gcc, since all this does is turn​:

  EXPECT(expr,val)

from

  (expr)

to

  __builtin_expect(expr,val)

and EXPECT(expr,val) is only used in the LIKELY() and UNLIKELY() macros​:

  #define LIKELY(cond) EXPECT(cBOOL(cond),TRUE)
  #define UNLIKELY(cond) EXPECT(cBOOL(cond),FALSE)

__builtin_expect() is documented at https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html

Reverting the above commit prevented the crash for me.

Tony

@p5pRT
Copy link
Author

p5pRT commented Dec 4, 2018

From @sisyphus

On Sun, 02 Dec 2018 20​:52​:42 -0800, tonyc wrote​:

On Sat, 01 Dec 2018 23​:46​:44 -0800, sisyphus@​cpan.org wrote​:

Problem still persists in 5.29.5.

After some fumbling, this bisected to​:

69aa5eb is the first bad commit
commit 69aa5eb
Author​: Tomasz Konojacki <me@​xenu.pl>
Date​: Mon Aug 13 01​:56​:47 2018 +0200

win32​: define HAS_BUILTIN_EXPECT on MinGW

According to perlwin32, the oldest officially supported version of
gcc is 3.4.5. This built-in was introduced in 3.0, which means we
don't have to do any version checks.

[perl #133442]

:040000 040000 b4bfa77c7d830364d15c8004a70fb0e05bdff24e
6c7cd94c61bee93056f74b169328b169583428bd M win32
bisect run success

which implies a bug in gcc, since all this does is turn​:

EXPECT(expr,val)

from

(expr)

to

__builtin_expect(expr,val)

and EXPECT(expr,val) is only used in the LIKELY() and UNLIKELY()
macros​:

#define LIKELY(cond) EXPECT(cBOOL(cond),TRUE)
#define UNLIKELY(cond) EXPECT(cBOOL(cond),FALSE)

__builtin_expect() is documented at
https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html

Reverting the above commit prevented the crash for me.

Tony

Reverting those changes to win32/config.gc and win32/config_H.gc also works for me - and I'll continue to make those changes whenever I build perl using my mingw-w64 port of 64-bit gcc-8.1.0. (I already patch those 2 files for my own builds as a result of #133582, anyway.)

If anyone comes up with a C program that demonstrates this gcc bug, please let me know as I'd like to file a bug report for it with the mingw-w64 project.
In the meantime I'll try to create that demo myself.

Thanks Tony.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Dec 4, 2018

From @xenu

On Tue, 04 Dec 2018 03​:31​:29 -0800
"sisyphus@​cpan.org via RT" <perlbug-followup@​perl.org> wrote​:

Reverting those changes to win32/config.gc and win32/config_H.gc also works for me - and I'll continue to make those changes whenever I build perl using my mingw-w64 port of 64-bit gcc-8.1.0. (I already patch those 2 files for my own builds as a result of #133582, anyway.)

If anyone comes up with a C program that demonstrates this gcc bug, please let me know as I'd like to file a bug report for it with the mingw-w64 project.
In the meantime I'll try to create that demo myself.

Thanks Tony.

Cheers,
Rob

I also am trying to find the cause of the crash, but I'm having no luck
so far. However, I managed to narrow it down a bit. Removing UNLIKELY()
from SvGETMAGIC() macro makes it not crash​:

Inline Patch
diff --git a/sv.h b/sv.h
index f3392b08ec..58c3a8f33c 100644
--- a/sv.h
+++ b/sv.h
@@ -2055,7 +2055,7 @@ properly null terminated. Equivalent to sv_setpvs(""), but more efficient.
 #define SvUNLOCK(sv) PL_unlockhook(aTHX_ sv)
 #define SvDESTROYABLE(sv) PL_destroyhook(aTHX_ sv)

-#define SvGETMAGIC(x) ((void)(UNLIKELY(SvGMAGICAL(x)) && mg_get(x)))
+#define SvGETMAGIC(x) ((void)((SvGMAGICAL(x)) && mg_get(x)))
 #define SvSETMAGIC(x) STMT_START { if (UNLIKELY(SvSMAGICAL(x))) mg_set(x); } STMT_END

 #define SvSetSV_and(dst,src,finally) \

@p5pRT
Copy link
Author

p5pRT commented Dec 4, 2018

From @khwilliamson

On 12/4/18 7​:02 AM, Tomasz Konojacki wrote​:

On Tue, 04 Dec 2018 03​:31​:29 -0800
"sisyphus@​cpan.org via RT" <perlbug-followup@​perl.org> wrote​:

Reverting those changes to win32/config.gc and win32/config_H.gc also works for me - and I'll continue to make those changes whenever I build perl using my mingw-w64 port of 64-bit gcc-8.1.0. (I already patch those 2 files for my own builds as a result of #133582, anyway.)

If anyone comes up with a C program that demonstrates this gcc bug, please let me know as I'd like to file a bug report for it with the mingw-w64 project.
In the meantime I'll try to create that demo myself.

Thanks Tony.

Cheers,
Rob

I also am trying to find the cause of the crash, but I'm having no luck
so far. However, I managed to narrow it down a bit. Removing UNLIKELY()
from SvGETMAGIC() macro makes it not crash​:

diff --git a/sv.h b/sv.h
index f3392b08ec..58c3a8f33c 100644
--- a/sv.h
+++ b/sv.h
@​@​ -2055,7 +2055,7 @​@​ properly null terminated. Equivalent to sv_setpvs(""), but more efficient.
#define SvUNLOCK(sv) PL_unlockhook(aTHX_ sv)
#define SvDESTROYABLE(sv) PL_destroyhook(aTHX_ sv)

-#define SvGETMAGIC(x) ((void)(UNLIKELY(SvGMAGICAL(x)) && mg_get(x)))
+#define SvGETMAGIC(x) ((void)((SvGMAGICAL(x)) && mg_get(x)))
#define SvSETMAGIC(x) STMT_START { if (UNLIKELY(SvSMAGICAL(x))) mg_set(x); } STMT_END

#define SvSetSV_and(dst,src,finally) \

In the meantime, it seems that a hints directive could be used on mingw
to make UNLIKELY and LIKELY do nothing, and that could be put in blead.

@p5pRT
Copy link
Author

p5pRT commented Apr 2, 2019

From @sisyphus

On Tue, 04 Dec 2018 07​:26​:12 -0800, public@​khwilliamson.com wrote​:

On 12/4/18 7​:02 AM, Tomasz Konojacki wrote​:

On Tue, 04 Dec 2018 03​:31​:29 -0800
"sisyphus@​cpan.org via RT" <perlbug-followup@​perl.org> wrote​:

Reverting those changes to win32/config.gc and win32/config_H.gc
also works for me - and I'll continue to make those changes whenever
I build perl using my mingw-w64 port of 64-bit gcc-8.1.0. (I already
patch those 2 files for my own builds as a result of #133582,
anyway.)

If anyone comes up with a C program that demonstrates this gcc bug,
please let me know as I'd like to file a bug report for it with the
mingw-w64 project.
In the meantime I'll try to create that demo myself.

Thanks Tony.

Cheers,
Rob

I also am trying to find the cause of the crash, but I'm having no
luck
so far. However, I managed to narrow it down a bit. Removing
UNLIKELY()
from SvGETMAGIC() macro makes it not crash​:

diff --git a/sv.h b/sv.h
index f3392b08ec..58c3a8f33c 100644
--- a/sv.h
+++ b/sv.h
@​@​ -2055,7 +2055,7 @​@​ properly null terminated. Equivalent to
sv_setpvs(""), but more efficient.
#define SvUNLOCK(sv) PL_unlockhook(aTHX_ sv)
#define SvDESTROYABLE(sv) PL_destroyhook(aTHX_ sv)

-#define SvGETMAGIC(x) ((void)(UNLIKELY(SvGMAGICAL(x)) && mg_get(x)))
+#define SvGETMAGIC(x) ((void)((SvGMAGICAL(x)) && mg_get(x)))
#define SvSETMAGIC(x) STMT_START { if (UNLIKELY(SvSMAGICAL(x)))
mg_set(x); } STMT_END

#define SvSetSV_and(dst,src,finally) \

In the meantime, it seems that a hints directive could be used on
mingw
to make UNLIKELY and LIKELY do nothing, and that could be put in
blead.

Hi,
I'm still experiencing the issue with each and every devel release of perl-5.29.x, and I'm still fixing it by reverting the changes to win32/config.gc and win32/config_H.gc (as mentioned a coupla posts back in this thread).

My gcc-8.1.0 was built by the mingw-w64 project, and today I've also built latest devel release (5.29.9) using a gcc-8.2.1 toolchain that was built by the msys2 project.

That build of perl also experienced the same issues as the gcc-8.1.0 build, and those issues were again resolved when I reverted the changes to win32/config.gc and win32/config_H.gc.

So we can now say that this issue afflicts both 64-bit gcc-8.1.0 (with mingw runtime version 6.0) and 64-bit gcc-8.2.1 (with mingw runtime version 7.0).

Ought this be deemed a blocker for 5.30 ?

Personally, I've no objection to continuing to revert win32/config.gc and win32/config_H.gc for my local builds.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Apr 2, 2019

From @khwilliamson

On Mon, 01 Apr 2019 19​:31​:47 -0700, sisyphus@​cpan.org wrote​:

On Tue, 04 Dec 2018 07​:26​:12 -0800, public@​khwilliamson.com wrote​:

On 12/4/18 7​:02 AM, Tomasz Konojacki wrote​:

On Tue, 04 Dec 2018 03​:31​:29 -0800
"sisyphus@​cpan.org via RT" <perlbug-followup@​perl.org> wrote​:

Reverting those changes to win32/config.gc and win32/config_H.gc
also works for me - and I'll continue to make those changes
whenever
I build perl using my mingw-w64 port of 64-bit gcc-8.1.0. (I
already
patch those 2 files for my own builds as a result of #133582,
anyway.)

If anyone comes up with a C program that demonstrates this gcc
bug,
please let me know as I'd like to file a bug report for it with
the
mingw-w64 project.
In the meantime I'll try to create that demo myself.

Thanks Tony.

Cheers,
Rob

I also am trying to find the cause of the crash, but I'm having no
luck
so far. However, I managed to narrow it down a bit. Removing
UNLIKELY()
from SvGETMAGIC() macro makes it not crash​:

diff --git a/sv.h b/sv.h
index f3392b08ec..58c3a8f33c 100644
--- a/sv.h
+++ b/sv.h
@​@​ -2055,7 +2055,7 @​@​ properly null terminated. Equivalent to
sv_setpvs(""), but more efficient.
#define SvUNLOCK(sv) PL_unlockhook(aTHX_ sv)
#define SvDESTROYABLE(sv) PL_destroyhook(aTHX_ sv)

-#define SvGETMAGIC(x) ((void)(UNLIKELY(SvGMAGICAL(x)) &&
mg_get(x)))
+#define SvGETMAGIC(x) ((void)((SvGMAGICAL(x)) && mg_get(x)))
#define SvSETMAGIC(x) STMT_START { if (UNLIKELY(SvSMAGICAL(x)))
mg_set(x); } STMT_END

#define SvSetSV_and(dst,src,finally) \

In the meantime, it seems that a hints directive could be used on
mingw
to make UNLIKELY and LIKELY do nothing, and that could be put in
blead.

Hi,
I'm still experiencing the issue with each and every devel release of
perl-5.29.x, and I'm still fixing it by reverting the changes to
win32/config.gc and win32/config_H.gc (as mentioned a coupla posts
back in this thread).

My gcc-8.1.0 was built by the mingw-w64 project, and today I've also
built latest devel release (5.29.9) using a gcc-8.2.1 toolchain that
was built by the msys2 project.

That build of perl also experienced the same issues as the gcc-8.1.0
build, and those issues were again resolved when I reverted the
changes to win32/config.gc and win32/config_H.gc.

So we can now say that this issue afflicts both 64-bit gcc-8.1.0 (with
mingw runtime version 6.0) and 64-bit gcc-8.2.1 (with mingw runtime
version 7.0).

Ought this be deemed a blocker for 5.30 ?

Personally, I've no objection to continuing to revert win32/config.gc
and win32/config_H.gc for my local builds.

Cheers,
Rob

I added this to the 5.30 blockers list, to make sure it gets adequate attention

--
Karl Williamson

@p5pRT
Copy link
Author

p5pRT commented Apr 23, 2019

From @iabyn

On Mon, Apr 01, 2019 at 08​:01​:04PM -0700, Karl Williamson via RT wrote​:

On Mon, 01 Apr 2019 19​:31​:47 -0700, sisyphus@​cpan.org wrote​:

I'm still experiencing the issue with each and every devel release of
perl-5.29.x, and I'm still fixing it by reverting the changes to
win32/config.gc and win32/config_H.gc (as mentioned a coupla posts
back in this thread).

I added this to the 5.30 blockers list, to make sure it gets adequate attention

I think perhaps for 5.30.0 we should revert the entirely of the
"HAS_BUILTIN_EXPECT on MinGW" patch, then move this ticket from the 5.30.0
to the 5.32.0 blockers list and return to the issue after 5.30.0 is
released.

The commit I propose reverting in full is​:

commit 69aa5eb
Author​: Tomasz Konojacki <me@​xenu.pl>
AuthorDate​: Mon Aug 13 01​:56​:47 2018 +0200
Commit​: Tony Cook <tony@​develop-help.com>
CommitDate​: Tue Oct 16 10​:22​:11 2018 +1100

  win32​: define HAS_BUILTIN_EXPECT on MinGW
 
  According to perlwin32, the oldest officially supported version of
  gcc is 3.4.5. This built-in was introduced in 3.0, which means we
  don't have to do any version checks.
 
  [perl #133442]

Affected files ...
  M win32/config.gc
  M win32/config_H.gc

Differences ...

Inline Patch
diff --git a/win32/config.gc b/win32/config.gc
index 4477ebc04c..56f367f91d 100644
--- a/win32/config.gc
+++ b/win32/config.gc
@@ -117,7 +117,7 @@ d_bsdgetpgrp='undef'
 d_bsdsetpgrp='undef'
 d_builtin_add_overflow='undef'
 d_builtin_choose_expr='undef'
-d_builtin_expect='undef'
+d_builtin_expect='define'
 d_builtin_mul_overflow='undef'
 d_builtin_sub_overflow='undef'
 d_c99_variadic_macros='undef'
diff --git a/win32/config_H.gc b/win32/config_H.gc
index 87e90bac4b..756d4a7cf2 100644
--- a/win32/config_H.gc
+++ b/win32/config_H.gc
@@ -2319,7 +2319,7 @@
  *	Can we handle GCC builtin for telling that certain values are more
  *	likely
  */
-/*#define HAS_BUILTIN_EXPECT	/ **/
+#define HAS_BUILTIN_EXPECT	/**/
 /*#define HAS_BUILTIN_CHOOSE_EXPR	/ **/
 
 /* HAS_C99_VARIADIC_MACROS:


-- 

"Emacs isn't a bad OS once you get used to it.
It just lacks a decent editor."

@p5pRT
Copy link
Author

p5pRT commented Apr 23, 2019

From @xenu

On Tue, 23 Apr 2019 16​:43​:50 +0100
Dave Mitchell <davem@​iabyn.com> wrote​:

I think perhaps for 5.30.0 we should revert the entirely of the
"HAS_BUILTIN_EXPECT on MinGW" patch, then move this ticket from the 5.30.0
to the 5.32.0 blockers list and return to the issue after 5.30.0 is
released.

The commit I propose reverting in full is​:

commit 69aa5eb
Author​: Tomasz Konojacki <me@​xenu.pl>
AuthorDate​: Mon Aug 13 01​:56​:47 2018 +0200
Commit​: Tony Cook <tony@​develop-help.com>
CommitDate​: Tue Oct 16 10​:22​:11 2018 +1100

win32&#8203;: define HAS\_BUILTIN\_EXPECT on MinGW

According to perlwin32\, the oldest officially supported version of
gcc is 3\.4\.5\. This built\-in was introduced in 3\.0\, which means we
don't have to do any version checks\.

\[perl \#133442\]

Affected files ...
M win32/config.gc
M win32/config_H.gc

Differences ...

diff --git a/win32/config.gc b/win32/config.gc
index 4477ebc04c..56f367f91d 100644
--- a/win32/config.gc
+++ b/win32/config.gc
@​@​ -117,7 +117,7 @​@​ d_bsdgetpgrp='undef'
d_bsdsetpgrp='undef'
d_builtin_add_overflow='undef'
d_builtin_choose_expr='undef'
-d_builtin_expect='undef'
+d_builtin_expect='define'
d_builtin_mul_overflow='undef'
d_builtin_sub_overflow='undef'
d_c99_variadic_macros='undef'
diff --git a/win32/config_H.gc b/win32/config_H.gc
index 87e90bac4b..756d4a7cf2 100644
--- a/win32/config_H.gc
+++ b/win32/config_H.gc
@​@​ -2319,7 +2319,7 @​@​
* Can we handle GCC builtin for telling that certain values are more
* likely
*/
-/*#define HAS_BUILTIN_EXPECT / **/
+#define HAS_BUILTIN_EXPECT /**/
/*#define HAS_BUILTIN_CHOOSE_EXPR / **/

/* HAS_C99_VARIADIC_MACROS​:

--
"Emacs isn't a bad OS once you get used to it.
It just lacks a decent editor."

Yeah, I think it's the best course of action. I hoped we would find the
cause and report it to gcc/binutils/mingw-w64/whatever-is-at-fault so
they could fix it in time for 5.30 release, but unfortunately that
didn't happen.

@p5pRT
Copy link
Author

p5pRT commented Apr 24, 2019

From @iabyn

On Tue, Apr 23, 2019 at 11​:03​:35PM +0200, Tomasz Konojacki wrote​:

Dave Mitchell <davem@​iabyn.com> wrote​:

I think perhaps for 5.30.0 we should revert the entirely of the
"HAS_BUILTIN_EXPECT on MinGW" patch, then move this ticket from the 5.30.0
to the 5.32.0 blockers list and return to the issue after 5.30.0 is
released.

Yeah, I think it's the best course of action. I hoped we would find the
cause and report it to gcc/binutils/mingw-w64/whatever-is-at-fault so
they could fix it in time for 5.30 release, but unfortunately that
didn't happen.

Now reverted with v5.29.10-13-gabd494f123. I'll move it to the 5.32.0
blockers list.

--
Lear​: Dost thou call me fool, boy?
Fool​: All thy other titles thou hast given away; that thou wast born with.

@p5pRT
Copy link
Author

p5pRT commented May 31, 2019

From @jkeenan

On Wed, 24 Apr 2019 11​:32​:38 GMT, davem wrote​:

On Tue, Apr 23, 2019 at 11​:03​:35PM +0200, Tomasz Konojacki wrote​:

Dave Mitchell <davem@​iabyn.com> wrote​:

I think perhaps for 5.30.0 we should revert the entirely of the
"HAS_BUILTIN_EXPECT on MinGW" patch, then move this ticket from the
5.30.0
to the 5.32.0 blockers list and return to the issue after 5.30.0 is
released.

Yeah, I think it's the best course of action. I hoped we would find
the
cause and report it to gcc/binutils/mingw-w64/whatever-is-at-fault so
they could fix it in time for 5.30 release, but unfortunately that
didn't happen.

Now reverted with v5.29.10-13-gabd494f123. I'll move it to the 5.32.0
blockers list.

Have we identified the cause of the problem reported in this ticket (such that we can report the bug externally)?

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2019

From @sisyphus

AFAIK, although we know how to trigger it, we haven't yet identified the
bug - and therefore we are still unable to report it.

I did spend some time a while back trying to create a C script that would
demo the issue, but failed miserably.

Using the mingw64 port of gcc-8.3.0 that ships with StrawberryPerl-5.30.0,
I find it's also exhibiting the bug - just like the 8.1.0 and 8.2.1
mingw64 compilers that I had tried earlier.

Cheers,
Rob

On Fri, May 31, 2019 at 10​:43 PM James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

On Wed, 24 Apr 2019 11​:32​:38 GMT, davem wrote​:

On Tue, Apr 23, 2019 at 11​:03​:35PM +0200, Tomasz Konojacki wrote​:

Dave Mitchell <davem@​iabyn.com> wrote​:

I think perhaps for 5.30.0 we should revert the entirely of the
"HAS_BUILTIN_EXPECT on MinGW" patch, then move this ticket from the
5.30.0
to the 5.32.0 blockers list and return to the issue after 5.30.0 is
released.

Yeah, I think it's the best course of action. I hoped we would find
the
cause and report it to gcc/binutils/mingw-w64/whatever-is-at-fault so
they could fix it in time for 5.30 release, but unfortunately that
didn't happen.

Now reverted with v5.29.10-13-gabd494f123. I'll move it to the 5.32.0
blockers list.

Have we identified the cause of the problem reported in this ticket (such
that we can report the bug externally)?

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

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

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2019

From @xenu

On Sat, 1 Jun 2019, at 03​:20, sisyphus wrote​:

AFAIK, although we know how to trigger it, we haven't yet identified the bug - and therefore we are still unable to report it.

I did spend some time a while back trying to create a C script that would demo the issue, but failed miserably.

Using the mingw64 port of gcc-8.3.0 that ships with StrawberryPerl-5.30.0, I find it's also exhibiting the bug - just like the 8.1.0 and 8.2.1 mingw64 compilers that I had tried earlier.

Cheers,
Rob

My findings so far​:

1. The attached patch makes it not crash with HAS_BUILTIN_EXPECT commit *not* reverted. It removes LIKELY() from SvGETMAGIC() inside pp_subst().

2. I have attached assembly code of pp_hot.c with and without my patch applied. The difference between them is really tiny​:

--- a/C​:/Users/xenu/Documents/pp_hot/crashing/pp_hot.s
+++ b/C​:/Users/xenu/Documents/pp_hot/not_crashing/pp_hot.s
@​@​ -10462,6 +10462,8 @​@​ Perl_pp_subst​:
  movq %rax, %rdx
  call Perl_sv_setiv
  jmp .L2429
+.L2379​:^M
+ call Perl_croak_no_modify^M
.L2617​:
  movq %r12, %rcx
  movq %r11, %rdx
@​@​ -10471,8 +10473,6 @​@​ Perl_pp_subst​:
.L2464​:
  subq -9(%rcx), %rcx
  jmp .L2463
-.L2379​:
- call Perl_croak_no_modify
  .seh_endproc
  .section .text.unlikely,"x"
  .def Perl_pp_subst.cold.16; .scl 3; .type 32; .endef

Unless I'm missing something, this change is completely harmless. The only difference is Perl_croak_no_modify() call being moved a bit, but the control flow in both versions is *exactly* the same.

That makes me suspect that it's a bug in the linker, which is probably the worst possible scenario, because linkers are extremely hard to debug :(

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2019

From @xenu

crashing_pp_hot.s.gz

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2019

From @xenu

not_crashing_pp_hot.s.gz

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2019

From @xenu

make_it_not_crash.patch
diff --git a/pp_hot.c b/pp_hot.c
index 2df5df8303..9ecb0b678f 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -4146,7 +4146,7 @@ PP(pp_subst)
 	EXTEND(SP,1);
     }
 
-    SvGETMAGIC(TARG); /* must come before cow check */
+    ((void)((SvGMAGICAL(TARG)) && mg_get(TARG))); /* must come before cow check */
 #ifdef PERL_ANY_COW
     /* note that a string might get converted to COW during matching */
     was_cow = cBOOL(SvIsCOW(TARG));

@demerphq
Copy link
Collaborator

demerphq commented Feb 2, 2020

I've created a meta-ticket around the general issue of this type of failure in mingw at #17521

@xsawyerx
Copy link
Member

xsawyerx commented Apr 1, 2020

@iabyn Any chance to review the patch from @xenu?

@xenu
Copy link
Member

xenu commented Apr 3, 2020

This patch wasn't meant to be applied, its purpose was to demonstrate the issue.

Anyway, there's nothing we can do about this issue in time for 5.32. We still don't have a minimal code sample, so we can't report it to binutils (or gcc) developers.

@khwilliamson khwilliamson removed this from the 5.32.0 milestone Apr 3, 2020
@khwilliamson
Copy link
Contributor

Based on the above comment, I have removed this as a 5.32 blocker

@khwilliamson
Copy link
Contributor

What is the current status of this? @xenu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants