Re: Data overrun in Perl_magic_get '?' (Was: Access violation in SV.C new_body_inline())

On Tue, May 19, 2009 at 08:18:17AM -0500, John E. Malmberg wrote:
> Craig A. Berry wrote:

> >So you're saying that these lines in Perl_magic_get in mg.c:
> >
> >    case '?':
> >        {
> >            sv_setiv(sv, (IV)STATUS_CURRENT);
> >#ifdef COMPLEX_STATUS
> >            LvTARGOFF(sv) = PL_statusvalue;
> >            LvTARGLEN(sv) = PL_statusvalue_vms;
> >#endif
> >        }
> >
> >
> >are where the damage occurs?  So it looks like the SV in question does 
> >not even have the relevant slots (xlv_targlen) we're trying to update 
> >here.
> 
> Yes.
> 
> > I wonder if it's because IPC::Cmd declares $? as local.  Maybe we 
> >are assuming $? is always lexical but it's not?
> 
> I do not know.

That code rings a bell. The only thing I can find that I did near it was:

http://perl5.git.perl.org/perl.git/commit/35f998ddd1e1665f7d0899ae3e50f9262c59d848

However I had a suspicion that I also did something that restricted the upgrade
to the minimal case.

I don't think that "lexical" has anything to do with it - I suspect that the
bug is because IPC::Cmd localises $?, and the new scalar is created like this:

STATIC SV *
S_save_scalar_at(pTHX_ SV **sptr, const U32 flags)
{
    dVAR;
    SV * const osv = *sptr;
    register SV * const sv = *sptr = newSV(0);

    PERL_ARGS_ASSERT_SAVE_SCALAR_AT;

    if (SvTYPE(osv) >= SVt_PVMG && SvMAGIC(osv) && SvTYPE(osv) != SVt_PVGV) {
	if (SvGMAGICAL(osv)) {
	    const bool oldtainted = PL_tainted;
	    SvFLAGS(osv) |= (SvFLAGS(osv) &
	       (SVp_IOK|SVp_NOK|SVp_POK)) >> PRIVSHIFT;
	    PL_tainted = oldtainted;
	}
	mg_localize(osv, sv, (flags & SAVEf_SETMAGIC) != 0);
    }
    return sv;
}


where that call to mg_localize() will upgrade it to PVMG, but not PVLV.

If this is the cause, I'm not sure whether the correct fix is to make
mg_localize generally upgrade the new scalar to the type of the existing
scalar, or special case it for $?.

Nicholas Clark