Perl / perl5

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

Attempt to free unreferenced scalar #6854

Closed p5pRT closed 18 years ago

p5pRT commented 21 years ago

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

Searchable as RT24254$

p5pRT commented 21 years ago

From perl-5.8.0@ton.iguana.be

Created by perl-5.8.0@ton.iguana.be

perl -e 'map eval"exit"\,1 for 1' Attempt to free unreferenced scalar.

(In more complex cases this actually causes segmentation faults)

Perl Info ``` Flags: category=core severity=low Site configuration information for perl v5.8.0: Configured by ton at Tue Nov 12 01:56:18 CET 2002. Summary of my perl5 (revision 5.0 version 8 subversion 0) configuration: Platform: osname=linux, osvers=2.4.19, archname=i686-linux-thread-multi-64int-ld uname='linux quasar 2.4.19 #5 wed oct 2 02:34:25 cest 2002 i686 unknown ' config_args='' hint=recommended, useposix=true, d_sigaction=define usethreads=define use5005threads=undef useithreads=define usemultiplicity=define useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=define use64bitall=undef uselongdouble=define usemymalloc=y, bincompat5005=undef Compiler: cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2 -fomit-frame-pointer', cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -I/usr/local/include' ccversion='', gccversion='2.95.3 20010315 (release)', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 ivtype='long long', ivsize=8, nvtype='long double', nvsize=12, Off_t='off_t', lseeksize=8 alignbytes=4, prototype=define Linker and Libraries: ld='cc', ldflags =' -L/usr/local/lib' libpth=/usr/local/lib /lib /usr/lib libs=-lnsl -lndbm -ldb -ldl -lm -lpthread -lc -lposix -lcrypt -lutil perllibs=-lnsl -ldl -lm -lpthread -lc -lposix -lcrypt -lutil libc=/lib/libc-2.2.4.so, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.2.4' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic' cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib' Locally applied patches: @INC for perl v5.8.0: /usr/lib/perl5/5.8.0/i686-linux-thread-multi-64int-ld /usr/lib/perl5/5.8.0 /usr/lib/perl5/site_perl/5.8.0/i686-linux-thread-multi-64int-ld /usr/lib/perl5/site_perl/5.8.0 /usr/lib/perl5/site_perl . Environment for perl v5.8.0: HOME=/home/ton LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/ton/bin.Linux:/home/ton/bin:/home/ton/bin.SampleSetup:/usr/local/bin:/usr/local/sbin:/usr/local/jre/bin:/home/oracle/product/9.0.1/bin:/usr/local/ar/bin:/usr/games/bin:/usr/X11R6/bin:/usr/share/bin:/usr/bin:/usr/sbin:/bin:/sbin:. PERL_BADLANG (unset) SHELL=/bin/bash ```
p5pRT commented 21 years ago

From @schwern

On Mon\, Oct 20\, 2003 at 12​:59​:16PM -0000\, perl-5.8.0@​ton.iguana.be (via RT) wrote​:

perl -e 'map eval"exit"\,1 for 1' Attempt to free unreferenced scalar.

Still exists in 5.8.1.

-- Michael G Schwern schwern@​pobox.com http​://www.pobox.com/~schwern/ Here's hoping you don't harbor a death wish!

p5pRT commented 20 years ago

From perl-bug@ton.iguana.be

This is a bug report for perl from perl-5.8.0@​ton.iguana.be\, generated with the help of perlbug 1.34 running under perl v5.8.0.

----------------------------------------------------------------- [Please enter your report here]

perl -e 'map eval"exit"\,1 for 1' Attempt to free unreferenced scalar.

(In more complex cases this actually causes segmentation faults)

I seem to keep running into what looks like variations of this bug. Now I met​:

perl -e 'my @​foo=1..2; for(1) { () = map z()\, @​foo }' Undefined subroutine &main​::z called at -e line 1. Attempt to free unreferenced scalar​: SV 0x8162170.

So it seems that a non-local exit out of the map inside for triggers it. You also don't have to leave the program completely​: perl -wle 'eval { my @​foo=1..2; for(1) { () = map z()\, @​foo } }; print STDERR "hoi"' Attempt to free unreferenced scalar​: SV 0x8162170 at -e line 1. hoi

I did a bit more internals tracking here\, and the unreferenced scalar in the message is actually $foo[1]. If you have a debugging perl\, you also get a "Attempt to free temp prematurely​: SV 0x8162170"

