Skip to content

Commit 8240f0a

Browse files
committed
op.c: slim down const_av_xsub const_sv_xsub XSUB.h: add GetXSTARG()
-don't use XSRETURN() macros, they use "unit of 1" relative offset "ax" and constantly reread global my_parl->Istack_base and constant rescale ax from unit of 1 to unit of 4/8 -don't use ST() macros for the same reason -in const_sv_xsub() use XSTARG if its available, but just put our long life SV* right on the PL stack if we can't write directly into the caller lval SV*. Don't do "sv_setsv(sv_newmortal(), ssv);" on missing XSTARG branch. Don't use dXSTARG; 50% chance there will be a 2nd!!! secret "sv_setsv(sv_newmortal(),rval)" or "newSVsv(rval)" right after this XSUB anyways, so just pass our SV* on stack instead of TWO "sv_setsv(sv_newmortal(), ssv);" statements executing in a row. const_av_xsub(): -if we will croak, do it ASAP once the minimum amount of data has been read from global memory, AV* is delivered through C function argument CV* cv, its not from the PL stack. So do the check and execute the no return before creating/reading/writing a ton of PL stack related global vars and C auto vars. -GetXSTARG() and GIMME_V() both dig inside PL_op, keep them together w/o any func calls in between such as EXTEND() so the CC can read OP* stored in PL_op only once -break apart Copy()'s overflow bounds checks so we can write a new "length" to global state before copying the large in bytes array, historically Perl has issues with letting PP end users code to keep running for severe overflows/heap corruption/CVE type stuff, eval{}, %SIG, tied, MG, sub END, etc. So do the asserts early before the actual memcpy or PUTBACK. -handle an empty/0 elems long AV* better, don't EXTEND(0), don't call extern libc memcpy with 0 size -don't keep re-reading the AV head and AV body and AvFILLp(av) over and over, func calls EXTEND() and memcpy() won't realloc the AV body ptr or modify the AvFILL member in the body struct XSUB.h: -add a version of dXSTARG where the user handles what to do, if parent frame didn't supply a SV* TARG for lval filling. This gets rid of the inefficient sv_newmortal() that is forced on users when there almost always better faster recipie of how to create a 0, 1, or G_LIST PL stack retval. &PL_sv_undef, newSVpvn_flags(SVs_TEMP), sv_2mortal(newSViv()), return your hash key's value SV* directly, etc. Macro undocumented until further notice, so it can gather some unofficial usage/CORE usage and some opinions regarding is it good or flawed.
1 parent 4f3b6bc commit 8240f0a

File tree

2 files changed

+129
-19
lines changed

2 files changed

+129
-19
lines changed

XSUB.h

+65
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,71 @@ is a lexical C<$_> in scope.
181181
#define dXSTARG SV * const targ = ((PL_op->op_private & OPpENTERSUB_HASTARG) \
182182
? PAD_SV(PL_op->op_targ) : sv_newmortal())
183183

