Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.9k stars 540 forks source link

splice generates undef? #7403

Closed p5pRT closed 20 years ago

p5pRT commented 20 years ago

Migrated from rt.perl.org#30568 (status was 'resolved')

Searchable as RT30568$

p5pRT commented 20 years ago

From perl@nevcal.com

Created by perl@nevcal.com

Apologies if this is fixed in newer versions; could someone please test one or more of them against this report? I've been trying to keep my environment stable during a long development cycle\, rather than tracking the latest perl.

Where does the undef come from in the below? Looks like handling [ -2 ] is unique.... the first and last cases both work as expected\, but not the middle case.

d​:\MY\PERL\src>type test.pl type test.pl @​stk = ( 1\, 2\, 3 ); print "@​stk!\n"; splice @​stk\, -2\, 2\, $stk[ -1 ]; print "@​stk!\n"; @​stk = ( 1\, 2\, 3 ); print "@​stk!\n"; splice @​stk\, -2\, 2\, $stk[ -2 ]; print "@​stk!\n"; @​stk = ( 1\, 2\, 3 ); print "@​stk!\n"; splice @​stk\, -2\, 2\, $stk[ -3 ]; print "@​stk!\n"; __END__

d​:\MY\PERL\src>perl -w test.pl perl -w test.pl 1 2 3! 1 3! 1 2 3! Use of uninitialized value in join or string at test.pl line 8. 1 ! 1 2 3! 1 1!

Perl Info ``` Flags: category=core severity=critical Site configuration information for perl v5.8.0: Configured by ActiveState at Tue Feb 4 18:07:44 2003. Summary of my perl5 (revision 5 version 8 subversion 0) configuration: Platform: osname=MSWin32, osvers=4.0, archname=MSWin32-x86-multi-thread uname='' config_args='undef' hint=recommended, useposix=true, d_sigaction=undef usethreads=undef use5005threads=undef useithreads=define usemultiplicity=def ine useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cl', ccflags ='-nologo -Gf -W3 -MD -Zi -DNDEBUG -O1 -DWIN32 -D_CONSOLE - DNO_STRICT -DHAVE_DES_FCRYPT -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_ PERLIO -DPERL_MSVCRT_READFIX', optimize='-MD -Zi -DNDEBUG -O1', cppflags='-DWIN32' ccversion='', gccversion='', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=10 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='__int64', lseeksi ze=8 alignbytes=8, prototype=define Linker and Libraries: ld='link', ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf -libpath:"C: \Perl\lib\CORE" -machine:x86' libpth="C:\Perl\lib\CORE" libs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32 .lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib wsoc k32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib msvcrt.lib perllibs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comd lg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib wsock32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib msvcrt.lib libc=msvcrt.lib, so=dll, useshrplib=yes, libperl=perl58.lib gnulibc_version='undef' Dynamic Linking: dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug -opt:ref,icf - libpath:"C:\Perl\lib\CORE" -machine:x86' Locally applied patches: ACTIVEPERL_LOCAL_PATCHES_ENTRY @INC for perl v5.8.0: C:/Perl/lib C:/Perl/site/lib . Environment for perl v5.8.0: HOME (unset) LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=c:\program files\imagemagick-5.5.5-q8;d:\my\ntpgms;d:\my\perl;d:\emacs\ bin;C:\Perl\bin;C:\WINNT\system32;C:\WINNT;C:\WINNT\System32\Wbem;C:\Program Fil es\Support Tools;d:\my\pgms PERL_BADLANG (unset) SHELL (unset) -- Glenn -- http://nevcal.com/ =========================== The best part about procrastination is that you are never bored, because you have all kinds of things that you should be doing. ```
p5pRT commented 20 years ago

From @iabyn

On Sat\, Jul 03\, 2004 at 06​:17​:22AM -0000\, Glenn Linderman wrote​:

Apologies if this is fixed in newer versions; could someone please test one or more of them against this report? I've been trying to keep my environment stable during a long development cycle\, rather than tracking the latest perl.

Where does the undef come from in the below? Looks like handling [ -2 ] is unique.... the first and last cases both work as expected\, but not the middle case.