A traceback at that moment when i was triggering it in another program with a croak() in XS code was (this is versus perl-5.8.5)​: #0 Perl_sv_free (my_perl=0x81ae050\, sv=0x839a104) at sv.c​:5359 #1 0x080df173 in Perl_av_clear (my_perl=0x81ae050\, av=0x835c72c) at av.c​:470 #2 0x0811eb35 in Perl_leave_scope (my_perl=0x81ae050\, base=0) at scope.c​:926 #3 0x0811befd in Perl_pop_scope (my_perl=0x81ae050) at scope.c​:137 #4 0x0806b4a3 in S_my_exit_jump (my_perl=0x81ae050) at perl.c​:4767 #5 0x0806b13c in Perl_my_failure_exit (my_perl=0x81ae050) at perl.c​:4747 #6 0x080cd645 in Perl_vcroak (my_perl=0x81ae050\,   pat=0x40020b8c "recursive heap change"\, args=0xbfffeb40) at util.c​:1209 #7 0x080cd672 in Perl_croak_nocontext (pat=0x40020b8c "recursive heap change")   at util.c​:1219

The av on frame 1 is @​foo.

p5pRT commented 19 years ago

From @salva

this bug still exists on perl 4.8.4

yet another way to trigger it​:

  for (1) { map { die } 2 }

or inside an eval block​:

  eval { for (1) { map { die } 2 } };

cheers.

p5pRT commented 19 years ago

From @salva

I have been looking at this bug and I know why it's happening​:

$ ./miniperl -Dlt -e 'for (125) { map { exit } (213)}'

...

EXECUTING...

(-e​:0) Entering new RUNOPS level (-e​:0) enter (-e​:0) ENTER scope 2 at pp_hot.c​:1650 Entering block 0\, type BLOCK (-e​:0) nextstate (-e​:1) pushmark (-e​:1) const(IV(125)) (-e​:1) gv(main​::_) (-e​:1) enteriter

(-e​:1) ENTER scope 3 at pp_ctl.c​:1805 (-e​:1) ENTER scope 4 at pp_ctl.c​:1834 Entering block 1\, type LOOP   # here PUSHLOOP is called\, and a pointer to   # &(DEFSV) is stored inside the context structure on   # PL_curstackinfo->si_cxstack[1].cx_u.cx_blk.blk_u.blku_loop.itervar   # (set a watch on the debugger for this expression and *expression   # to see the problem)

(-e​:1) iter   # DEFSV = IV(125)

(-e​:1) and (-e​:1) nextstate (-e​:1) pushmark (-e​:1) const(IV(213)) (-e​:1) mapstart (-e​:1) ENTER scope 5 at pp_ctl.c​:903 (-e​:1) ENTER scope 6 at pp_ctl.c​:910   # DEFSV is saved   # DEFSV = IV(213)

(-e​:1) exit Unwinding block 1\, type LOOP (-e​:0) POPBLOCK scope 2 at perl.c​:4834   # on POPBLOCK(1)   # DEFSV *still* points to IV(213)   # and CxITERVAR(cx) points to &(DEFSV)   # so IV(213) ref count gets decrement instead of IV(125) one!

Leaving block 0\, type BLOCK (-e​:0) LEAVE scope 2 at perl.c​:4835   # IV(213) gets prematurely free here

(-e​:0) popping jumplevel was bffff950\, now 817d700 (-e​:0) Setting up jumplevel bffff950\, was 817d700 (-e​:0) popping jumplevel was bffff950\, now 817d700 (-e​:0) LEAVE scope 1 at perl.c​:428

Attempt to free unreferenced scalar​: SV 0x8190428 = IV(213)

p5pRT commented 18 years ago

From chris@heathens.co.nz

I have been looking at this off and on over the past couple of months after I reported it as bug #37094. (My apologies for opening a new bug report. From now on\, I will update the original bug.)

I have attached a patch that appears to work\, and I'll try to explain my logic behind the patch\, because it's not at all obvious (to me anyway).

First\, some more test cases​:

perl -e 'map die\,4 for 3' perl -e 'grep die\,4 for 3' perl -e 'for $a (3) {@​b=sort {die} 4\,5}'

There are two underlying issues\, which contribute to this bug​:

* map\, grep\, sort\, and possibly others\, automatically localize certain variables ($_\, $a\, and/or $b) for use in the inner block\, but they don't increment the refcounts on those variables when they are in use.

* when unwinding due to an exception\, there are two stacks (context stack and save stack) that are not popped in the opposite order from when they were pushed. In particular\, this is a problem because POPLOOP (part of the context stack unwinding) does some things that assumes the save stack is where it was when PUSHLOOP was called.

Now for a perfectly good (non-buggy) example\, to show how... umm... odd the stack unwinding really is​:

For ease of later reference\, I have labeled each SV that $_ points to.

$ perl -Dlt -e 'for (3) {local $_=4;die}' [snip] EXECUTING...

(-e​:0) enter   # DEFSV starts out as\, let's say\, SV0.

(-e​:0) ENTER scope 2 at pp_hot.c​:1701 Entering block 0\, type BLOCK (-e​:0) nextstate (-e​:1) pushmark (-e​:1) const(IV(3)) (-e​:1) gv(main​::_) (-e​:1) enteriter   # This saves DEFSV using SAVEGENERICSV\,   # then does DEFSV = NEWSV(0\,0). Let's call this new SV SV1.   # PUSHLOOP also sets itersave = SV1 and increments SV1->sv_refcnt.