184+
/* A faster, more efficient variant of C<dXSTARG>. Similar to the optree's
185+
GETTARGET, but "specialized" for XSUBs written by core or written by CPAN.
186+
Do not evaluate this macro multiple times. Save the result of this macro to
187+
a C<SV*> var. You must test the returned value for C<NULL> and procede
188+
accordingly. Do not make any assumptions on why did or did not get NULL
189+
from this macro. This macro is not a substitute for using L<GIMME_V>.
190+
The non-NULL or NULL result of this macro has no correlation to what
191+
C<@_> context, the caller PP/XS sub, has requested from your XSUB through
192+
C<GIMME_V>.
193+
194+
Assume C<GIMME_V> can return <G_VOID> while at the same time C<GetXSTARG>
195+
returns non-NULL. Also assume C<if (!(sv = GetXSTARG()) && GIMME_V == G_SCALAR)>
196+
can happen and therefore you very likely will need to allocate a new C<SV*>
197+
and mortalize it. It is discouraged and probably a bug, for an XSUB to
198+
bump the C<SvREFCNT(sv)> on C<TARG> and save the C<SV*> for later use.
199+
Do not make assumptions about C<TARG>'s C<SvREFCNT>, or what is the outer
200+
container that really the C<SV*>. Something else inside the interpreter
201+
which is unspecified, owns C<SV* TARG>, and unspecified caller, probably
202+
wants you to write into this C<SV*> as an lval, vs you doing a less
203+
efficient C<sv_newmortal()>/C<sv_2mortal(newSVxxxv())>, and later on the
204+
unspecified caller has to call <sv_setsv()>, and let the mortal stack dispose
205+
of your short lived <SV*>.
206+
207+
Although this is undocumented and private to the interpreter and you may not
208+
write code that is aware of this private implementation detail. Keep in
209+
mind the interpreter uses the C<SV* TARG> concept for both input and/or output
210+
in various places between parts of itself and might be using C<SV* TARG> as
211+
lvalue scratch pad in a loop.
212+
213+
Remember that the C<SV*> from C<dXSTARG> or C<GetXSTARG>, might be C<SvOK()>
214+
and have stale prior contents that you need to wipe or free. C<sv_setxxx()>
215+
functions will always do this for you. There is no guarentee the <SV*>
216+
from C<dXSTARG> or C<GetXSTARG> will be set to C<undef> when you get it.
217+
If you need to return C<undef>, you have 2 choices. Don't fetch and
218+
don't use C<TARG>, and push C<&PL_sv_undef> on the stack. The other choices
219+
you have is to call, sorted most efficient to least efficient:
220+
221+
sv_setsv_undef(my_targ); SvSETMAGIC(my_targ);
222+
sv_setpv_mg(my_targ, NULL);
223+
sv_setsv_mg(my_targ, NULL);
224+
sv_setsv_mg(my_targ, &PL_sv_undef); //more readable
225+
226+
Also consider, there is no clear guidance for this. Do you think you the
227+
PP or XS caller, that called your XSUB, if it is interested in getting a
228+
C<@_> return value in the first place. Is the caller going to write it as
229+
a true/false check, like C<if(do_xsub()) {0;}>, or will it write
230+
C<my $result = do_xsub();> and capture your return value for future use.
231+
The first probably don't set up a TARG for your to use. The 2nd probably
232+
will, but there are no guarentees it will set one up ahead of time.
233+
234+
Returning address C<&PL_sv_undef> is much faster than C<sv_newmortal()> or
235+
C<sv_set_undef()>. C<sv_set_undef()> is faster than the caller later on
236+
doing a C<sv_setsv()>. C<sv_setsv()> has a quick bailout shortcut in it
237+
if src and dest C<SV*>s are the same addr.
238+
239+
There is also no guarentee about what its C<SvTYPE()> is.
240+
Always assume it is of type <SVt_NULL>, and it has no SV body until you
241+
you test its type and possibly call C<SvUPGRADE> or <sv_setiv>/C<sv_setpvn>
242+
on it. There is no guarentee C<SvANY()> is non-NULL or C<SvANY()> contains
243+
a valid address or points to initialized memory. There is no guarentee
244+
C<SvTYPE()> is at minimum C<SVt_IV> and reading C<SvIVX()> won't SEGV. */
245+
246+
#define GetXSTARG() ((PL_op->op_private & OPpENTERSUB_HASTARG) \
247+
? PAD_SV(PL_op->op_targ) : NULL)
248+
184249
/* Should be used before final PUSHi etc. if not in PPCODE section. */
185250
#define XSprePUSH (sp = PL_stack_base + ax - 1)
186251

op.c

