On May 19, 2009, at 8:31 AM, Nicholas Clark wrote:
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.
Maybe you were thinking of this:
http://www.nntp.perl.org/group/perl.perl5.changes/2008/09/msg22279.html
?
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 $?.
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.
________________________________________
Craig A. Berry
mailto:craigbe...@mac.com
"... getting out of a sonnet is much more
difficult than getting in."
Brad Leithauser