d​:\MY\PERL\src>type test.pl type test.pl @​stk = ( 1\, 2\, 3 ); print "@​stk!\n"; splice @​stk\, -2\, 2\, $stk[ -1 ]; print "@​stk!\n"; @​stk = ( 1\, 2\, 3 ); print "@​stk!\n"; splice @​stk\, -2\, 2\, $stk[ -2 ]; print "@​stk!\n"; @​stk = ( 1\, 2\, 3 ); print "@​stk!\n"; splice @​stk\, -2\, 2\, $stk[ -3 ]; print "@​stk!\n"; __END__

d​:\MY\PERL\src>perl -w test.pl perl -w test.pl 1 2 3! 1 3! 1 2 3! Use of uninitialized value in join or string at test.pl line 8. 1 ! 1 2 3! 1 1!

Two things are interracting here to produce the effect you see. First\, Perl doesn't reference count things pushed onto the stack\, so for example

  sub f { @​a=(); print $_[0]\,"\n" }   @​a = (1);   f();

prints an uninit value. This menas that when splice deletes elements [1\,2]\, they get immediately freed\, leaving the freed value $stk[-2] or whatever on the stack to be later inserted into the array.

Second\, splice in a void context behaves the same is in scalar context\, ie it returns the last element removed. This 'protects' one of the elements and extends its life\, which is why you don't aways see the effect.

Not sure of the best fix yet.

-- Little fly\, thy summer's play my thoughtless hand has terminated with extreme prejudice.   (with apologies to William Blake)

p5pRT commented 20 years ago

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

p5pRT commented 20 years ago

From @pjcj

On Sat\, Jul 03\, 2004 at 12​:16​:12PM +0100\, Dave Mitchell wrote​:

First\, Perl doesn't reference count things pushed onto the stack\, so for example

sub f \{ @​a=\(\); print $\_\[0\]\,"\\n" \}
@​a = \(1\);
f\(\);

prints an uninit value.

I suppose this is also the reason for this bug\, reported at http​://bugs.debian.org/cgi-bin/bugreport.cgi?bug=248784

  sub foo {   print STDERR "\@​_ = ("\,join("\,"\,@​_)\,")\n" ;   $arrayref = 0 ;   print STDERR "\@​_ = ("\,join("\,"\,@​_)\,")\n" ;   $_[0]->[0] ;   print STDERR "\@​_ = ("\,join("\,"\,@​_)\,")\n" ;   pop @​_ ;   print STDERR "I survived!" ;   }   $arrayref = [ 17 \, [19] \, 21 ] ;   foo @​{$arrayref} ;

  Sample output​:

  $ perl perlbug.pl   @​_ = (17\,ARRAY(0x814cbb8)\,21)   @​_ = (\,\,)   @​_ = (ARRAY(0x814cad4)\,ARRAY(0x814cad4)\,)   Segmentation fault   $

-- Paul Johnson - paul@​pjcj.net http​://www.pjcj.net

p5pRT commented 20 years ago

From perl5-porters@ton.iguana.be

In article \20040703111612\.GA1887@​iabyn\.com\,   Dave Mitchell \davem@​iabyn\.com writes​:

Two things are interracting here to produce the effect you see. First\, Perl doesn't reference count things pushed onto the stack\, so for example

sub f \{ @​a=\(\); print $\_\[0\]\,"\\n" \}
@​a = \(1\);
f\(\);

prints an uninit value. This menas that when splice deletes elements [1\,2]\, they get immediately freed\, leaving the freed value $stk[-2] or whatever on the stack to be later inserted into the array.

Second\, splice in a void context behaves the same is in scalar context\, ie it returns the last element removed. This 'protects' one of the elements and extends its life\, which is why you don't aways see the effect.

Not sure of the best fix yet.

While finding a fix would be neat\, I don't think it *HAS* to be fixed (unless it can lead to refcount problems) since strictly speaking it's like using $i + $i++ : you shouldn't depend on where exactly in the splice process the actual value fetch takes place since perl isn't pass by value but pass by reference.

It's not really different from e.g.​:

