develooper Front page | perl.perl5.porters | Postings from September 2013

[perl #3112] Bizarre copy of ARRAY in last

From:
Father Chrysostomos via RT
Date:
September 20, 2013 08:09
Subject:
[perl #3112] Bizarre copy of ARRAY in last
Message ID:
rt-3.6.HEAD-1873-1379664551-188.3112-15-0@perl.org
On Mon Jul 19 03:07:26 2010, nicholas wrote:
> On Fri, Jul 16, 2010 at 07:34:17PM -0700, George Greer via RT wrote:
> > On Fri Jul 16 19:32:48 2010, greerga wrote:
> > > On Thu Apr 27 11:14:44 2006, animator wrote:
> 
> > > > A somewhat shorter version of this problem: perl -le 'for (1) {
> push
> > > > @a, last; }'
> > > >
> > > > (If in the initial problem parans are used around push then the
> > > error
> > > > goes away ( push (@a, $_), last ... )
> > > >
> > > > This problem still exists in bleadperl (and 5.8.8, and 5.9.3)...
> > >
> > > I've attached a stab at it.  I added a new test and it passes the
> > > current ones also, however I'm not 100% versed in the perl stack
> > > enough
> > > to know if using SvTYPE is the most accurate criteria here.
> > >
> > > http://m-l.org/~perl/misc/0001-Fix-for-RT-3112-Bizarre-copy-of-
> ARRAY-
> > > when-using-las.patch
> > >
> > >
> >
>
http://github.com/greerga/perl/commit/aeb45651ef8a89da73a16a74c7b676305e8ec7cd
> >
> > Duh. Now with 100% more attaching action.
> 
> Which, frustratingly, I can't persuade my mailer to inline for a
> reply. So
> I've had to grab it:
> 
> diff --git a/pp_ctl.c b/pp_ctl.c
> index a93d6dc..ea25810 100644
> --- a/pp_ctl.c
> +++ b/pp_ctl.c
> @@ -2292,7 +2292,8 @@ PP(pp_last)
>      }
>      else if (gimme == G_ARRAY) {
>  	while (++MARK <= SP) {
> -	    *++newsp = ((pop2 == CXt_SUB) && SvTEMP(*MARK))
> +	    *++newsp = ((pop2 == CXt_SUB && SvTEMP(*MARK))
> +                        || SvTYPE(*MARK) == SVt_PVAV)
>  			? *MARK : sv_mortalcopy(*MARK);
>  	    TAINT_NOT;		/* Each item is independent */
>  	}
> 
> 
> I'm not even sure that that is right solution.
> 
> Looking through the code I see roughly that idiom in a lot of places.
> 
> 1: Why is this one special?
>    (ie can the same problem hit the other paths?)
> 2: What is the code actually trying to do?
>    I *thought* that it's doing the following:
>    a: FREETMPS and LEAVE have to be called to leave the subroutine
>    b: This would free mortals which are on the stack
>    c: Some of these need to survive
>    d: Therefore it copies the values of mortals (ie, onto an outer
> mortals
>       stack which will last a bit longer)
> 
>   Only it's *not* doing that. It's copying everything that isn't
> marked as
>   TEMP.
>   Why does it need to do this at all?

As already discussed, it doesn’t need to do that for last (just reset
the stack).

But for other things, like eval and subroutines, copying appears to be
the intent.  The assumption is that anything marked TEMP is unused
elsewhere and can therefore be returned without copying, since that fact
that it is not copied is not observable.  (That assumption is the cause
of bug #105910.  I think I see several other problems with it, too.)

George’s patch will probably actually fix bug #119797, but it would only
be a partial fix, since S_adjust_stack_on_leave would still do the wrong
thing for scalar lvalues.

-- 

Father Chrysostomos


---
via perlbug:  queue: perl5 status: open
https://rt.perl.org:443/rt3/Ticket/Display.html?id=3112



nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About