+64-19
Original file line numberDiff line numberDiff line change
@@ -16266,39 +16266,84 @@ Perl_wrap_op_checker(pTHX_ Optype opcode,
1626616266
static void
1626716267
const_sv_xsub(pTHX_ CV* cv)
1626816268
{
16269+
SV * sv = MUTABLE_SV(XSANY.any_ptr);
1626916270
dXSARGS;
16270-
SV *const sv = MUTABLE_SV(XSANY.any_ptr);
16271-
PERL_UNUSED_ARG(items);
16272-
if (!sv) {
16273-
XSRETURN(0);
16271+
SP -= items; /* wipe incoming, this is a ... vararg on PP level */
16272+
/* Don't optimize/chk for G_VOID, very unlikely or caller bug
16273+
to reach this XSUB and discard its @_ retval. */
16274+
if (sv) {
16275+
EXTEND(SP, 1);
16276+
SV *const targ = GetXSTARG();
16277+
/* If we have it, write into it, to prevent and shortcut
16278+
the inevitable sv_setsv() the caller will do */
16279+
if (targ) {
16280+
SV *const ssv = sv;
16281+
sv = targ;
16282+
sv_setsv_mg(targ, ssv);
16283+
}
16284+
PUSHs(sv);
1627416285
}
16275-
EXTEND(sp, 1);
16276-
ST(0) = sv;
16277-
XSRETURN(1);
16286+
PUTBACK; /* ret 0 or 1 SV*s */
1627816287
}
1627916288

1628016289
static void
1628116290
const_av_xsub(pTHX_ CV* cv)
1628216291
{
16283-
dXSARGS;
16292+
U8 gm;
16293+
SV * retsv;
16294+
SSize_t av_cur;
1628416295
AV * const av = MUTABLE_AV(XSANY.any_ptr);
16285-
SP -= items;
16296+
16297+
if (av && SvRMAGICAL(av))
16298+
Perl_croak_nocontext("Magical list constants are not supported");
1628616299
assert(av);
16300+
16301+
dXSARGS;
16302+
SP = MARK; /* wipe all */
1628716303
#ifndef DEBUGGING
1628816304
if (!av) {
16289-
XSRETURN(0);
16305+
PUTBACK;
16306+
return;
1629016307
}
1629116308
#endif
16292-
if (SvRMAGICAL(av))
16293-
croak("Magical list constants are not supported");
16294-
if (GIMME_V != G_LIST) {
16295-
EXTEND(SP, 1);
16296-
ST(0) = sv_2mortal(newSViv((IV)AvFILLp(av)+1));
16297-
XSRETURN(1);
16309+
av_cur = AvFILLp(av)+1;
16310+
/* protect PUTBACK before Copy(), in case perl's Copy()/memcpy()
16311+
returns execution control to PP code with longjmp(). */
16312+
MEM_WRAP_CHECK(av_cur, SV *);
16313+
16314+
gm = GIMME_V;
16315+
if (gm != G_LIST) { /* Close so GIMME_V GetXSTARG will share PL_op derefs. */
16316+
retsv = GetXSTARG();
16317+
if (retsv)
16318+
sv_setiv_mg(retsv, (IV)av_cur);
16319+
else
16320+
retsv = sv_2mortal(newSViv((IV)av_cur));
16321+
}
16322+
else if(av_cur == 0) { /* empty array */
16323+
PUTBACK;
16324+
return;
16325+
}
16326+
else
16327+
retsv = NULL;
16328+
EXTEND(SP, retsv ? 1 : av_cur);
16329+
SP++; /* move to ST(0), returning atleast 1 elem */
16330+
if (retsv) {
16331+
SETs(retsv);
16332+
PUTBACK;
16333+
}
16334+
else {
16335+
SV ** avarr = AvARRAY(av);
16336+
SV ** sp_start = SP;
16337+
perl_assert_ptr(sp_start);
16338+
perl_assert_ptr(avarr);
16339+
SP += (av_cur-1); /* leave SP on top of last valid element, not 1 after */
16340+
PUTBACK;
16341+
/* Idealy Copy() will tailcall to libc or do a theoretical unrealistic
16342+
croak() which resumes normal PP control flow. So do all of Copy()'s
16343+
croak()s and checks earlier. Now the PUTBACK to global state can be
16344+
done safely before Copy/memcpy executes, and tailcail out of here. */
16345+
memcpy((char*)(sp_start),(const char*)(avarr), (av_cur) * sizeof(SV *));
1629816346
}
16299-
EXTEND(SP, AvFILLp(av)+1);
16300-
Copy(AvARRAY(av), &ST(0), AvFILLp(av)+1, SV *);
16301-
XSRETURN(AvFILLp(av)+1);
1630216347
}
1630316348

1630416349
/* Copy an existing cop->cop_warnings field.

0 commit comments

Comments
 (0)