Bleadperl breaks TOKUHIROM/Test-SharedFork-0.24 on Windows #13763
Comments
From @chornyCreated by @chornyTest 05_nest.t has segfault, but only if tests are called as "&main". With Error reported here: tokuhirom/Test-SharedFork#13 Tested on Strawberry perl 5.9.11. Perl Info |
From @bulk88On Wed Apr 23 13:55:56 2014, chorny wrote:
VC 2003 32 bit 5.19.10 ish.
Unhandled exception at 0x28062a1c (perl519.dll) in perl.exe: 0xC0000005: Access violation reading location 0xabababb3. SV * sstr is 0xabababb3 /* duplicate an SV of any type (including AV, HV etc) */ static SV * PERL_ARGS_ASSERT_SV_DUP_COMMON; if (SvTYPE(sstr) == (svtype)SVTYPEMASK) {<<<<<<<<<<<<<<<CRASH attached pic of var dump of var context * ncx in Perl_cx_dup. -- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @bulk88No idea why the patch works or if its appropriate but it stopped the crash. Without a rational of why these werent null before. Or why it didn't crash before. This shouldn't be applied. I got no crashed on AP 5.10 and VC 5.12 and SP Win32 strawberry-perl 5.18.0.1. It might be because the bug isn't there in older Perls, or by chance the uninit memory was filled with NULL so no crash. -- |
From @bulk880001-WIP-no-idea-why-it-works-or-if-its-correct.patchFrom 5ef2f410578fbe4a6cfd9b8f8d22661224afad8f Mon Sep 17 00:00:00 2001
From: Daniel Dragan <bulk88@hotmail.com>
Date: Wed, 23 Apr 2014 19:21:04 -0400
Subject: [PATCH] WIP no idea why it works or if its correct
---
pp_hot.c | 5 ++++-
pp_sort.c | 5 ++++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/pp_hot.c b/pp_hot.c
index ac69bc7..61acdf5 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -2715,7 +2715,10 @@ try_autoload:
}
MARK++;
}
- }
+ } else {
+ cx->blk_sub.savearray = NULL;
+ cx->blk_sub.argarray = NULL;
+ }
SAVETMPS;
if (UNLIKELY((cx->blk_u16 & OPpENTERSUB_LVAL_MASK) == OPpLVAL_INTRO &&
!CvLVALUE(cv)))
diff --git a/pp_sort.c b/pp_sort.c
index 0fe0411..ff5f34d 100644
--- a/pp_sort.c
+++ b/pp_sort.c
@@ -1695,7 +1695,10 @@ PP(pp_sort)
GvAV(PL_defgv) = MUTABLE_AV(SvREFCNT_inc_simple(av));
CX_CURPAD_SAVE(cx->blk_sub);
cx->blk_sub.argarray = av;
- }
+ } else {
+ cx->blk_sub.savearray = NULL;
+ cx->blk_sub.argarray = NULL;
+ }
}
}
--
1.7.9.msysgit.0
|
From @chornyI've simplified this code, so it can be converted to test. &main; P.S. Is run_multiple_progs from test.pl documented anywhere? -- |
From [Unknown Contact. See original ticket]I've simplified this code, so it can be converted to test. &main; P.S. Is run_multiple_progs from test.pl documented anywhere? -- |
From @iabynOn Wed, Apr 23, 2014 at 04:29:48PM -0700, bulk88 via RT wrote:
It might be better to just skip duping savearray and argarray As to why it didn't previously fail, I don't know. New CX stacks have -- |
From @bulk88On Wed Apr 23 17:08:50 2014, chorny wrote:
Crashed on DEBUGGING VC 2003 32 bit with psuedofork 5.12. perl512.dll!Perl_sv_dup(interpreter * my_perl=0x00830eec, const sv * const sstr=0xabababab, clone_params * const param=0x0006fa48) Line 10966 + 0x3 C No crash on AP 5.10, but that isn't proof the bug isn't in 5.10 since this is uninit mem bug. -- |
From @bulk88On Thu Apr 24 08:36:45 2014, davem wrote:
Why are we poisoning *anything*at*all* in a non-PERL_POISON or atleast a non-DEBUGGING build? (note DEBUGGING builds atleast on Win32 aren't PERL_POISON builds (why? (valgrind?)), I've used -DPERL_POISON a couple times in history). The 2 scope.c mentioned in this patch, which is were I *think* (and I know as much about this area perl internals as a drunk bum) context structs are alloced, git blame to http://perl5.git.perl.org/perl.git/commitdiff/7e337ee0bc836d3147f3b2579c7e35127637e377 and then to background-less, useless description, commit from 2002/ithreads beginnings era http://perl5.git.perl.org/perl.git/commitdiff/9965345dfe11415fe4409828505acf6c7fe193b9 which sounds like debugging code that someone forgot to remove for a problem that is lost to time. Technically, I think, reverting that commit to zero the structs instead of poisoning and therefore the SV * dup code will return a NULL SV* in child thread when passes NULL SV * (designed behavior) from parent thread instead of duping SV* 0xabababab. Some other questions to answer, since a memset is already done on the context structs, switching it to zero it, instead of poison, and NOT have else {} blocks in the sloppy patch above would be more efficient, or not? Or remove the memset, regardless if its NULLing or poisoning, and just use the else blocks? Should there be asserts for segv readability of those extra members of a context struct? Why don't we have variable length context structs if in some cases we dont ever set later members in it?
set to exactly where (I need a file and line number/C func name)? Kentnl on #p5p wants this to be a 5.20 blocker, something about Catalyst he said. IDK when I'll have time to deal with this further, and I might not have any time before 5-20-14. TonyC/SteveH feel free to fix it if I dont get around to it. -- |
From @iabynOn Fri, Apr 25, 2014 at 12:56:30AM -0700, bulk88 via RT wrote:
Well the comments in the code say: /* Without any kind of initialising PUSHSUBST() Based on that and the commit history, it appears that for some reason
Note that this poisoning is only done when the stack is first allocated, Reemoving the poisoning wont fix this bug, it just means that whatever is
see above.
Because that would complicate matters. The only time these fields aren't &foo; (as oppposed to foo() or &foo()). In this case the CxHASARGS() flag wont be set, and all the relevant code So I still think the simple and correct fix for this is to test for
In Perl_cx_dup() at the point where it tries to dup() the 'bad' savearray; -- |
From @bulk88Crashed in same way on Perl 5.10.1 DEBUGGING. C:\Documents and Settings\Owner\Desktop>perl -V
but it doesn't crash on AP 5.10.0. I'll try building a debugging 5.10.0. -- |
From @bulk88On Sat Apr 26 15:14:57 2014, bulk88 wrote:
DEBUGGING 5.10.0 crashes. The member offset of cx->blk_sub.savearray in context struct between my DEBUGGING 5.10.0 and my AP 5.10.0 are different asm wise. Still trying to figure out exactly why AP 5.10.0 isn't crashing. -- |
From @bulk88On Sat Apr 26 20:11:20 2014, bulk88 wrote:
AP, -O1 10363: ncx->blk_sub.savearray = av_dup_inc(cx->blk_sub.savearray, param); DEBUGGING -Od 10363: ncx->blk_sub.savearray = av_dup_inc(cx->blk_sub.savearray, param); Offsets are different because a compiler optimization keeps the context * at +0xC for the life of the C function instead of 0x0, and 0xC+0x18 add to 0x24. False alarm. The context struct contents in Perl_cx_dup is different between AP 5.10.0 and my DEBUGGING 5.10.0 build. IDK why yet. See attached pics, check names of the files to know which is which. -- |
From @bulk88On Sat Apr 26 21:14:33 2014, bulk88 wrote:
Compiled a non-DEBUGGING 5.10.0, same crash as DEBUGGING with 0xabXXXXXX SV *. Which means the AP 5.10.0 probably has a patch that isn't in blead/p5p 5.10 or 5.12. *sigh* -- |
From @bulk88On Sat Apr 26 21:20:31 2014, bulk88 wrote:
pp_entersub in the AP, var hasargs is 1. My VC Perl, hasargs is 0. So in the AP, later on, savearray is assigned to. This doesn't happen in my VC Perl. But in the AP, it looks like the OP struct is malformed. Putting a breakpoint in entersub with callstack perl510.dll!Perl_pp_entersub(interpreter * my_perl=0x00343e84) Line 2636 C shows 2 different OP structs in PL_op, the AP one doesn't have OPf_STACKED (0x40) in its flags. |
From @bulk88 |
From @bulk88On Sun Apr 27 10:31:55 2014, bulk88 wrote:
Correction, AP5100 has OPf_STACKED, my 5100DBG doesn't. Note the AP one has no pp_addr. I wrote a script (noargscrash.pl) to dump the optree, it now crashes AP5100 in a similar but different way as in 5100DBG. It is attached. AP running noargscrash.pl perl510.dll!Perl_sv_dup(interpreter * my_perl=0x019437bc, const sv * sstr=0x0000004c, clone_params * param=0x0140fa70) Line 10012 + 0xb C 5100 no dbging perl510.dll!Perl_sv_dup(interpreter * my_perl=0x019425a4, const sv * sstr=0x0000004c, clone_params * param=0x0140fa50) Line 10012 + 0x9 C AP C:\Documents and Settings\Owner\Desktop>perl noargcrash.pl This is perl, v5.10.0 built for MSWin32-x86-multi-thread Copyright 1987-2007, Larry Wall Binary build 1003 [285500] provided by ActiveState http://www.ActiveState.com Perl may be copied only under the terms of either the Artistic License or the Complete documentation for Perl, including FAQ lists, should be found on C:\Documents and Settings\Owner\Desktop> My 5100 C:\Documents and Settings\Owner\Desktop>perl noargcrash.pl This is perl, v5.10.0 built for MSWin32-x86-multi-thread Copyright 1987-2007, Larry Wall Perl may be copied only under the terms of either the Artistic License or the Complete documentation for Perl, including FAQ lists, should be found on C:\Documents and Settings\Owner\Desktop> IDK how to dump the root psuedo CV with Concise, since if the "&main;" call is in a sub, a different bug/callstack crash happens, vs if "&main;" is in root psuedo sub. -- |
From @bulk88 |
From @bulk88noargcrash.pl does *NOT* crash on blead. C:\Documents and Settings\Owner\Desktop>perl noargcrash.pl C:\Documents and Settings\Owner\Desktop>perl -v This is perl 5, version 19, subversion 11 (v5.19.11) built for MSWin32-x86-multi Copyright 1987-2014, Larry Wall Perl may be copied only under the terms of either the Artistic License or the Complete documentation for Perl, including FAQ lists, should be found on C:\Documents and Settings\Owner\Desktop> -- |
From @bulk88On Sun Apr 27 11:21:58 2014, bulk88 wrote:
Because I patched blead and forgot to revert it. DOPE -- |
From @bulk88On Sun Apr 27 13:17:24 2014, bulk88 wrote:
noargcrash.pl (paramless main call in a sub) on blead and SP 5.18 and 5.12 doesn't crash. On 5.10 it crashes. paramless main call in root crashes on 5.10, 5.12, 5.18, and blead paramless main call in root is &main; -- |
From @iabynOn Sun, Apr 27, 2014 at 10:40:38PM -0700, bulk88 via RT wrote:
I think we can conclude from this that the bug has always been present, commit 9625867 Pseudo-fork dups arg array on argless calls -- |
|
@iabyn - Status changed from 'open' to 'resolved' |
From @bulk88This needs to be reopened until perldelta is written. There was none in http://perl5.git.perl.org/perl.git/commit/96258673547f51dc588c290d9c8ff3d9b2b93397 -- |
|
@tonycoz - Status changed from 'resolved' to 'open' |
From @tonycozOn Wed Apr 23 17:08:50 2014, chorny wrote:
The data format is documented from line 997 in t/test.pl. Tony |
From @rjbsOn Tue Apr 29 08:51:35 2014, bulk88 wrote:
I suggest something along the lines of "a bug leading to stack corruption when calling a sub with ampersand in a pseudo-forking perl has been fixed," in the Selected Bug Fixes section. If you concur, I will apply it in my perldelta branch. -- |
|
@tonycoz - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#121721 (status was 'resolved')
Searchable as RT121721$
The text was updated successfully, but these errors were encountered: