More complete handling of padlists for XSUBs.

[Code by Zefram, commit message by Nicholas]

Following discussion with Zefram on IRC, I agree that his code is more
correct than mine. Prior to the use of the CvPADLIST slot on an XSUB for
binary incompatibility detection, cv_undef() would unconditionally treat
CvPADLIST() as a pointer to a padlist structure (on both PVCVs and PVFMs),
walking it and calling the relevant free functions. Hence on those versions,
CvPADLIST could not be anything other than a legitimate padlist, else the
interpreter would crash. So we should be consistent and calculate the size
on the same basis.

CPAN #102909

diff --git a/CHANGES b/CHANGES
index 1360c79..c4f0f2f 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -7,6 +7,7 @@ Revision history for Perl extension Devel::Size.
   * Handle PADNAMELIST/PADNAME introduced in v5.21.7
     two patches from Zefram:
   * Add handling of children of METHOP and UNOP_AUX ops [CPAN #102911]
+  * More complete handling of padlists for XSUBs [CPAN #102909]
 
 0.79_52 2015-03-20 nicholas
     two patches from Zefram:

diff --git a/Size.xs b/Size.xs
index 40404e5..ada99cc 100644 (file)
--- a/Size.xs
+++ b/Size.xs
@@ -942,7 +942,8 @@ sv_size(pTHX_ struct state *const st, const SV * const orig_thing,
 
 
   case SVt_PVFM: TAG;
-    padlist_size(aTHX_ st, CvPADLIST(thing), SOME_RECURSION);
+    if (PERL_VERSION*1000+PERL_SUBVERSION < 21006 || !CvISXSUB(thing))
+       padlist_size(aTHX_ st, CvPADLIST(thing), SOME_RECURSION);
     sv_size(aTHX_ st, (SV *)CvOUTSIDE(thing), recurse);
 
     if (st->go_yell && !st->fm_whine) {
@@ -955,15 +956,14 @@ sv_size(pTHX_ struct state *const st, const SV * const orig_thing,
     sv_size(aTHX_ st, (SV *)CvSTASH(thing), SOME_RECURSION);
     sv_size(aTHX_ st, (SV *)SvSTASH(thing), SOME_RECURSION);
     sv_size(aTHX_ st, (SV *)CvGV(thing), SOME_RECURSION);
+    if (PERL_VERSION*1000+PERL_SUBVERSION < 21006 || !CvISXSUB(thing))
+       padlist_size(aTHX_ st, CvPADLIST(thing), SOME_RECURSION);
     sv_size(aTHX_ st, (SV *)CvOUTSIDE(thing), recurse);
     if (CvISXSUB(thing)) {
        sv_size(aTHX_ st, cv_const_sv((CV *)thing), recurse);
-    } else {
-        padlist_size(aTHX_ st, CvPADLIST(thing), SOME_RECURSION);
-        if (CvROOT(thing)) {
-            op_size(aTHX_ CvSTART(thing), st);
-            op_size(aTHX_ CvROOT(thing), st);
-        }
+    } else if (CvROOT(thing)) {
+       op_size(aTHX_ CvSTART(thing), st);
+       op_size(aTHX_ CvROOT(thing), st);
     }
     goto freescalar;