[perl #107008] UTF8 patches for 5.16

Front page | perl.perl5.porters | Postings from March 2012

[perl #107008] UTF8 patches for 5.16

Thread Previous | Thread Next

From:

Father Chrysostomos via RT

Date:

March 25, 2012 14:08

Subject:

[perl #107008] UTF8 patches for 5.16

Message ID:

rt-3.6.HEAD-4610-1332709672-1829.107008-15-0@perl.org

On Wed Mar 21 18:49:00 2012, Hugmeir wrote:
> On Wed, Mar 21, 2012 at 10:04 PM, Father Chrysostomos via RT <
> perlbug-followup@perl.org> wrote:
> 
> > On Sun Dec 25 14:33:33 2011, sprout wrote:
> > I still don’t understand why parse_label is causing problems.
> Zefram,
> > can you look into that?
> >
> 
> For a quick recap, this is how Father C spotted this:
> 
> $ PERL_DESTRUCT_LEVEL=2 ./perl -Ilib ext/XS-APItest/t/swaplabel.t
> 1..56
> ok 1
> ... truncated ...
> ok 56
> panic: free from wrong pool during global destruction.
> 
> The problem is in these lines of parse_label:
> 
> lsv = newSV_type(SVt_PV);
> SvPV_set(lsv, lpv);
> SvCUR_set(lsv, llen);
> 
> And it goes away if you do this:
> 
> lsv = newSV_type(SVt_PV);
> SvPV_set(lsv, pl_yylval.opval->op_type == OP_CONST ? savepv(lpv) :
> lpv);
> SvCUR_set(lsv, llen);
> 
> But I don't have a clue why! So that's definitely not ready for
> inclusion.
> I dimly recall that the normal label parsing (with normal, I mean the
> one
> that doesn't use parse_label) takes ownership of the SV's PV slot, but
> I'm
> nowhere near knowledgeable enoughto figure out if that applies to
> parse_label too, or why it only shows up with PERL_DESTRUCT_LEVEL=2.

OK, I’ve figured it out.

In your (Brian Fraser’s) label patch, you make this change in the way
the label is stored by the tokeniser:

-	    pl_yylval.pval = CopLABEL_alloc(PL_tokenbuf);
+	    pl_yylval.opval = (OP*)newSVOP(OP_CONST, 0,
+                                            newSVpvn_flags(PL_tokenbuf,
+                                                        len, UTF ?
SVf_UTF8 : 0));

So it is no longer just an allocated PV with pl_yylval pointing to it,
but an svop, which, of course, owns the SV.

So, in perly.y, you make this change:

 labfullstmt:	LABEL barestmt
 			{
-			  $$ = newSTATEOP(0, PVAL($1), $2);
+			  $$ = newSTATEOP_flags(0, savepv(SvPVX(((SVOP*)$1)->op_sv)),
+                                               
SvUTF8(((SVOP*)$1)->op_sv), $2);
 			  TOKEN_GETMAD($1,
 			      $2 ? cLISTOPx($$)->op_first : $$, 'L');
 			}
 	|	LABEL labfullstmt
 			{
-			  $$ = newSTATEOP(0, PVAL($1), $2);
+			  $$ = newSTATEOP_flags(0, savepv(SvPVX(((SVOP*)$1)->op_sv)),
+                                               
SvUTF8(((SVOP*)$1)->op_sv), $2);
 			  TOKEN_GETMAD($1, cLISTOPx($$)->op_first, 'L');
 			}

where the standard parser *copies* (savepv) the strings, so that the
svop doesn’t free the string used by the new stateop.

In parse_label, you changed it to get the string out of the svop, but
without adding the copying step:

-	    char *lpv = pl_yylval.pval;
-	    STRLEN llen = strlen(lpv);
+	    STRLEN llen;
+	    char *lpv = SvPV(cSVOPx(pl_yylval.opval)->op_sv, llen);

So it is parse_label that needs to change, but not because of anything
Zefram did.

-- 

Father Chrysostomos


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

Thread Previous | Thread Next