develooper Front page | perl.perl5.porters | Postings from August 2012

ALIAS breaks PPCODE (Re: [perl.git] branch blead,updated. v5.17.2-321-gbe8851f)

Thread Next
From:
Craig A. Berry
Date:
August 16, 2012 16:04
Subject:
ALIAS breaks PPCODE (Re: [perl.git] branch blead,updated. v5.17.2-321-gbe8851f)
Message ID:
craigberry-28866A.18040016082012@cpc2-bmly6-0-0-cust974.2-3.cable.virginmedia.com
In article <E1T1CWq-0006Wa-FZ@camel.ams6.corp.booking.com>,
 nick@ccl4.org ("Nicholas Clark") wrote:

> commit c0810f8ef849bf940e296c00ef5a0c1bd77f9c62
> Author: Nicholas Clark <nick@ccl4.org>
> Date:   Mon Aug 13 13:38:03 2012 +0200
> 
>     Use ALIAS to provide XS::APItest::newCONSTSUB and newCONSTSUB_flags
>     
>     Previously both C routines were wrapped with newCONSTSUB_type, which used 
>     a
>     "type" parameter to determine which C code to call. Use an ALIAS to bind 
>     the
>     code to two names, and eliminate the "type" parameter.
>     
>     This makes the test code clearer. It's not perfect, as the XS wrapper
>     XS::APItest::newCONSTSUB has a flags parameter whereas the underlying C 
>     code
>     does not, but fixing this would require considerably more XS hackery.

The problem with this is that ALIAS causes the override of the name of 
the function in the boilerplate error checking code created by PPCODE. 
xsubpp correctly thought, before this change, that the name of the 
function was "XS::APItest::newCONSTSUB_type".  Now it thinks the name 
of the function is GvNAME(CvGV(cv)), but cv has not been initialized 
yet.

Which on compilers that care about such things breaks the build with:


            GvNAME(CvGV(cv)),
................................^
%CC-W-UNINIT1, The scalar variable "cv" is fetched but not initialized.  And there may be other such 
fetches of this variable that have not been reported in this compilation.

And on compilers that don't care about such things, you'll get a 
segfault at run time if you ever fail the check for whether the first 
argument to the function is a hash reference.

The boilerplate code that looks different when ALIAS is present comes 
from lib/ExtUtils/typemap and looks like:


T_HVREF
        STMT_START {
                SV* const xsub_tmp_sv = $arg;
                SvGETMAGIC(xsub_tmp_sv);
                if (SvROK(xsub_tmp_sv) && SvTYPE(SvRV(xsub_tmp_sv)) == SVt_PVHV){
                    $var = (HV*)SvRV(xsub_tmp_sv);
                }
                else{
                    Perl_croak(aTHX_ \"%s: %s is not a HASH reference\",
                                ${$ALIAS?\q[GvNAME(CvGV(cv))]:\qq[\"$pname\"]},
                                \"$var\");
                }
        } STMT_END


For the case in question (ext/XS-APItest/APItest.xs), the code expands to:


XS_EUPXS(XS_XS__APItest_newCONSTSUB); /* prototype to pass -Wmissing-prototypes */
XS_EUPXS(XS_XS__APItest_newCONSTSUB)
{
    dVAR; dXSARGS;
    dXSI32;
    if (items != 4)
       croak_xs_usage(cv,  "stash, name, flags, sv");
    PERL_UNUSED_VAR(ax); /* -Wall */
    SP -= items;
    {
        HV*     stash;
        SV*     name = ST(1)
;
        I32     flags = (I32)SvIV(ST(2))
;
        SV*     sv = ST(3)
;
#line 1968 "APItest.xs"
        CV* cv;
        STRLEN len;
        const char *pv = SvPV(name, len);
#line 3104 "APItest.c"

        STMT_START {
                SV* const xsub_tmp_sv = ST(0);
                SvGETMAGIC(xsub_tmp_sv);
                if (SvROK(xsub_tmp_sv) && SvTYPE(SvRV(xsub_tmp_sv)) == SVt_PVHV){
                    stash = (HV*)SvRV(xsub_tmp_sv);
                }
                else{
                    Perl_croak(aTHX_ "%s: %s is not a HASH reference",
                                GvNAME(CvGV(cv)),
                                "stash");
                }
        } STMT_END
;
#line 1972 "APItest.xs"
        switch (ix) {
           case 0:
          cv = newCONSTSUB(stash, pv, SvOK(sv) ? SvREFCNT_inc(sv) : NULL);
               break;
           case 1:
               cv = newCONSTSUB_flags(
                 stash, pv, len, flags | SvUTF8(name), SvOK(sv) ? SvREFCNT_inc(sv) : NULL
               );
               break;
        }

Why anything happens in the order it does in XS is too much of a 
mystery to me to venture a guess as to whether the initialization of cv 
could be moved up into the PREINIT section.  But in any case my compiler is
telling the truth and we are trying to use cv before it's initialized.

Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About