perl -wle 'sub foo { shift @​{$_[0]}; print $_[1] } @​a="a".."z"; foo(\@​a\,$a[0])' Use of uninitialized value in print at -e line 1.

p5pRT commented 20 years ago

From perl@nevcal.com

On approximately 7/3/2004 4​:16 AM\, came the following characters from the keyboard of Dave Mitchell​:

On Sat\, Jul 03\, 2004 at 06​:17​:22AM -0000\, Glenn Linderman wrote​:

Apologies if this is fixed in newer versions; could someone please test one or more of them against this report? I've been trying to keep my environment stable during a long development cycle\, rather than tracking the latest perl.

Where does the undef come from in the below? Looks like handling [ -2 ] is unique.... the first and last cases both work as expected\, but not the middle case.

d​:\MY\PERL\src>type test.pl type test.pl @​stk = ( 1\, 2\, 3 ); print "@​stk!\n"; splice @​stk\, -2\, 2\, $stk[ -1 ]; print "@​stk!\n"; @​stk = ( 1\, 2\, 3 ); print "@​stk!\n"; splice @​stk\, -2\, 2\, $stk[ -2 ]; print "@​stk!\n"; @​stk = ( 1\, 2\, 3 ); print "@​stk!\n"; splice @​stk\, -2\, 2\, $stk[ -3 ]; print "@​stk!\n"; __END__

d​:\MY\PERL\src>perl -w test.pl perl -w test.pl 1 2 3! 1 3! 1 2 3! Use of uninitialized value in join or string at test.pl line 8. 1 ! 1 2 3! 1 1!

Two things are interracting here to produce the effect you see.

I appreciate your confirmation that there is a problem\, and your testing it on a newer Perl.

I don't understand your explanation\, as noted below\, and although I'm interested in understanding it\, that is less important to me than your ability to find a fix.

On the other hand\, a fix won't help me for months\, likely\, and I have figured out that the sequence

  my $tmp = $stk[ -2 ];   splice @​stk\, -2\, 2\, $tmp;

works fine\, and would seem to avoid the bug. Although the circumstances   of code in finding this bug were much more complex than the report\, the above technique of using a temporary seems work as a solution\, regardless of the complexity of the expression or list passed as the 4th parameter to splice.

First\, Perl doesn't reference count things pushed onto the stack\, so for example

sub f \{ @​a=\(\); print $\_\[0\]\,"\\n" \}
@​a = \(1\);
f\(\);

prints an uninit value.

I don't understand why f shouldn't be expected to print an uninit value... no values were passed to it\, so all of @​_ is uninit... no?

OK\, maybe you meant to f(@​a); ? I see that even that prints an uninit value\, which would be less expected.

OK\, so @​_ is getting only aliases to values? and unreference counted aliases... is that what you are getting at? So that when the @​a=(); assignment is executed it wipes out what the @​_ is pointing out\, because there is no knowledge (reference count) that @​a is being pointed at?

Maybe I'm close to understanding this now.

I don't guess I understand how it becomes undef rather than 67\, though\, or some random memory content.

                    This menas that when splice deletes elements

[1\,2]\, they get immediately freed\, leaving the freed value $stk[-2] or whatever on the stack to be later inserted into the array.

I guess I don't understand this comment either... but maybe it is because I don't fully understand the one above. While I've named the variable @​stk\, which means stack\, that isn't the Perl stack\, which I think you are referring to. The 4th parameter to splice\, in all cases\, is to an element of my variable\, not something pushed on the stack? And it is supposed to be evaluated prior to the splice operation? So unless the problem arises out of optimizations during code generation\, I don't understand.

But maybe... since you bring up the function call and the stack\, I guess splice is implemented as a function\, that does something similar to your code above... getting some aliases as parameters\, but also manipulating the "whole array" that is one of the parameters\, and those manipulations affect the value of what @​_[3] is pointing at (sometimes).

I guess I'm close to understanding this now too\, if all my assumptions are correct.

So another workaround would be

@​stk = ( 1\, 2\, 3 ); print "@​stk!\n"; splice @​stk\, -2\, 2\, 0 + $stk[ -2 ]; print "@​stk!\n";

