Skip to content

Commit 6da1306

Browse files
author
Father Chrysostomos
committed
[perl #129090] Crash with sub c{sub c}
The crash in the bug report was caused by v5.21.7-215-g307cbb9, which added the code in question in order to fix another crash. It exposed an existing problem caused by v5.21.6-386-ga70f21d. Some background: When parsing ‘sub c{’, perl creates a new CV. The CV gets installed in the symbol table when the final ‘}’ is reached. If there is already an undefined CV at that point, it gets reused and the contents of the new CV are transferred to the existing one. That means that any subs defined within now have their CvOUTSIDE pointer pointing to a stub that is about to be freed, so pad_fix_inner_anons gets called to update those CvOUTSIDE pointers accordingly. Formats were not getting their CvOUTSIDE pointers updated, so commit v5.11.2-160-g421f30e added formats to the containing sub’s pad to allow the CvOUTSIDE fix-up to apply to them, too. That caused a crash, as explained in the commit message for v5.17.1-213-ge09ac07, that the cited commit fixed by putting a weak RV in the pad, not a direct pointer to the format, and giving the format a strong CvOUTSIDE pointer. Problem: Commit v5.21.6-386-ga70f21d¹ fixed a problem with named subs not cor- rectly referencing outer state vars (because of bad CvOUTSIDE point- ers hiding the lexical vars). It did so by extending the mechanism already used for formats to apply to named subs as well. The one mistake that that commit made was to add a pad entry for the sub even if it was just a stub declaration. That caused a sub with a stub declaration inside it to a loop if they have the same name: $ ./perl -Ilib -le '%c; sub c { sub c; eval q|$x| } c' This happens because the ‘sub c;’ inserts a stub into the symbol table and inserts a reference to that stub also into the pad of the sub cur- rently being compiled. The final CvOUTSIDE fix-up sets the CvOUTSIDE of the erstwhile stub (that has just become the newly defined sub) to the sub itself. (The %c in the example is necessary to vivify the typeglob; otherwise perl will cheat and the stub declaration won’t actually create a CV.) In addition, a crash can occur in this one-liner (reduced from the original bug report): $ ./miniperl -e '$a="a$a";my sub b;%c;sub c{sub b;sub c}' The fix-up code iterates through the pad backwards, starting from the last entry, so the CvOUTSIDE(c) -> c loop is set up before the lexical &b entry is encountered. When it is encountered, CvOUTSIDE pointers are followed to find the outer pad where b is defined, but the PARENT_PAD_INDEX stored in the &b padname is wrong for c’s own pad, which is what we end up looking at due to the CvOUTSIDE loop. The ‘outer’ entry may then turn out be an RV, not a CV, or just about any- thing. Crashes ensue. Another, perhaps clearer, way to make it crash is to give the lexical sub pad entry an outer offset far beyond the end of the directly- enclosing sub: $ ./perl -Ilib -lE '%c; my ($a,$b,$c,$d,$e,$f,$g,$h,$i,$j,$k,$l,$m,$n,$o,$p,$q,$r,$s,$t,$u); state sub b; sub c { sub b {} sub c }' Segmentation fault: 11 Solution: The fix is to stop package stubs from creating pad entries. ¹ Which I found by bisecting like this, in case anyone is interested: ../perl.git/Porting/bisect.pl --expect-fail --start=v5.20.0 --end=v5.22.0 -e '%c; sub c{sub c}; use B; print B::svref_2object(\&c)->PADLIST->ARRAYelt(0)->ARRAYelt(1)->PV'
1 parent bbd6d87 commit 6da1306

File tree

2 files changed

+15
-1
lines changed

2 files changed

+15
-1
lines changed

op.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8875,7 +8875,7 @@ Perl_newATTRSUB_x(pTHX_ I32 floor, OP *o, OP *proto, OP *attrs,
88758875
if (slab)
88768876
Slab_to_ro(slab);
88778877
#endif
8878-
if (cv && name && CvOUTSIDE(cv) && !CvEVAL(CvOUTSIDE(cv)))
8878+
if (cv && name && block && CvOUTSIDE(cv) && !CvEVAL(CvOUTSIDE(cv)))
88798879
pad_add_weakref(cv);
88808880
}
88818881
return cv;

t/op/sub.t

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,3 +415,17 @@ sub curpm {
415415
}
416416
"a" =~ /(.)/;
417417
is(curpm(), 'c', 'return and PL_curpm');
418+
419+
# [perl #129090] Crashes and hangs
420+
watchdog 10;
421+
{ no warnings;
422+
eval '$a=qq|a$a|;my sub b;%c;sub c{sub b;sub c}';
423+
}
424+
eval '
425+
()= %d;
426+
{my ($a,$b,$c,$d,$e,$f,$g,$h,$i,$j,$k,$l,$m,$n,$o,$p,$q,$r,$s,$t,$u);}
427+
{my ($a,$b,$c,$d,$e,$f,$g,$h,$i,$j,$k,$l,$m,$n,$o,$p,$q,$r,$s,$t,$u);}
428+
{my ($a,$b,$c,$d,$e,$f,$g,$h,$i,$j,$k,$l,$m,$n,$o,$p,$q,$r,$s,$t,$u);}
429+
CORE::state sub b; sub d { sub b {} sub d }
430+
';
431+
eval '()=%e; sub e { sub e; eval q|$x| } e;';

0 commit comments

Comments
 (0)