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

Attempt to free unreferenced scalar #6854

Closed
p5pRT opened this issue Oct 20, 2003 · 9 comments
Closed

Attempt to free unreferenced scalar #6854

p5pRT opened this issue Oct 20, 2003 · 9 comments

Comments

@p5pRT
Copy link

p5pRT commented Oct 20, 2003

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

Searchable as RT24254$

@p5pRT
Copy link
Author

p5pRT commented Oct 20, 2003

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

p5pRT commented Oct 21, 2003

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

p5pRT commented Oct 14, 2004

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

p5pRT commented Apr 27, 2005

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

p5pRT commented Jun 4, 2005

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

p5pRT commented Nov 7, 2005

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

p5pRT commented Nov 7, 2005

From chris@heathens.co.nz

for.patch
--- 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
Copy link
Author

p5pRT commented Nov 7, 2005

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

p5pRT commented Nov 7, 2005

@rgs - 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