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

On Wed, May 20, 2009 at 07:23:32PM -0500, Craig A. Berry wrote:
> 
> On May 19, 2009, at 8:31 AM, Nicholas Clark wrote:

> >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.
> 
> Maybe you were thinking of this:
> 
> http://www.nntp.perl.org/group/perl.perl5.changes/2008/09/msg22279.html

Yes, it was exactly that one. Thanks.

> >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 $?.
> 
> 
> Thanks for the analysis.  I don't really know the implications of the  
> alternatives you propose.  I assume a general change is more risky and  
> potentially adds unnecessary processing to hot code.  To me the least  
> risky thing would be to add the following by analogy with what's  
> already going on in gv.c:
> 
> --- mg.c;-0     2009-04-27 02:42:10 -0500
> +++ mg.c        2009-05-20 19:11:11 -0500
> @@ -974,6 +974,7 @@ Perl_magic_get(pTHX_ SV *sv, MAGIC *mg)
>         {
>             sv_setiv(sv, (IV)STATUS_CURRENT);
>  #ifdef COMPLEX_STATUS
> +           SvUPGRADE(sv, SVt_PVLV);
>             LvTARGOFF(sv) = PL_statusvalue;
>             LvTARGLEN(sv) = PL_statusvalue_vms;
>  #endif
> [end]
> 
> Does that make sense?
> 
> This does fix the memory error that started this thread.  I'm now  
> starting a complete test run to make sure nothing else goes pear-shaped.

Presumably an upgrade check is also needed at any point that tries to read
the complex status, as right now it could be using LvTARGOFF() or LvTARGLEN()
on a localised $? that is only PVMG, not PVLV.

Nicholas Clark