Pass clone parameter to CLONE methods as a UV

[Leont]

Any CLONE method that needs to call sv_clone() and friends needs to pass this on as an argument.

On older releases such an argument would not be passed, so they would need to take it as an optional argument to work in both cases. E.g.

void CLONE(SV* classname, UV uv_params = 0)
CODE:
    CLONE_PARAMS* params = INT2PTR(params);

That means this change can break XS modules not taking the extra argument into account. Fortunately most modules have cargo-culted a CLONE(...) from perlxs so they're safe from this change. A quick grep suggests only four CPAN dists are affected such, Data::UUID, Net::LDNS, GObject, and Zonemaster::LDNS

• This set of changes requires a perldelta entry, and I'll write it for 5.43 because adding it now will only cause merge conflicts.

[bulk88]

void CLONE(SV* classname, UV uv_params = 0)
CODE:
    CLONE_PARAMS* params = INT2PTR(params);

Can't C level arg CLONE_PARAMS* params be smuggled somewhere inside the child my_perl struct instead? Less boiler plate, and CC type checking comes automatically vs a cargo cult copy paste. PP CLONE subs dont need and cant use that ptr. Its only for XSUBs.

2nd problem, I've found in rare cases CLONE method runs "too late" during a clone, and Module A, calls into Module B, but Module B didnt get its CLONE sub called yet, so its MY_CXT struct isn't duplicated/ctor-ed yet, and Mod B will now see uninited memory (PL_my_cxt_list is null) or see a wrong my_perl ptr "global view" with SV*s from the parent ithread.

HV* PL_modglobal or PP visible package $scalars @arrays get their MGf_DUP MG vtable callback fired earlier than CLONE methods firing AFAIK.

[Leont]

Can't C level arg CLONE_PARAMS* params be smuggled somewhere inside the child my_perl struct instead? Less boiler plate, and CC type checking comes automatically vs a cargo cult copy paste. PP CLONE subs dont need and cant use that ptr. Its only for XSUBs.

In this particular case, I guess it could. It kind of feels wrong because in other cases (e.g. svt_dup) it kind of can't.

2nd problem, I've found in rare cases CLONE method runs "too late" during a clone, and Module A, calls into Module B, but Module B didnt get its CLONE sub called yet, so its MY_CXT struct isn't duplicated/ctor-ed yet, and Mod B will now see uninited memory (PL_my_cxt_list is null) or see a wrong my_perl ptr "global view" with SV*s from the parent ithread.

Yeah they're unordered, but that doesn't have anything to do with this PR.

HV* PL_modglobal or PP visible package $scalars @arrays get their MGf_DUP MG vtable callback fired earlier than CLONE methods firing AFAIK.

Correct.

[mauke]

perlmod says:

Currently CLONE is called with no parameters other than the invocant package name, but code should not assume that this will remain unchanged, as it is likely that in future extra parameters will be passed in to give more information about the state of cloning.

"Currently CLONE is called with no parameters" should be amended, but this also means modules that don't accept an additional parameter are Doing It Wrong™. (This wording has been in perlmod since 2004; before that, CLONE's parameters were not documented at all.)

void CLONE(SV* classname, UV uv_params = 0)
CODE:
    CLONE_PARAMS* params = INT2PTR(params);

Does this mean I can now crash XS modules in pure perl by simply calling Some::XS::Module->CLONE(123)?

[Leont]

"Currently CLONE is called with no parameters" should be amended, but this also means modules that don't accept an additional parameter are Doing It Wrong™. (This wording has been in perlmod since 2004; before that, CLONE's parameters were not documented at all.)

Good to know, I will adapt that.

Does this mean I can now crash XS modules in pure perl by simply calling Some::XS::Module->CLONE(123)?

Yeah, but the same is true of a lot of others things really (e.g. anything taking a PTROBJ)

[bulk88]

If my_perl or CV* data smuggling is not picked as the final solution. I suggest adding CLONE_PARAMS * type to the root p5p typemap file so the user has less src code opportunities to make a mistake vs a long copy paste of casting the UV to a CLONE_PARAMS * type and EUPXS hides more internals from the user.

[Leont]

If my_perl or CV* data smuggling is not picked as the final solution.

I'm not very invested in any particular solution, I'm mainly invested in solving the problem of "sometimes CLONE needs a CLONE_PARAMS* but doesn't have it" (e.g. when there's an SV* in the MY_CXT). By all means write a competing PR so we can actually compare them if that's what you want.

[bulk88]

If my_perl or CV* data smuggling is not picked as the final solution.

I'm not very invested in any particular solution, I'm mainly invested in solving the problem of "sometimes CLONE needs a CLONE_PARAMS* but doesn't have it" (e.g. when there's an SV* in the MY_CXT). By all means write a competing PR so we can actually compare them if that's what you want.

I agree its a defect.

[jkeenan]

@Leont, can you provide an update on the current status of this p.r.? Thanks.