Curiously\, my original case had an expression\, but it was composed of a ternary operator that resulted (sometimes) in $stk[ -2 ] and sometimes to other things\, and the expression failed only in the case where it resulted in $stk[ -2 ]. So that must have been optimized as "push this" or "push that" onto the parameters for splice.

Second\, splice in a void context behaves the same is in scalar context\, ie it returns the last element removed. This 'protects' one of the elements and extends its life\, which is why you don't aways see the effect.

Not sure of the best fix yet.

-- Glenn -- http​://nevcal.com/

The best part about procrastination is that you are never bored\, because you have all kinds of things that you should be doing.

p5pRT commented 20 years ago

From wolfgang.laun@alcatel.at

On Sat\, Jul 03\, 2004 at 06​:17​:22AM -0000\, Glenn Linderman wrote​:

d​:\MY\PERL\src>type test.pl @​stk = ( 1\, 2\, 3 ); print "@​stk!\n"; splice @​stk\, -2\, 2\, $stk[ -1 ]; print "@​stk!\n"; @​stk = ( 1\, 2\, 3 ); print "@​stk!\n"; splice @​stk\, -2\, 2\, $stk[ -2 ]; print "@​stk!\n"; @​stk = ( 1\, 2\, 3 ); print "@​stk!\n"; splice @​stk\, -2\, 2\, $stk[ -3 ]; print "@​stk!\n";

On Sat\, Jul 03\, 2004 at 12​:16​:12PM +0100\, Dave Mitchell wrote​:

First\, Perl doesn't reference count things pushed onto the stack\,

All right\, I understand that (boast\, boast ;-). Consequentely   for my $i ( -1\, -2\, -3 ){   @​stk = (1\, 2\, 3);   splice @​stk\, -2\, 2\, ${\$stk[$i]};   print @​stk!\n"\,   } avoids the loss of the reference. And so does   for my $i ( -1\, -2\, -3 ){   @​stk = (1\, 2\, 3);   splice @​stk\, -2\, 2\, ( $stk[$i] );   print @​stk!\n"\,   }

