Re: 5.9.1 release now integrated into ponie

On Thu, Apr 08, 2004 at 07:44:31PM +0100, Dave Mitchell wrote:
> On Thu, Apr 08, 2004 at 04:22:09PM +0100, Nicholas Clark wrote:
> > We're in parts of perl I don't know. The save stack is getting restored:
> > 
> > 	case SAVEt_COMPPAD:
> > 	    PL_comppad = (PAD*)SSPOPPTR;
> > 	    if (PL_comppad)
> > 		PL_curpad = AvARRAY(PL_comppad);
> > 	    else
> > 		PL_curpad = Null(SV**);
> > 	    break;
> > 
> > and the crash is because PL_comppad has just been reloaded with this:
> > 
> > (gdb) print *PL_comppad
> > $13 = {
> >   sv_any = 0x101e7c0, 
> >   sv_refcnt = 0, 
> >   sv_flags = 255, 
> >   sv_is_pmc = 1 '\001'
> > }
> > 
> > so that AvARRAY() kicks in. Should that if(PL_comppad) be checking the
> > SvFLAGS?  And silently treating type 0xFF the same way as PL_comppad of
> > NULL? Or is this a (perl) bug and it should be pitching a fit?
> 
> I don't know anything about the Ponie side of things, but from a purely
> Perl perspective, PL_comppad should at all times ether point to a valid AV
> or be null.  This looks like it's pointing to a freed something (type ==
> SVTYPEMASK, refcnt == 0).

From a purely perl perspective, with this added to blead:

@@ -1050,8 +1050,10 @@
            break;
        case SAVEt_COMPPAD:
            PL_comppad = (PAD*)SSPOPPTR;
-           if (PL_comppad)
+           if (PL_comppad) {
+               assert(SvTYPE(PL_comppad) == SVt_PVAV);
                PL_curpad = AvARRAY(PL_comppad);
+           }
            else
                PL_curpad = Null(SV**);
            break;



I get Failed 8 test scripts out of 842, 99.05% okay.

with a lot of diagnostics such as

t/op/sort............................Assertion (((my_perl->Icomppad))->sv_flags & 0xff) == SVt_PVAV failed: file "scope.c", line 1054 during global destruction.
ok


So it does end up pointing to a freed thing about 8 times during the regression
tests. I'm not sure how this happens - I guess that the save stack has a
duplicate reference to a scalar, such that the "free everything" routines in
global destruction manage to free things that are on the save stack.

> My suspcicians would point to the handling of the parent padlist AV, which
> is an AV of AVs (rather than the usual AV of RVs to AVs) The padlist AV
> does not have the AvREAL flag set, so the individual elements within the
> padlist aren't refcounted, and the freeing is explicitly handled by the
> pad code itself.  If Ponie isn't properly handling AVs where AvREAL is
> turned off, that might explain what you're seeing.

I don't think that ponie has changed anything (at least intentionally) in this
area. Given that I can add an assertion to vanilla perl 5 that codifies
your previous paragraph (either AV or null) and existing tests fail, I
suspect that ponie is tickling an existing "problem". I wonder if it only
shows up because of the deliberately aggressive policy of the MUMBLE macro -
ie by inverting the bits on PMC pointers make all illegal reads SEGV.
Without this all the assertion failures and the failure I describe would be
hidden - (I'm inferring that) garbage data is read but never acted upon.

Nicholas Clark