(-e​:1) ENTER scope 3 at pp_ctl.c​:1682 (-e​:1) ENTER scope 4 at pp_ctl.c​:1713 Entering block 1\, type LOOP (-e​:1) iter   # This sets DEFSV to IV(3)\, which I will also call SV2.   # Of course\, it also decrements SV1->sv_refcnt back to 1.

(-e​:1) and (-e​:1) nextstate (-e​:1) const(IV(4)) (-e​:1) gvsv(main​::_)   # This calls save_scalar\, which saves the old DEFSV (SV2)   # on the stack\, and sets DEFSV to a new SV\, SV3.

(-e​:1) sassign (-e​:1) nextstate (-e​:1) pushmark (-e​:1) die Died at -e line 1. Unwinding block 1\, type LOOP (-e​:0) POPBLOCK scope 2 at perl.c​:4825   # POPLOOP mortalizes the "wrong" SV\, SV3.   # Then it sets DEFSV to itersave\, which is SV1.

Leaving block 0\, type BLOCK (-e​:0) LEAVE scope 2 at perl.c​:4826   # This undoes the save_scalar by decrementing the refcount   # of DEFSV (SV1)\, and setting DEFSV to the saved value SV2.

(-e​:0) Setting up jumplevel bfd489e0\, was 830319c (-e​:0) LEAVE scope 1 at perl.c​:524   # This undoes the SAVEGENERICSV by decrementing the refcount   # of DEFSV (SV2)\, and setting DEFSV to the saved value SV0.

In summary\, there are no leaks in this example. Every variable's refcount is decremented the correct number of times. It is just done in a non-intuitive order​:   SV3​: incremented by gvsv/save_scalar\, i.e. when in scope 2.   mortalized by POPLOOP (!)   SV2​: incremented by iter\, i.e. when in scope 1.   decremented by LEAVE scope 1.   SV1​: incremented by enteriter and PUSHLOOP\,   decremented by LEAVE scope 2 (!)

Even if I didn't know about the buggy test cases\, I would still call this a classic "bug waiting to happen". :-)

Now for the buggy case. For brevity\, I'll only give the summary of what happens\, because the program flow is almost identical to the above.

$ perl -e 'grep die\,4 for 3' Died at -e line 1. Attempt to free unreferenced scalar​: SV 0x9ed9120\, Perl interpreter​: 0x9ec1008.

Summary​:

  SV3​: NOT incremented by grep\, i.e. when in scope 2.   mortalized by POPLOOP (!)   SV2​: incremented by iter\, i.e. when in scope 1.   decremented by LEAVE scope 1.   SV1​: incremented by enteriter and PUSHLOOP\,   NOT decremented by LEAVE scope 2 (!)

Because grep doesn't increment the refcount of $_ as it traverses through its list\, and because POPLOOP mortalizes grep's variable\, we get a mess.

So\, I thought\, the obvious solution is to make POPLOOP not modify the itervar to point back to itersave. But when I tried that\, it fixed the above test cases\, but it completely broke the case when itervar is a lexical variable. (for my $i (3) {die})

That's why my patch keeps the old behavior when SvPADMY(itervar) is true\, but when it is false\, POPLOOP no longer modifies itervar because it may have been localized.

p5pRT commented 18 years ago

From chris@heathens.co.nz

for.patch ```diff --- cop.h.orig 2005-09-24 11:15:09.000000000 -0400 +++ cop.h 2005-11-06 11:30:59.000000000 -0500 @@ -407,9 +407,14 @@ #define POPLOOP(cx) \ SvREFCNT_dec(cx->blk_loop.iterlval); \ if (CxITERVAR(cx)) { \ - SV **s_v_p = CxITERVAR(cx); \ - sv_2mortal(*s_v_p); \ - *s_v_p = cx->blk_loop.itersave; \ + if (SvPADMY(cx->blk_loop.itersave)) { \ + SV **s_v_p = CxITERVAR(cx); \ + sv_2mortal(*s_v_p); \ + *s_v_p = cx->blk_loop.itersave; \ + } \ + else { \ + SvREFCNT_dec(cx->blk_loop.itersave); \ + } \ } \ if (cx->blk_loop.iterary && cx->blk_loop.iterary != PL_curstack)\ SvREFCNT_dec(cx->blk_loop.iterary); ```
p5pRT commented 18 years ago

From @rgs

Chris Heath via RT wrote​:

So\, I thought\, the obvious solution is to make POPLOOP not modify the itervar to point back to itersave. But when I tried that\, it fixed the above test cases\, but it completely broke the case when itervar is a lexical variable. (for my $i (3) {die})

That's why my patch keeps the old behavior when SvPADMY(itervar) is true\, but when it is false\, POPLOOP no longer modifies itervar because it may have been localized.

Thanks\, your patch applied as change #26027 to bleadperl.

p5pRT commented 18 years ago

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