Being evil (according to H.Merijn Brand ;-)\, I tried (perl 5.8.0 on Linux and Solaris; 5.8.4 on Solaris)​:   for my $i ( -1\, -2\, -3 ){   @​stk = (1\, 2\, 3);   splice @​stk\, -3\, 3\, $stk[0]\, $stk[$i];   print @​stk!\n"\,   } and promptly got   Use of uninitialized value in join or string at line "print..."   3!   Segmentation fault

Kind regards Wolfgang Laun

p5pRT commented 20 years ago

From @rgs

Dave Mitchell wrote​:

Second\, splice in a void context behaves the same is in scalar context\, ie it returns the last element removed.

Shouldn't this be fixed ? (or "optimized out") ?

p5pRT commented 20 years ago

From @iabyn

On Tue\, Jul 06\, 2004 at 10​:47​:09AM +0200\, Rafael Garcia-Suarez wrote​:

Dave Mitchell wrote​:

Second\, splice in a void context behaves the same is in scalar context\, ie it returns the last element removed.

Shouldn't this be fixed ? (or "optimized out") ?

Yep. I plan to do this too once I find some time to fix splice.

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

p5pRT commented 20 years ago

From wolfgang.laun@alcatel.at

The patch fixes the problems arising from using elements deleted from the spliced array in the replacement list. Appropriate tests have been added to op/splice.t.

On 3 Jul 2004 06​:17​:22 -0000 Glenn Linderman wrote​:

Where does the undef come from in the below?

@​stk = ( 1\, 2\, 3 ); print "@​stk!\n"; splice @​stk\, -2\, 2\, $stk[ -2 ]; print "@​stk!\n";

Dave Mitchell has provided an analysis \20040703111612\.GA1887@​iabyn\.com.

On Sat\, 3 Jul 2004 14​:31​:16 +0000 Ton Hospel wrote​:

While finding a fix would be neat\, I don't think it *HAS* to be fixed (unless it can lead to refcount problems) since strictly speaking it's like using $i + $i++ : you shouldn't depend on where exactly in the splice process the actual value fetch takes place since perl isn't pass by value but pass by reference.

I disagree\, since a single call to splice() shouldn't considered as being in the same league as the quoted "$i + $i++" which contains two distinct op's (fetch\, inc) on the same object. The fact that splice uses pass by reference for the list of new elements is irrelevant since the outcome depends on the time these references are *used* in the process of doing the splice. (Actually\, it isn't necessary to do the deletion before the insertion\, this is an implementation decision. And it's easy to make splice safe - see below.)

The patch is simple​: Don't postpone the creation of SV's for the new elements until they are copied into the array. If this is done up front\, all new elements are safe (at the cost of a few extra cycles).

Regards Wolfgang

Inline Patch ```diff --- pp.c.old Fri Jul 9 13:25:23 2004 +++ pp.c Fri Jul 9 20:43:06 2004 @@ -4125,6 +4125,13 @@ if (newlen && !AvREAL(ary) && AvREIFY(ary)) av_reify(ary); + /* make new elements SVs now: avoid problems if they're from the array */ + for (dst = MARK, i = newlen; i; i--) { + SV *h = *dst; + *dst = NEWSV(46, 0); + sv_setsv(*dst++, h); + } + if (diff < 0) { /* shrinking the area */ if (newlen) { New(451, tmparyval, newlen, SV*); /* so remember insertion */ @@ -4181,11 +4188,7 @@ dst[--i] = &PL_sv_undef; if (newlen) { - for (src = tmparyval, dst = AvARRAY(ary) + offset; - newlen; newlen--) { - *dst = NEWSV(46, 0); - sv_setsv(*dst++, *src++); - } + Copy( tmparyval, AvARRAY(ary) + offset, newlen, SV* ); Safefree(tmparyval); } } @@ -4224,11 +4227,11 @@ } } - for (src = MARK, dst = AvARRAY(ary) + offset; newlen; newlen--) { - *dst = NEWSV(46, 0); - sv_setsv(*dst++, *src++); + if (newlen) { + Copy( MARK, AvARRAY(ary) + offset, newlen, SV* ); } + MARK = ORIGMARK + 1; if (GIMME == G_ARRAY) { /* copy return vals to stack */ if (length) { --- t/op/splice.t.old Fri Jul 9 19:21:39 2004 +++ t/op/splice.t Fri Jul 9 20:08:21 2004 @@ -1,6 +1,6 @@ #!./perl -print "1..12\n"; +print "1..18\n"; @a = (1..10); @@ -52,3 +52,33 @@ print "not " unless $foo eq 'red'; print "ok 12\n"; +# Bug [perl #30568] - insertions of deleted elements +@a = (1, 2, 3); +splice( @a, 0, 3, $a[1], $a[0] ); +print "not " unless j(@a) eq j(2,1); +print "ok 13\n"; + +@a = (1, 2, 3); +splice( @a, 0, 3 ,$a[0], $a[1] ); +print "not " unless j(@a) eq j(1,2); +print "ok 14\n"; + +@a = (1, 2, 3); +splice( @a, 0, 3 ,$a[2], $a[1], $a[0] ); +print "not " unless j(@a) eq j(3,2,1); +print "ok 15\n"; + +@a = (1, 2, 3); +splice( @a, 0, 3, $a[0], $a[1], $a[2], $a[0], $a[1], $a[2] ); +print "not " unless j(@a) eq j(1,2,3,1,2,3); +print "ok 16\n"; + +@a = (1, 2, 3); +splice( @a, 1, 2, $a[2], $a[1] ); +print "not " unless j(@a) eq j(1,3,2); +print "ok 17\n"; + +@a = (1, 2, 3); +splice( @a, 1, 2, $a[1], $a[1] ); +print "not " unless j(@a) eq j(1,2,2); +print "ok 18\n"; ```
p5pRT commented 20 years ago

From @rgs

LAUN Wolfgang wrote​:

The patch fixes the problems arising from using elements deleted from the spliced array in the replacement list. Appropriate tests have been added to op/splice.t.

Thanks\, applied to bleadperl as #23092.

p5pRT commented 20 years ago

@rspier - Status changed from 'open' to 'resolved'