Skip to content

Commit

Permalink
call_sv, amagic_call: call pp_entersub via runops
Browse files Browse the repository at this point in the history
These two functions do a slightly odd thing (which has been present
since 5.000) when calling out to a CV: they half fake up an OP_ENTERSUB,
then call pp_entersub() directly, and only then if it returns a non-null
PL_op value, execute the rest of the ops of the sub within a
CALLRUNOPS() loop.

I can't find any particular reason for this. I guess it might make
calling XS subs infinitesimally faster by not have to invoke the runops
loop when only a single op is executed (the entersub), but hardly seems
worth the effort.

Conversely, eliminating this special-case makes the code cleaner, and
it will allow the workarounds planned to be added shortly (to the runops
loops for reference-counted stacks) to work uniformly. Without this
commit, pp_entersub() would get called before runops() has had a chance
to fix up the stack if necessary.

So this commit *fully* populates the fake OP_ENTERSUB (including type
and pp address) then causes pp_entersub to be invoked implicitly from the
runops loop rather than explicitly.
  • Loading branch information
iabyn authored and demerphq committed Feb 28, 2023
1 parent e9ba788 commit 214a943
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 17 deletions.
13 changes: 6 additions & 7 deletions gv.c
Original file line number Diff line number Diff line change
Expand Up @@ -4079,7 +4079,7 @@ Perl_amagic_call(pTHX_ SV *left, SV *right, int method, int flags)

{
dSP;
BINOP myop;
UNOP myop;
SV* res;
const bool oldcatch = CATCH_GET;
I32 oldmark, nret;
Expand All @@ -4091,10 +4091,11 @@ Perl_amagic_call(pTHX_ SV *left, SV *right, int method, int flags)
? G_SCALAR : GIMME_V;

CATCH_SET(TRUE);
Zero(&myop, 1, BINOP);
myop.op_last = (OP *) &myop;
myop.op_next = NULL;
Zero(&myop, 1, UNOP);
myop.op_flags = OPf_STACKED;
myop.op_ppaddr = PL_ppaddr[OP_ENTERSUB];
myop.op_type = OP_ENTERSUB;


switch (gimme) {
case G_VOID:
Expand Down Expand Up @@ -4134,9 +4135,7 @@ Perl_amagic_call(pTHX_ SV *left, SV *right, int method, int flags)
PUSHs(MUTABLE_SV(cv));
PUTBACK;
oldmark = TOPMARK;

if ((PL_op = PL_ppaddr[OP_ENTERSUB](aTHX)))
CALLRUNOPS(aTHX);
CALLRUNOPS(aTHX);
LEAVE;
SPAGAIN;
nret = SP - (PL_stack_base + oldmark);
Expand Down
14 changes: 4 additions & 10 deletions perl.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,6 @@ static I32 read_e_script(pTHX_ int idx, SV *buf_sv, int maxlen);
# define validate_suid(rsfp) S_validate_suid(aTHX_ rsfp)
#endif

#define CALL_BODY_SUB(myop) \
if (PL_op == (myop)) \
PL_op = PL_ppaddr[OP_ENTERSUB](aTHX); \
if (PL_op) \
CALLRUNOPS(aTHX);

#define CALL_LIST_BODY(cv) \
PUSHMARK(PL_stack_sp); \
call_sv(MUTABLE_SV((cv)), G_EVAL|G_DISCARD|G_VOID);
Expand Down Expand Up @@ -3087,6 +3081,8 @@ Perl_call_sv(pTHX_ SV *sv, volatile I32 flags)
if (!(flags & G_NOARGS))
myop.op_flags |= OPf_STACKED;
myop.op_flags |= OP_GIMME_REVERSE(flags);
myop.op_ppaddr = PL_ppaddr[OP_ENTERSUB];
myop.op_type = OP_ENTERSUB;
SAVEOP();
PL_op = (OP*)&myop;

Expand Down Expand Up @@ -3119,13 +3115,11 @@ Perl_call_sv(pTHX_ SV *sv, volatile I32 flags)
method_op.op_ppaddr = PL_ppaddr[OP_METHOD];
method_op.op_type = OP_METHOD;
}
myop.op_ppaddr = PL_ppaddr[OP_ENTERSUB];
myop.op_type = OP_ENTERSUB;
}

if (!(flags & G_EVAL)) {
CATCH_SET(TRUE);
CALL_BODY_SUB((OP*)&myop);
CALLRUNOPS(aTHX);
retval = PL_stack_sp - (PL_stack_base + oldmark);
CATCH_SET(oldcatch);
}
Expand All @@ -3142,7 +3136,7 @@ Perl_call_sv(pTHX_ SV *sv, volatile I32 flags)
switch (ret) {
case 0:
redo_body:
CALL_BODY_SUB((OP*)&myop);
CALLRUNOPS(aTHX);
retval = PL_stack_sp - (PL_stack_base + oldmark);
if (!(flags & G_KEEPERR)) {
CLEAR_ERRSV();
Expand Down

0 comments on commit 214a943

Please sign in to comment.