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

splice generates undef? #7403

Closed
p5pRT opened this issue Jul 3, 2004 · 12 comments
Closed

splice generates undef? #7403

p5pRT opened this issue Jul 3, 2004 · 12 comments

Comments

@p5pRT
Copy link

p5pRT commented Jul 3, 2004

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

Searchable as RT30568$

@p5pRT
Copy link
Author

p5pRT commented Jul 3, 2004

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
Copy link
Author

p5pRT commented Jul 3, 2004

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
Copy link
Author

p5pRT commented Jul 3, 2004

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

@p5pRT
Copy link
Author

p5pRT commented Jul 3, 2004

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
Copy link
Author

p5pRT commented Jul 3, 2004

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 \{ @&#8203;a=\(\); print $\_\[0\]\,"\\n" \}
@&#8203;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
Copy link
Author

p5pRT commented Jul 3, 2004

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 \{ @&#8203;a=\(\); print $\_\[0\]\,"\\n" \}
@&#8203;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
Copy link
Author

p5pRT commented Jul 5, 2004

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
Copy link
Author

p5pRT commented Jul 6, 2004

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
Copy link
Author

p5pRT commented Jul 6, 2004

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
Copy link
Author

p5pRT commented Jul 12, 2004

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
--- 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
Copy link
Author

p5pRT commented Jul 13, 2004

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
Copy link
Author

p5pRT commented Jul 27, 2004

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

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

1 participant