-
Notifications
You must be signed in to change notification settings - Fork 560
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
[Win32] Unable to build 64-bit blead using gcc-4.8.2 #14556
Comments
From @sisyphusHi, This is a bug report following on from the p5p thread AFAIK, no bug report was filed for this, but the issue is still unresolved. To re-iterate, early on in the 'dmake' stage we get: ..\miniperl.exe -I..\lib -f ..\write_buildcustomize.pl .. Cheers, |
From @tonycozOn Tue Mar 03 18:22:30 2015, sisyphus wrote:
Attached is a dump with the breakpoint in the correct place. Tony |
From @tonycoz(gdb) dir .. Breakpoint 1, Perl_gv_fetchpvn_flags (nambeg=<optimized out>, full_len=8, Breakpoint 1, Perl_gv_fetchpvn_flags (nambeg=<optimized out>, full_len=8, Breakpoint 1, Perl_gv_fetchpvn_flags (nambeg=<optimized out>, full_len=8, Inferior 1 [process 5860] will be killed. Quit anyway? (y or n) [answered Y; input not from terminal] |
The RT System itself - Status changed from 'new' to 'open' |
From @tonycozOn Tue Mar 31 17:56:12 2015, tonyc wrote:
Here's a simpler reproducer: BEGIN { Produces: Attempt to free unreferenced scalar: SV 0x346ec0. *without* the call to foo(). Tony |
From @tonycozOn Tue Mar 31 19:08:05 2015, tonyc wrote:
You don't need the variable, so just: BEGIN { will produce the "Attempt to free unreferenced scalar" and the assertion failure. Tony |
From @tonycozOn Tue Mar 03 18:22:30 2015, sisyphus wrote:
The attached seems to fix the problem for me, though I'm not sure if it's fixing Tony |
From @tonycoz0001-prevent-any-other-code-from-freeing-the-op-s-gv-whil.patchFrom 5078dd92544f00fa245a5272bc399e690d5711a6 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 1 Apr 2015 16:36:51 +1100
Subject: [PATCH 1/2] prevent any other code from freeing the op's gv while we
do
assuming I understand why this is breaking, which is unlikely
---
op.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/op.c b/op.c
index ad0b0b0..efacd31 100644
--- a/op.c
+++ b/op.c
@@ -824,17 +824,22 @@ void S_op_clear_gv(pTHX_ OP *o, SV**svp)
so any *valid* code that happens to do this during global
destruction might well trigger premature cleanup. */
bool still_valid = gv && SvREFCNT(gv);
+#ifdef USE_ITHREADS
+ PADOFFSET ix = *ixp;
+#else
+ SV *sv = *svp;
+#endif
if (still_valid)
SvREFCNT_inc_simple_void(gv);
#ifdef USE_ITHREADS
- if (*ixp > 0) {
- pad_swipe(*ixp, TRUE);
+ if (ix > 0) {
*ixp = 0;
+ pad_swipe(ix, TRUE);
}
#else
- SvREFCNT_dec(*svp);
*svp = NULL;
+ SvREFCNT_dec(sv);
#endif
if (still_valid) {
int try_downgrade = SvREFCNT(gv) == 2;
--
1.9.5.msysgit.0
|
From @iabynOn Tue, Mar 31, 2015 at 10:44:34PM -0700, Tony Cook via RT wrote:
Hmm, I've stared and stared at it and can't see a real problem there. One slightly odd thing about that particular bit of perl code is that the for (i=0; i< parser->yylen; i++) { and at that point, PL_curpad is null - so all the pad manipulation that -- |
From @tonycozOn Wed Apr 01 05:43:08 2015, davem wrote:
miniperl is built as non-threaded on Win32, so PL_curpad isn't checked and I've added a trace through of S_op_clear_gv - which asserts on the second Tony |
From @tonycozOn Wed Apr 01 15:43:20 2015, tonyc wrote:
Really added now. Tony |
From @tonycozj:\dev\perl\git\perl\win32>gdb ..\miniperl.exe Breakpoint 1, S_op_clear_gv (o=o@entry=0x2784d38, svp=svp@entry=0x2784d60) This application has requested the Runtime to terminate it in an unusual way. P r o g r a m : j : \ d e v \ p e r l \ g i t \ p e r l \ m i n i p e r l . e x e E x p r e s s i o n : S v T Y P E ( g v ) = = S V t _ P V G V | | S v T Y P E ( g v ) = = S V t _ P V L V |
From @iabynOn Wed, Apr 01, 2015 at 03:43:21PM -0700, Tony Cook via RT wrote:
Ah, I hadn't realised.
I'm not sure that's the case. The optimised code is showing enough random On my system, Perl_gv_init_pvn() isn't invoked at all around that time. $ cat /tmp/p $ gdb ./miniperl -- |
From @tonycozOn Thu Apr 02 11:20:19 2015, davem wrote:
You're right, it wasn't in the SvREFCNT_dec(). Here's the backtrace, I added code to break into the debugger when the SvTYPE() (gdb) list gv.c:411 Program received signal SIGSEGV, Segmentation fault. I also traced to see what was freeing the GV: (gdb) b S_op_clear_gv Breakpoint 1, S_op_clear_gv (o=o@entry=0x2884218, svp=svp@entry=0x2884240) Old value = 2 Old value = 3 Old value = 1 Program received signal SIGSEGV, Segmentation fault. I'm not sure that tells us anything useful. Tony |
From @iabynOn Thu, Apr 02, 2015 at 02:58:01PM -0700, Tony Cook via RT wrote:
I think that's the error. SvREFCNT_dec() appears to be decreasing I suspect that's a compiler bug. -- |
From @bulk88On Fri Apr 03 01:30:31 2015, davem wrote:
Can someone attach the broken binary? I want to take a peek in it. -- |
From @steve-m-hayOn Fri Apr 03 08:35:00 2015, bulk88 wrote:
Attached. C:\Dev\Git\perl\win32>..\miniperl.exe -I..\lib -f ..\write_buildcustomize.pl .. |
From @bulk88On Fri Apr 03 10:09:21 2015, shay wrote:
Does the perl521.dll version crash and can you attach the crashing perl.exe and perl521.dll, preferably unstripped? I can't really analyze the GCC miniperl.exe since it doesn't export anything (except accidental exports) and doesn't have symbols. Mingw64-64 4.8.2 isn't crashing for me, with out Cfg=Debug and with it on. C:\p521\srcpara>gcc -v C:\p521\srcpara> BEGIN { C:\p521\srcpara>miniperl.exe -Ilib c.pl C:\p521\srcpara> Also Strawberry 5.20-32 comes with 4.8.3, not 4.8.2, and it doesn't crash either. -v reproduced below of SP 5.20's GCC Using built-in specs. -- |
From @steve-m-hayOn Sun Apr 05 02:08:22 2015, bulk88 wrote:
It doesn't get as far as building perl521.dll/perl.exe because the miniperl.exe that would be used to build them crashes. It crashes in either release or debug (CFG=Debug) configurations. I've attached the miniperl.exe from a debug build in case its of any use, but it sounds like it won't be. I will try putting a miniperl from a different perl build in place and see if that lets the build continue to produce a perl521.dll/perl.exe... Meanwhile, here is the configuration of the MinGW-w64 gcc-4.8.0 that I'm using (It's a 64-bit Windows build which targets 64-bit Windows. The download file is called "x86_64-w64-mingw32-gcc-4.8.0-win64_rubenvb.7z" and I downloaded it from http://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win64/Personal%20Builds/rubenvb/gcc-4.8-release/ ): Using built-in specs. |
From @steve-m-hayOn Sun Apr 05 09:40:01 2015, shay wrote:
I put a miniperl.exe from a gcc-4.7.2 build into place and that allowed the gcc-4.8.0 build to complete, but the resulting perl.exe doesn't suffer the same problem as the miniperl.exe did, even when used to run the same write_buildcustomize.pl command. |
From @bulk88On Sun Apr 05 10:05:55 2015, shay wrote:
I can reproduce now with the rubenvb build. But if I breath at the bug, the attempt to free unreferenced goes away. This tiny little patch fixed it for me. Also -O0 and -O1 dont have the bug. -- |
From @bulk880001-add-watchsv.patchFrom 564879a4ea0ae4391693b5f1c8c9237f96c64da8 Mon Sep 17 00:00:00 2001
From: Daniel Dragan <bulk88@hotmail.com>
Date: Sun, 5 Apr 2015 19:21:58 -0400
Subject: [PATCH] add watchsv
---
inline.h | 4 ++++
perlvars.h | 1 +
sv.c | 1 +
3 files changed, 6 insertions(+)
diff --git a/inline.h b/inline.h
index 46a8cb6..2919384 100644
--- a/inline.h
+++ b/inline.h
@@ -159,6 +159,8 @@ PERL_STATIC_INLINE void
S_SvREFCNT_dec(pTHX_ SV *sv)
{
if (LIKELY(sv != NULL)) {
+ if(sv == PL_watch_sv)
+ DebugBreak();
U32 rc = SvREFCNT(sv);
if (LIKELY(rc > 1))
SvREFCNT(sv) = rc - 1;
@@ -171,6 +173,8 @@ PERL_STATIC_INLINE void
S_SvREFCNT_dec_NN(pTHX_ SV *sv)
{
U32 rc = SvREFCNT(sv);
+ if(sv == PL_watch_sv)
+ DebugBreak();
if (LIKELY(rc > 1))
SvREFCNT(sv) = rc - 1;
else
diff --git a/perlvars.h b/perlvars.h
index 7bafa40..c2262e8 100644
--- a/perlvars.h
+++ b/perlvars.h
@@ -237,3 +237,4 @@ PERLVAR(G, malloc_mutex, perl_mutex) /* Mutex for malloc */
PERLVARI(G, hash_seed_set, bool, FALSE) /* perl.c */
PERLVARA(G, hash_seed, PERL_HASH_SEED_BYTES, unsigned char) /* perl.c and hv.h */
+PERLVARI(G, watch_sv, SV *, NULL)
diff --git a/sv.c b/sv.c
index 467dc24..67524cc 100644
--- a/sv.c
+++ b/sv.c
@@ -7066,6 +7066,7 @@ Perl_sv_free2(pTHX_ SV *const sv, const U32 rc)
}
#endif
/* This may not return: */
+ DebugBreak();
Perl_warner(aTHX_ packWARN(WARN_INTERNAL),
"Attempt to free unreferenced scalar: SV 0x%"UVxf
pTHX__FORMAT, PTR2UV(sv) pTHX__VALUE);
--
1.8.0.msysgit.0
|
From @bulk88On Sun Apr 05 16:30:37 2015, bulk88 wrote: PERL_STATIC_INLINE void fixes it. IDK why yet. -- |
From @bulk88The sin that http://perl5.git.perl.org/perl.git/commitdiff/ab57679753417e2765475e79c13a2adc48615ac0 "add S_op_clear_gv() to op.c" did was creating a pointer alias. GV * gv, in 1 one branch might be created from "(GV*(*svp)". Then in quick succession, and function call free (side effects dont have to be stored to mem until the next function call (ABI reasons)), this executes, ++ the SV/GV *'s refcnt by 1. if (still_valid) then right after #ifdef USE_ITHREADS we -- the same SV *, but through a different C auto. Since GCC -O2 always includes "-fstrict-aliasing" by default, 2 different C autos holding the same ptr is forbidden on paper (if I understand the concept correctly). VC doesn't do such dangerous optimizations so life is easy in VC land :D I've included 2 pics of the asm code showing how the volatile version that writes to C stack, writes, then reads, while the version without volatile, READS, THEN WRITES a stale copy. Doing the read before the write, is http://en.wikipedia.org/wiki/Instruction-level_parallelism or breaking up dependencies/pipline stalls in the CPU. In designing a CPU, writes to memory can be slower/lazier than reads from memory, since a slow read stops the CPU, while a slow write is invisible, and the next instruction can execute immediately. So GCC's "instruction scheduler" reordered them. The real bug is GCC Win32 miniperl failed to do -fno-strict-aliasing the way Win32 GCC full perl does. The -fno-strict-aliasing for full Win32 perl is from http://perl5.git.perl.org/perl.git/commitdiff/7a1f88acf2a2aec3c61c878c837070234df78c08 from 2000 with nothing that I can find (maybe you can find somethign) on the ML from Sarathy so it is unknown why he did that. I've attached a patch which fixes the lack of Win32 GCC miniperl not having fno-strict-aliasing. I have no comment on whether fno-strict-aliasing is appropriate for the perl 5 project or not but I did find this thread from 1999 when GCC first implemented the optimization. http://markmail.org/message/v5r4lylkgv5zk2uk -- |
From @bulk880001-fix-123976-Win32-GCC-miniperl-needs-fno-strict-alias.patchFrom 72e70f1bc568a8c06a2dd1d6ee9eef356e05d945 Mon Sep 17 00:00:00 2001
From: Daniel Dragan <bulk88@hotmail.com>
Date: Sun, 5 Apr 2015 21:39:14 -0400
Subject: [PATCH] fix #123976 Win32 GCC miniperl needs -fno-strict-aliasing
just like full perl uses -fno-strict-aliasing
---
win32/makefile.mk | 1 +
1 file changed, 1 insertion(+)
diff --git a/win32/makefile.mk b/win32/makefile.mk
index 5fd10dc..34dd7e9 100644
--- a/win32/makefile.mk
+++ b/win32/makefile.mk
@@ -511,6 +511,7 @@ EXEOUT_FLAG = -o
LIBOUT_FLAG =
BUILDOPT += -fno-strict-aliasing -mms-bitfields
+MINIBUILDOPT += -fno-strict-aliasing
.ELSE
--
1.8.0.msysgit.0
|
From @bulk880001-selective-volatile-in-S_SvREFCNT_dec.patchFrom 270a7da6c53a7fbe98992b850a9271a8bd808425 Mon Sep 17 00:00:00 2001
From: Daniel Dragan <bulk88@hotmail.com>
Date: Sun, 5 Apr 2015 21:15:39 -0400
Subject: [PATCH] selective volatile in S_SvREFCNT_dec
---
inline.h | 3 +++
op.c | 32 ++++++++++++++++++++++++++++++--
2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/inline.h b/inline.h
index 46a8cb6..92fe94b 100644
--- a/inline.h
+++ b/inline.h
@@ -155,6 +155,8 @@ S_SvREFCNT_inc_void(SV *sv)
if (LIKELY(sv != NULL))
SvREFCNT(sv)++;
}
+
+#ifndef NODEC
PERL_STATIC_INLINE void
S_SvREFCNT_dec(pTHX_ SV *sv)
{
@@ -166,6 +168,7 @@ S_SvREFCNT_dec(pTHX_ SV *sv)
Perl_sv_free2(aTHX_ sv, rc);
}
}
+#endif
PERL_STATIC_INLINE void
S_SvREFCNT_dec_NN(pTHX_ SV *sv)
diff --git a/op.c b/op.c
index ad0b0b0..cc59ad4 100644
--- a/op.c
+++ b/op.c
@@ -97,7 +97,8 @@ recursive, but it's recursive on basic blocks, not on tree nodes.
saves the current C<PL_compiling.cop_hints_hash> on the save stack, so that
it will be correctly restored when any inner compiling scope is exited.
*/
-
+#define NODEC
+#define S_SvREFCNT_dec S_SvREFCNT_decNV
#include "EXTERN.h"
#define PERL_IN_OP_C
#include "perl.h"
@@ -105,6 +106,29 @@ recursive, but it's recursive on basic blocks, not on tree nodes.
#include "feature.h"
#include "regcomp.h"
+PERL_STATIC_INLINE void
+S_SvREFCNT_decV(pTHX_ SV *sv)
+{
+ if (LIKELY(sv != NULL)) {
+ volatile U32 rc = SvREFCNT(sv);
+ if (LIKELY(rc > 1))
+ SvREFCNT(sv) = rc - 1;
+ else
+ Perl_sv_free2(aTHX_ sv, rc);
+ }
+}
+PERL_STATIC_INLINE void
+S_SvREFCNT_decNV(pTHX_ SV *sv)
+{
+ if (LIKELY(sv != NULL)) {
+ U32 rc = SvREFCNT(sv);
+ if (LIKELY(rc > 1))
+ SvREFCNT(sv) = rc - 1;
+ else
+ Perl_sv_free2(aTHX_ sv, rc);
+ }
+}
+
#define CALL_PEEP(o) PL_peepp(aTHX_ o)
#define CALL_RPEEP(o) PL_rpeepp(aTHX_ o)
#define CALL_OPFREEHOOK(o) if (PL_opfreehook) PL_opfreehook(aTHX_ o)
@@ -800,7 +824,9 @@ void S_op_clear_gv(pTHX_ OP *o, PADOFFSET *ixp)
void S_op_clear_gv(pTHX_ OP *o, SV**svp)
#endif
{
-
+/* change from S_SvREFCNT_decV to S_SvREFCNT_decNV to see crash */
+#undef S_SvREFCNT_dec
+#define S_SvREFCNT_dec S_SvREFCNT_decV
GV *gv = (o->op_type == OP_GV || o->op_type == OP_GVSV
|| o->op_type == OP_MULTIDEREF)
#ifdef USE_ITHREADS
@@ -842,6 +868,8 @@ void S_op_clear_gv(pTHX_ OP *o, SV**svp)
if (try_downgrade)
gv_try_downgrade(gv);
}
+#undef S_SvREFCNT_dec
+#define S_SvREFCNT_dec S_SvREFCNT_decNV
}
--
1.8.0.msysgit.0
|
From @bulk88 |
From @bulk88 |
From @iabynOn Sun, Apr 05, 2015 at 06:55:57PM -0700, bulk88 via RT wrote:
That commit didn't introduce the aliasing - that was already present - it
I think that is the correct approach.
I think it's its definitely needed. For example it's a common core switch (sv->sv_type) { (Although it's perhaps less common to continue using the first var after -- |
From @steve-m-hayOn 6 April 2015 at 11:20, Dave Mitchell <davem@iabyn.com> wrote:
Confirmed it fixes the problem here too, so now applied in commit Many thanks for fixing this! |
@steve-m-hay - Status changed from 'open' to 'pending release' |
From @khwilliamsonThank you for submitting this ticket. The issue should now be resolved with the release today of Perl v5.22, which is available at http://www.perl.org/get.html |
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#123976 (status was 'resolved')
Searchable as RT123976$
The text was updated successfully, but these errors were encountered: