This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
perl5.git
5 years agoleave_adjust_stacks() fix some code comments
David Mitchell [Mon, 18 Jan 2016 13:09:35 +0000 (13:09 +0000)]
leave_adjust_stacks() fix some code comments

One comment was obsolete; the other referred to the wrong pp function

5 years agoleave_adjust_stacks(): avoid accessing random tmps
David Mitchell [Mon, 18 Jan 2016 12:31:24 +0000 (12:31 +0000)]
leave_adjust_stacks(): avoid accessing random tmps

There was some code in leave_adjust_stacks() that checked whether the
current arg sv being processed was the same SV as the first SvTEMP
above the 'cut' on the tmps stack. If there was nothing above the cut,
it was actually comparing against whatever garbage was 1 slot above the
current PL_tmps_ix. This was almost always harmless (but of course wrong);
the only symptom was an occasional smoke failure in t/re/pat_re_eval_thr.t,
due to this:

    local our $s = "abc";
    my $qr = qr/^(?{1})$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s/;

where a qr// with a code blocks acts like

    my $qr = sub : lvalue { .....; }->()

to make closures happen correctly. The lvalue return from the anon sub was
triggering this because the address of $s was in one of the unused slots
above PL_tmp_ix.

I couldn't get it to fail in a simple test case.

At the same time, I moved a SvREFCNT_inc() inside a check for
!SvIMMORTAL(sv) since there's no need to do it for PL_sv_undef etc.

5 years agomake gimme consistently U8
David Mitchell [Mon, 4 Jan 2016 09:16:52 +0000 (09:16 +0000)]
make gimme consistently U8

The value of gimme stored in the context stack is U8.
Make all other uses in the main core consistent with this.

My primary motivation on this was that the new function cx_pushblock(),
which I gave a 'U8 gimme' parameter, was generating warnings where callers
were passing I32 gimme vars to it. Rather than play whack-a-mole, it
seemed simpler to just uniformly use U8 everywhere.

Porting/bench.pl shows a consistent reduction of about 2 instructions on
the loop and sub benchmarks, so this change isn't harming performance.

5 years agofix -DPERL_GLOBAL_STRUCT_PRIVATE
David Mitchell [Sun, 3 Jan 2016 19:38:13 +0000 (19:38 +0000)]
fix -DPERL_GLOBAL_STRUCT_PRIVATE

Perl_leave_adjust_stacks() needed a dVAR

5 years agoperlfunc: say what block types 'return' recognises
David Mitchell [Sun, 3 Jan 2016 17:28:06 +0000 (17:28 +0000)]
perlfunc: say what block types 'return' recognises

'return' recognises these blocks

    sort { ...; return } ...
    /(?{ ...; return })/;

but ignores these:

    grep { ...; return } ...;
    map  { ...; return } ...;

5 years agoperlguts: add section on context stack
David Mitchell [Sun, 3 Jan 2016 15:23:56 +0000 (15:23 +0000)]
perlguts: add section on context stack

5 years agofix cx_dup for CXt_LOOP_PLAIN
David Mitchell [Sun, 3 Jan 2016 15:17:40 +0000 (15:17 +0000)]
fix cx_dup for CXt_LOOP_PLAIN

The context stack duplication code tries to duplicate the loop var
even for CXt_LOOP_PLAIN, which doesn't have a loop var. This didn't
use to matter, since PUSHLOOP_PLAIN() used to set the field to NULL;
for efficiency its now left untouched. So don't try to use it.

Also update the debugging context names since the ordering of the
CXt_LOOP_* has changed recently.

5 years agoMULTICALL *shouldn't* clear savestack
David Mitchell [Thu, 31 Dec 2015 10:39:17 +0000 (10:39 +0000)]
MULTICALL *shouldn't* clear savestack

About 25 commits ago in this branch I added a commit:

    MULTICALL should clear scope after each call

To fix RT #116577, which reported that lexicals were only being freed
at the end of the MULTICALL, not after each individual call to the sub.

In that commit, I added a LEAVE_SCOPE() to the end of the MULTICALL()
definition. However, after further thought I realise that's wrong.  If a
multicall sub does something like { my $x = $_*2; $x }, then the returned
value would be freed before the XS code which calls MULTICALL() has a
chance to do anything with it (e.g. test for truth, or add it to the return
args or whatever).

So I think popping the save stack should be the responsibility of the
caller of MULTICALL(), rather than of MULTICALL() itself.

5 years agoadd blk_old_tmpsfloor shortcut
David Mitchell [Wed, 30 Dec 2015 15:48:52 +0000 (15:48 +0000)]
add blk_old_tmpsfloor shortcut

Add

    #define blk_old_tmpsfloor cx_u.cx_blk.blku_old_tmpsfloor

to match all the other 'struct block' fields which have similar short cuts

5 years agodMULTICALL: remove unused vars
David Mitchell [Wed, 30 Dec 2015 15:20:41 +0000 (15:20 +0000)]
dMULTICALL: remove unused vars

dMULTICALL declares several vars that are used either to maintain
state across multiple calls, or to pass values to PUSHSUB etc, where
those macros expected to obtain some of their args by values being
implicitly passed via local vars. Since PUSHSUB has been replaced by
cx_pushsub() which now has all parameters explicitly passed, there is
no longer any need for those vars. So this commit eliminates them:

    newsp
    hasargs

There are also a couple vars which are no longer used due to changes to
the implementation over time; these can also be eliminated:

    cx multicall_cv

Finally, this branch introduced a new var, saveix_floor; rename it to
multicall_saveix_floor for consistency with other dMULTICALL vars.

Although none of these vars are listed in the documentation, its possible
that some code out there may rely on them in some way, and will need to be
fixed up.

5 years agoconvert CX_{PUSH|POP}{WHEN|GIVEN} to inline fns
David Mitchell [Wed, 30 Dec 2015 14:33:51 +0000 (14:33 +0000)]
convert CX_{PUSH|POP}{WHEN|GIVEN} to inline fns

Replace CX_PUSHGIVEN() with cx_pushgiven() etc.

5 years agoconvert CX_PUSHLOOP*/POPLOOP to inline fns
David Mitchell [Wed, 30 Dec 2015 14:18:05 +0000 (14:18 +0000)]
convert CX_PUSHLOOP*/POPLOOP to inline fns

Replace CX_PUSHLOOP_FOR() with cx_pushfloop_for() etc.

5 years agoconvert CX_PUSHEVAL/POPEVAL to inline fns
David Mitchell [Wed, 30 Dec 2015 13:23:47 +0000 (13:23 +0000)]
convert CX_PUSHEVAL/POPEVAL to inline fns

Replace CX_PUSHEVAL() with cx_pusheval() etc.

No functional changes.

5 years agoconvert CX_PUSHFORMAT/POPFORMAT to inline fns
David Mitchell [Wed, 30 Dec 2015 13:09:36 +0000 (13:09 +0000)]
convert CX_PUSHFORMAT/POPFORMAT to inline fns

Replace CX_PUSHFORMAT() with cx_pushformat() etc.

No functional changes.

5 years agoconvert CX_PUSHSUB/POPSUB to inline fns
David Mitchell [Wed, 30 Dec 2015 12:33:48 +0000 (12:33 +0000)]
convert CX_PUSHSUB/POPSUB to inline fns

Replace CX_PUSHSUB() with cx_pushsub() etc.

No functional changes.

5 years agoconvert CX_PUSH/POP/TOPBLOCK to inline fns
David Mitchell [Wed, 30 Dec 2015 11:46:22 +0000 (11:46 +0000)]
convert  CX_PUSH/POP/TOPBLOCK to inline fns

Replace CX_PUSHBLOCK() with cx_pushblock() etc.

No functional changes.

5 years agoadd a few grep and map benchmarks
David Mitchell [Tue, 29 Dec 2015 10:05:29 +0000 (10:05 +0000)]
add a few grep and map benchmarks

5 years agooffset PL_savestack_max by SS_MAXPUSH
David Mitchell [Sun, 27 Dec 2015 14:07:02 +0000 (14:07 +0000)]
offset PL_savestack_max by SS_MAXPUSH

The newer SS_ADD macros expect there to always be SS_MAXPUSH slots
free on the savestack; so they can push multiple items, then only check
once at the end whether stack needs expanding.

This commit makes savestack_grow() set PL_savestack_max to SS_MAXPUSH
short of what has actually been allocated. This makes the tests
to see whether the stack needs growing slightly simpler, i.e.

    PL_savestack_ix > PL_savestack_max

rather than

    PL_savestack_ix + SS_MAXPUSH > PL_savestack_max

5 years agoadd SAVEt_TMPSFLOOR save type and Perl_savetmps()
David Mitchell [Sun, 27 Dec 2015 10:41:41 +0000 (10:41 +0000)]
add SAVEt_TMPSFLOOR save type and Perl_savetmps()

By making SAVETMPS have its own dedicated save type, it avoids having to
push the address of PL_tmps_floor onto the save stack each time.
By also giving it a dedicated save function, the function can do
the PL_tmpsfloor = PL_tmps_ix step too, making the binary slightly more
compact.

5 years agorename PUSHBLOCK,PUSHSUB etc to CX_PUSHBLOCK etc
David Mitchell [Sat, 26 Dec 2015 22:28:59 +0000 (22:28 +0000)]
rename PUSHBLOCK,PUSHSUB etc to CX_PUSHBLOCK etc

Earlier all the POPFOO macros were renamed to CX_POPFOO to reflect
the changed API (like POPBLOCK no longer decremented cxstack_ix).

Now rename the PUSH ones for consistency.

5 years agoeliminate PUSH/POPBASICBLK macros
David Mitchell [Sat, 26 Dec 2015 22:14:00 +0000 (22:14 +0000)]
eliminate PUSH/POPBASICBLK macros

These are both NOOPs now, and were introduced within this branch
as a temporary measure while extra stuff needed doing when pushing or
popping a CXt_BOCK (pp_enter/pp_leave).

5 years agopp_enteriter: add comment about setting cxt type
David Mitchell [Sat, 26 Dec 2015 21:49:28 +0000 (21:49 +0000)]
pp_enteriter: add comment about setting cxt type

5 years agoconsolidate common code in PUSHLOOP_FOR,_PLAIN
David Mitchell [Sat, 26 Dec 2015 13:48:43 +0000 (13:48 +0000)]
consolidate common code in PUSHLOOP_FOR,_PLAIN

5 years agoPUSHEVAL: make n param an SV rather than a string
David Mitchell [Sat, 26 Dec 2015 12:45:01 +0000 (12:45 +0000)]
PUSHEVAL: make n param an SV rather than a string

Rather than doing

    cx->blk_eval.old_namesv = (n ? newSVpv(n,0) : NULL);

make the caller responsible for creating and passing in the SV. Since
only only place (pp_require) passes a non-null value, this saves the
other places having to test for nullness.

5 years agoPUSHSUB: make retop a parameter
David Mitchell [Sat, 26 Dec 2015 12:37:30 +0000 (12:37 +0000)]
PUSHSUB: make retop a parameter

Rather than doing cx->blk_sub.retop = NULL in PUSHSUB, then relying on
the caller to subsequently change it to something more useful, make it an
arg to PUSHSUB.

5 years agoPUSHEVAL: make retop a parameter
David Mitchell [Sat, 26 Dec 2015 12:30:25 +0000 (12:30 +0000)]
PUSHEVAL: make retop a parameter

Rather than doing cx->blk_eval.retop = NULL in PUSHEVAL, then relying on
the caller to subsequently change it to something more useful, make it an
arg to PUSHEVAL.

5 years agoPUSHFORMAT: don't use implicit args
David Mitchell [Sat, 26 Dec 2015 12:07:50 +0000 (12:07 +0000)]
PUSHFORMAT: don't use implicit args

Make cv and gv explicit parameters of PUSHFORMAT(), rather than just
assuming that there are such vars in scope.

5 years agoPUSHSUB: don't use implicit args
David Mitchell [Sat, 26 Dec 2015 11:52:33 +0000 (11:52 +0000)]
PUSHSUB: don't use implicit args

Make cv and hasargs explicit parameters of PUSHSUB(), rather than just
assuming that there are such vars in scope.

5 years agoPUSHBLOCK: don't use implicit args
David Mitchell [Sat, 26 Dec 2015 11:39:30 +0000 (11:39 +0000)]
PUSHBLOCK: don't use implicit args

Make gimme a parameter of PUSHBLOCK() rather than just assuming that
there's a 'gimme' var in scope.

5 years agomove PL_savestack_ix saving into PUSHBLOCK
David Mitchell [Fri, 25 Dec 2015 23:54:23 +0000 (23:54 +0000)]
move PL_savestack_ix saving into PUSHBLOCK

Currently blku_oldsaveix was being set by the various PUSHFOO macros,
except for PUSHSUB and PUSHEVAL which expected their caller to do it
manually.

Now that all the main context state is stored on the context stack
rather than than some on the save stack too, things are a lot simpler,
and this messy transitional state can now be rationalised, whereby
blku_oldsaveix is now always set by PUSHBLOCK; the exact value being
specified by a new arg to PUSHBLOCK.

5 years agoPUSH_MULTICALL: use SAVEOP()
David Mitchell [Fri, 25 Dec 2015 23:12:29 +0000 (23:12 +0000)]
PUSH_MULTICALL: use SAVEOP()

SAVEOP() should be more efficient than SAVEVPTR(PL_op), since it
uses the dedicated SAVEt_OP.

5 years agoeliminate PERL_STACK_OVERFLOW_CHECK
David Mitchell [Fri, 25 Dec 2015 22:41:21 +0000 (22:41 +0000)]
eliminate PERL_STACK_OVERFLOW_CHECK

This macro is defined as NOOP on all platforms except for MacOS classic,
where it was added as a hook to allow for OSes that have a small CPU
stack size. Since pp_entersub et al don't actually use the CPU stack,
this hook looks misconceived from the beginning. So remove all
uses of it in the core.

5 years agosort compare subs: don't do unnecessary scope work
David Mitchell [Fri, 25 Dec 2015 22:28:14 +0000 (22:28 +0000)]
sort compare subs: don't do unnecessary scope work

The 3 functions S_sortcv(), S_sortcv_stacked(), S_sortcv_xsub(),
which call a comparison function to compare $a and $b, do unnecessary work
with the scope and save stacks.

First, they pop any excess scope stack entries; but there shouldn't
be, since exiting the sort sub should have already pooped any inner
scopes. Indeed, running with an assert that PL_scopestack_ix == oldscopeix
didn't trigger any failures.

Secondly replace the unconditional leave_scope(oldsaveix) with
LEAVE_SCOPE(), which only calls the function if PL_savestack_ix >
oldsaveix.

5 years agoMULTICALL should clear scope after each call
David Mitchell [Fri, 25 Dec 2015 22:03:10 +0000 (22:03 +0000)]
MULTICALL should clear scope after each call

RT #116577

Lexicals etc were only being freed at the end of the MULTICALL, not
after each individual call to the sub.

5 years agoDocument CxLVAL()
David Mitchell [Fri, 25 Dec 2015 12:03:00 +0000 (12:03 +0000)]
Document CxLVAL()

5 years agoCX_POPFOO(): assert cx is of the right type
David Mitchell [Fri, 25 Dec 2015 11:51:37 +0000 (11:51 +0000)]
CX_POPFOO(): assert cx is of the right type

At the start of each CX_POPFOO(cx) macro, add an assertion that cx is of
type CXt_FOO.

5 years agorename POPFOO() to CX_POPFOO()
David Mitchell [Fri, 25 Dec 2015 11:07:28 +0000 (11:07 +0000)]
rename POPFOO() to CX_POPFOO()

Rename all the context-popping macros such as POPBLOCK and POPSUB, by
giving them a CX_ prefix. (Do TOPBLOCK too).

This is principally to  deliberately break any existing non-core use of
these non-API macros, as their behaviour has changed in this branch.
In particular, POPBLOCK(cx) no longer decrements the cxt stack pointer
nor sets cx; instead, cx is now expected to already point to the stack
frame which POPBLOCK should process.

At the same time, giving them a CX_ prefix makes it clearer that these
are all part of a family of macros that manipulate the context stack.

The PUSHFOO() macros will be renamed in a later commit.

5 years agopp_redo()): reorder some stuff
David Mitchell [Thu, 24 Dec 2015 23:12:37 +0000 (23:12 +0000)]
pp_redo()): reorder some stuff

It probably doesn't make any difference, but reorder the FREETMPS,
CX_LEAVE_SCOPE and TOPBLOCK so it matches the order we do things when
leaving a scope.

5 years agooptimise bare 'next'
David Mitchell [Thu, 24 Dec 2015 22:37:48 +0000 (22:37 +0000)]
optimise bare 'next'

If a 'next' without a label appears directly in the scope of the current
loop, then skip searching the context stack for a suitable LOOP context.

5 years agomake S_unwind_loop static again
David Mitchell [Wed, 3 Feb 2016 09:12:56 +0000 (09:12 +0000)]
make S_unwind_loop static again

The previous commit but one accidentally removed the 'static' declaration

5 years agoS_unwind_loop(): remove opname param
David Mitchell [Thu, 24 Dec 2015 21:48:23 +0000 (21:48 +0000)]
S_unwind_loop(): remove opname param

This is only used for error messages, and can be derived from
OP_NAME(PL_op); so for efficiency, don't pass it.

5 years agoS_unwind_loop(): return pointer rather than index
David Mitchell [Thu, 24 Dec 2015 21:33:49 +0000 (21:33 +0000)]
S_unwind_loop(): return pointer rather than index

return &cxstack[cxix] rather than cxix, since this is what the caller
actually needs.

5 years agofactor out common actions in TOPBLOCK and POPBLOCK
David Mitchell [Thu, 24 Dec 2015 20:30:15 +0000 (20:30 +0000)]
factor out common actions in TOPBLOCK and POPBLOCK

5 years agodounwind(): do a POPBLOCK for final cx frame.
David Mitchell [Thu, 24 Dec 2015 19:44:05 +0000 (19:44 +0000)]
dounwind(): do a POPBLOCK for final cx frame.

Previously dounwind() relied on the caller to a TOPBLOCK or POPBLOCK
following the call to dounwind(). It's debatable who should be
responsible. Arguably its more efficient for dounwind() not to do a
POPBLOCK, since the caller will probably immediately follow on with
POPFOO; POPBLOCK for the next context frame anyway.

Logically however, dounwind() should do this, and its not possible
for the caller to do so retrospectively, as context frame cxstack_ix + 1
may have been overwritten by the time dounwind returns.

Also, the changes in this branch mean that the old PL_tmps_floor is now
saved in the context struct rather than on the save stack, so code that
does C<dounwind(-1); LEAVE_SCOPE();> will no longer automatically
restore PL_tmps_floor. With thiis commit, it will.

The change to pp_return reflects that we now need to copy any return args
*before* donwind() is called, so that "return $1" will mg_get($1) while
the correct (inner) PL_curpm is still in scope.

5 years agopp_break(): don't use TOPBLOCK
David Mitchell [Mon, 21 Dec 2015 16:33:57 +0000 (16:33 +0000)]
pp_break(): don't use TOPBLOCK

It appears to be using TOPBLOCK purely for its effect of resetting
PL_stack_sp. Since the next op will be pp_leavegiven which will do a
POPBLOCK, the other actions of TOPBLOCK are redundant. So just set
PL_stack_sp directly.

5 years agotweak POPLOOP and CXt_LOOP_* order
David Mitchell [Mon, 21 Dec 2015 16:00:59 +0000 (16:00 +0000)]
tweak POPLOOP and CXt_LOOP_* order

Re-arrange the order of the CXt_LOOP_* to make it easier for compilers
to produce efficient code, and tweak POPLOOP so that the code for
freeing ary and cur is shared (as they occupy the same position in the
union).

5 years agofix CxFOREACH
David Mitchell [Mon, 21 Dec 2015 15:45:57 +0000 (15:45 +0000)]
fix CxFOREACH

It wasn't detecting CXt_LOOP_LAZYIV as a 'for' loop type. This only
affected 'given' and 'break' (which need to distinguish between being
in a 'given' block and being in a 'for' block).

5 years agoonly set CXp_FOR_DEF with CXp_FOR_GV
David Mitchell [Mon, 21 Dec 2015 13:32:13 +0000 (13:32 +0000)]
only set CXp_FOR_DEF with CXp_FOR_GV

C<for (...)> is a special-case of C<for $pkg_var (...)>, so CXp_FOR_DEF
should only be set when CXp_FOR_GV also is. So in pp_enteriter(), only
test for (PL_op->op_private & OPpITER_DEF) in the GV branch and assert
that it's otherwise unset. This is slightly more efficient in the non-GV
branches.

Also add some more commentary to the CXp_FOR_* flag definitions.

5 years agogive POP_SAVEARRAY() macro a cx arg
David Mitchell [Fri, 18 Dec 2015 22:09:58 +0000 (22:09 +0000)]
give POP_SAVEARRAY() macro a cx arg

rather than just assuming that there's a 'cx' var in scope

5 years agoclarify code comment in pp_goto(()
David Mitchell [Fri, 18 Dec 2015 22:07:25 +0000 (22:07 +0000)]
clarify code comment in pp_goto(()

5 years agoAPItest.xs: fixup clone_with_stack()
David Mitchell [Fri, 18 Dec 2015 21:33:10 +0000 (21:33 +0000)]
APItest.xs: fixup clone_with_stack()

Make the unwinding and freeing of the old interpreter up to date
with the latest handling of PL_scopestack.

5 years agofixup cx_dup()
David Mitchell [Fri, 18 Dec 2015 20:18:13 +0000 (20:18 +0000)]
fixup cx_dup()

By visual inspection, tweak bits of the context stack duplicating code
to reflect recent changes in this branch, especially new fields, or
whether fields hold a reference count or not.

Since I don't use Windows, this has largely been done blind.

5 years agopp_iter: optimise integer setting in for (1..10) {}
David Mitchell [Fri, 18 Dec 2015 16:36:54 +0000 (16:36 +0000)]
pp_iter: optimise integer setting in for (1..10) {}

If the target SV is a simple SVt_IV, which it is likely to be,
just update it directly rather than calling sv_setiv()

5 years agofix *_ = "" for 0 .. 1;
David Mitchell [Fri, 18 Dec 2015 15:09:42 +0000 (15:09 +0000)]
fix *_ = "" for 0 .. 1;

RT #123994

pp_iter couldn't handle GvSv(gv) being NULL.

In that ticket, Tony Cook suggested two possible fixes. First,
always instantiate the GvSV slot at the start of pp_iter by using
GvSVn rather than GvSV in the CxITERVAR() macro;

Second, test for it being null within the two 'range' branches,
(for(1..9), for('a'..'z')), and if so create it. One advantage of doing
it there is that there's already code for (re)creating the SV if the
reference count is != 1. It also means that the list and array cases
(for(@a), for(1,3,5)) which always put the next iterated SV into the
pad/GvSV slot don't waste time creating and then immediately discarding an
SV if GvSV was NULL.

I went for the second fix.

It also means that's there's no longer any need in pp_enteriter to
initially poulate GvSV is it was null, as this will be detected during
the first pp_iter() anyway.

5 years agopp_enteriter: use efficient SvREFCNT_inc variant
David Mitchell [Fri, 18 Dec 2015 12:34:37 +0000 (12:34 +0000)]
pp_enteriter: use efficient SvREFCNT_inc variant

replace a couple of SvREFCNT_inc() with SvREFCNT_inc_simple_void_NN().

Also add a couple of code comments.

5 years agoprovide some basic documentation for Perl_dounwind
David Mitchell [Thu, 17 Dec 2015 17:50:52 +0000 (17:50 +0000)]
provide some basic documentation for Perl_dounwind

5 years agoTOPBLOCK: make comment clear its used by goto too
David Mitchell [Thu, 17 Dec 2015 17:34:36 +0000 (17:34 +0000)]
TOPBLOCK: make comment clear its used by goto too

5 years agoPOPSUB_ARGS: move a code comment to the right line
David Mitchell [Thu, 17 Dec 2015 17:30:48 +0000 (17:30 +0000)]
POPSUB_ARGS: move a code comment to the right line

5 years agopp_ctl.c: s/newsp/oldsp/g
David Mitchell [Thu, 17 Dec 2015 12:37:17 +0000 (12:37 +0000)]
pp_ctl.c: s/newsp/oldsp/g

There are various pp_leavefoo() functions that have similar code along the
lines of

    SV **newsp;
    newsp = PL_stack_base + cx->blk_oldsp;

Rename all these vars to 'oldsp' to reduce the cognitive dissonance.
The name 'newsp' was a hangover from when POPBLOCK used to implicitly
set a var called 'newsp.

5 years agoreplace leave_common() with leave_adjust_stacks()
David Mitchell [Thu, 17 Dec 2015 12:13:09 +0000 (12:13 +0000)]
replace leave_common() with leave_adjust_stacks()

Make the remaining callers of S_leave_common() use leave_adjust_stacks()
instead, then delete this static function.

This brings the benefits of freeing TEMPS on all scope exists that
has already been introduced on sub exits; uses the optimised code for
creating mortal copies; and finally unifies all the different 'process
return args on scope exit' implementations into single function.

5 years agomake pp_return() use leave_adjust_stacks()
David Mitchell [Wed, 16 Dec 2015 14:52:22 +0000 (14:52 +0000)]
make pp_return() use leave_adjust_stacks()

It was using S_leave_common(), but that's shortly to be removed.  It also
required adding an extra arg to leave_adjust_stacks() to indicate where to
shift the return args to. This will also be needed for when we replace the
remaining uses of  S_leave_common() with leave_adjust_stacks().

5 years agomake pp_leavesublv use S_leavesub_adjust_stacks()
David Mitchell [Wed, 16 Dec 2015 12:30:01 +0000 (12:30 +0000)]
make pp_leavesublv use S_leavesub_adjust_stacks()

Currently S_leavesub_adjust_stacks() is just used by pp_leavesub.
Rename it to Perl_leave_adjust_stacks(), extend its functionality
slightly, then make pp_leavesublv() use it too.

This means that lvalue sub exit gains the benefit of FREETMPS being done,
and (where mortal copying needs doing) the optimised copying code.
It also means there is now one less version of the "process args on scope
exit" code.

pp_leavesublv() still does a scan of its return args looking for things to
croak() on, but leaves everything else to leave_adjust_stacks().

leave_adjust_stacks() is intended shortly to be used in place of
S_leave_common() too, thus unifying all args-on-scope-exit code.

The changes to leave_adjust_stacks() in this commit (apart from the
renaming and doc changes) are:
* a new arg to indicate what condition to use to decide whether to
  pass or copy the arg;
* a new branch to mortalise and ref count bump an arg

5 years agopp_leavesublv(): document PL_sv_undef exception
David Mitchell [Mon, 14 Dec 2015 12:10:45 +0000 (12:10 +0000)]
pp_leavesublv(): document PL_sv_undef exception

5 years agopp_leavesublv(): croak on *all* PADTMPs
David Mitchell [Mon, 14 Dec 2015 11:55:14 +0000 (11:55 +0000)]
pp_leavesublv(): croak on *all* PADTMPs

pp_leavesublv() generally croaks on returned PADTMPs when called in lvalue
context. However, an exception was made in scalar context if the PADTMP
had set magic. This was to allow for things like

    sub :lvalue { $tied{foo} }
and
    sub :lvalue { substr($foo,1,2) }

However, it was later found that in places like pp_substr(), when
returning a PVLV, it should return a new mortal rather than upgrading
its pad target to PVLV, because the PVLV holds a reference to $foo which
then gets delayed being freed indefinitely.

Since places like pp_susbtr() no longer return lvalue PADTMPs, there's
no longer any need to make a special exception in pp_leavesublv().

I've added an assertion to the effect that PADTMPs don't have set
container magic, but I've added the assert to pp_leavesub() rather than
pp_leavesublv(), since the former is much likely to be used in many weird
situations and edge cases that would quickly flush out any false
assumptions.

If this assumption is wrong and the exception needs to be re-instated in
pp_leavesublv(), then presumably it needs adding to the ARRAY context
branch too - I'm assuming that its previous absence there was an oversight;
i.e.

    sub foo :lvalue { $tied{foo}, substr($foo,1,2) }
    foo() = (1,2);

should work too.

5 years agopp_leavesub(): call FREETMPS and optimise
David Mitchell [Sun, 8 Nov 2015 15:16:09 +0000 (15:16 +0000)]
pp_leavesub(): call FREETMPS and optimise

Currently pp_leavesub() doesn't call FREETMPS. Presumably this is due to
the danger of freeing temps that need to exist beyond the return, such
as the mortal copies we make of return args, or return args that are
already temps.

The down side of this is that temps aren't freed until the next nextstate
is executed following the function call. If the function isn't near a
statement boundary, then it may be a while before temps are freed; e.g.
in f(g()), when g() returns, its temps continue to exist during the call
to f(). For recursive subs it gets worse, although there is a specific
hack already in pp_leavesub() that says in scalar context if CvDEPTH > 1,
then temporarily RC++ the single return value then do a FREETMPS. This
can in theory leak if something dies during the FREETMPS.

This commit provides a more general solution. During the course of
processing (usually mortal copying) the return args, it divides the
current temps stack frame into two halves, with the temps that need
keeping migrating to the bottom half. It then does a FREETMPS equivalent
only of the top half.

This is actually more efficient than it sounds; but in addition, the code
has been heavily optimised: in particular the call to sv_mortalcopy()
has been unrolled and inlined, and within that code common cases (such as
IV, RV) handled directly, and the mortal stack is only checked/extended
once, rather than for every arg.

Also, the arg adjust / freetmps code has been moved out of pp_leavesub()
and into a separate static function.

5 years agomove SET_SVANY_FOR_BODYLESS_IV() from sv.c to sv.h
David Mitchell [Fri, 4 Dec 2015 15:50:54 +0000 (15:50 +0000)]
move SET_SVANY_FOR_BODYLESS_IV() from sv.c to sv.h

Ditto for SET_SVANY_FOR_BODYLESS_NV.

The IV variant will shortly be needed in pp_hot.c, so make it
available outside sv.c. For now, protect with #ifdef PERL_CORE,
so as little as possible is changed by this commit.

5 years agoPerl_runops_debug(): do FREETMPS
David Mitchell [Mon, 16 Nov 2015 14:40:04 +0000 (14:40 +0000)]
Perl_runops_debug(): do FREETMPS

In runops_debug() wrap the optional printing of next op, arg stack
etc with ENTER/SAVETMPS, FREETMPS/LEAVE - so that temporaries created by
the dump output are promptly freed, and thus don't alter the tmps stack.

(I'm trying to debug some tmps stack corruption, and running with -Dst
made the problem go away).

5 years agooptimise sv_setsv_flags()
David Mitchell [Mon, 9 Nov 2015 21:11:58 +0000 (21:11 +0000)]
optimise sv_setsv_flags()

This commit does two things.

First, it streamlines and re-orders some of the initial tests,
such as 'is sstr already freed?'.

Second, it looks for a reasonably common case where both sstr and dstr are
SVt_NULL/SVt_IV. This covers undef, int and refs, where the SV hasn't
previously been used for other things (like strings).

With just SVt_NULL/SVt_IV, we know that the SV won't have a real body, and
won't need one and can be very quick.

The check for SVt_NULL/SVt_IV is a single compare-and-branch, so
has a minimal effect on users of sv_setsv_flags() that have more complex
types.

5 years agointrvar.h: document PL_tmps_max
David Mitchell [Sun, 8 Nov 2015 20:08:20 +0000 (20:08 +0000)]
intrvar.h: document PL_tmps_max

Its name implies that it's the top allocated element; in fact it's top+1.

5 years agopp_leavesub: reset SP in void context
David Mitchell [Sun, 8 Nov 2015 15:05:01 +0000 (15:05 +0000)]
pp_leavesub: reset SP in void context

In void context, pp_leavesub() doesn't bother resetting SP to the
base; either as an efficiency measure, or as an oversight (the other
pp_leavefoo functions do reset).

This is mostly harmless, as being void context, it's likely to immediately
execute pp_nextstate or similar, which will reset the stack anyway.
However with attributes, something like

    my @foo :attr = ()

causes attributes::import() to be called in void context, and whatever
dross it leaves on the stack becomes part of the assign, i.e. that assign
becomes the equivalent of:

    my (@foo, dross) = ()

which again is fairly harmless, if slightly inefficient.

However, the next commit should make pp_leavesub() call FRETMPS,
which means that 'dross' may now include freed SVs, which will make
pp_aassign choke.

This commit also requires ext/XS-APItest/t/call.t to be fixed.
Some tests in there (added years ago by myself) test the statu quo; that
is, it expects that calling call_sv(G_VOID) will leave return args on the
stack. Now it doesn't.

5 years agoPerl_free_tmps(): don't test for PL_sv_undef
David Mitchell [Sun, 8 Nov 2015 19:19:02 +0000 (19:19 +0000)]
Perl_free_tmps(): don't test for PL_sv_undef

Each sv being popped off the tmps stack is compared against PL_sv_undef,
and if so, the SvTEMP_off(sv); SvREFCNT_dec_NN(sv); is skipped.

This condition was added by 8aacddc1 back in 2001, as part of a commit
to add support for hash placeholders. Initially it used &PL_sv_undef in
HE value slots to indicate a placeholder. In a later commit, that was
changed to &PL_sv_placeholder, but the test in free_tmps() continued
to test against &PL_sv_undef.

Im not sure what the original intent was, but I strongly suspect that
whatever purpose it may have originally served, it no longer served that
after the switch to PL_sv_placeholder.

On a purely pragmatic note, I can't see any harm in untemping PL_sv_undef;
at worst its ref count will occasionally reach 0, and the special code in
sv_free2() will reset its count to SvREFCNT_IMMORTAL. And no tests
fail....

5 years agorestore PL_tmps_floor on exit
David Mitchell [Wed, 28 Oct 2015 08:28:56 +0000 (08:28 +0000)]
restore PL_tmps_floor on exit

A couple of places were expecting dounwind(-1); LEAVE_SCOPE(0);
to restore PL_tmps_floor, since its old value was saved on the savestack.
Since that's now stored in the context struct instead, do
a POPBLOCK(&cxstack[0]).

This problem only showed up on 'make test' rather than 'make
test_harness', since only the former sets PERL_DESTRUCT_LEVEL.

5 years agoextend magic copy test to all scope exit types
David Mitchell [Wed, 21 Oct 2015 18:10:34 +0000 (19:10 +0100)]
extend magic copy test to all scope exit types

Commit v5.15.6-387-g6f48390 forced leavesub to copy returned items
if they were get-magical. Normally rvalue subs  are supposed to return a
copy of their return args, but that copy is sometimes skipped if leavesub
thinks the side-effects will never be visible. Tied elements was an
example where the implementation leaked.

However, this applies equally well to other leave types, such as
do { ....}, so test for get magic in those too.

5 years agotest that pp_leavesub copies returned PADTMPs.
David Mitchell [Wed, 21 Oct 2015 16:54:29 +0000 (17:54 +0100)]
test that pp_leavesub copies returned PADTMPs.

Although pp_leavesub() has in fact always done this, it was never actually
tested for.

5 years agoAlways copy return values when exiting scope
David Mitchell [Wed, 21 Oct 2015 12:10:44 +0000 (13:10 +0100)]
Always copy return values when exiting scope

v5.14.0-642-g3ed94dc fixed certain instances where returning from a sub
incorrectly returned the actual value rather than a copy, e.g.

    sub f { delete $foo{bar} }

This was because if the value(s) being returned had SvTEMP set, copying
was skipped. That commit added an extra condition to the skip test,
SvREFCNT(sv) == 1.

However, this applies equally well to other scope exits, for example

    do { ...; delete $foo{bar} }

So this commits adds the RC==1 test to S_leave_common() too, which handles
all the non-sub scope exits. As well as adding a test to do.t, it adds an
additional test to sub.t, since the original tests, although they
*detected* a non-copied return, didn't actually demonstrate a case where
it was actually harmful.

Note that S_leave_common() also sometimes skips on PADTMPs as well as
TEMPs, so this commit as a side-effect also makes it copy PADTMPs unless
their RC ==1. But since their RC should in fact almost always be 1 anyway,
in practice it makes no difference.

5 years agoop/sub.t: fix ticket number in comment
David Mitchell [Wed, 21 Oct 2015 11:28:35 +0000 (12:28 +0100)]
op/sub.t: fix ticket number in comment

Took me a while to track down that non-existent ticket!

5 years agomake TOPBLOCK(cx) not set cx
David Mitchell [Wed, 21 Oct 2015 08:47:21 +0000 (09:47 +0100)]
make TOPBLOCK(cx) not set cx

i.e. replace

    TOPBLOCK(cx);

with

     cx = CX_CUR();
     TOPBLOCK(cx);

This is part of general trend of separating out the cx/cxstack_ix
setting/manipulation from the save/restore actions

5 years agorename DEBUG_CX() to CX_DEBUG()
David Mitchell [Wed, 21 Oct 2015 08:41:52 +0000 (09:41 +0100)]
rename DEBUG_CX() to CX_DEBUG()

For consistency with the new naming convention for context-stack macros.
Also, give it a cx arg rather than using CX_CUR()

5 years agoadd CX_CUR() macro
David Mitchell [Wed, 21 Oct 2015 08:28:52 +0000 (09:28 +0100)]
add CX_CUR() macro

This is simply

    #define CX_CUR() (&cxstack[cxstack_ix])

5 years agomake POPSUBST re-entrant safe
David Mitchell [Mon, 19 Oct 2015 16:21:58 +0000 (17:21 +0100)]
make POPSUBST re-entrant safe

5 years agomake POPGIVEN re-entrant safe
David Mitchell [Mon, 19 Oct 2015 16:14:58 +0000 (17:14 +0100)]
make POPGIVEN re-entrant safe

5 years agomake POPSUB re-entrant safe
David Mitchell [Mon, 19 Oct 2015 16:04:02 +0000 (17:04 +0100)]
make POPSUB re-entrant safe

5 years agomake POPEVAL safe against re-entrancy
David Mitchell [Mon, 19 Oct 2015 15:46:06 +0000 (16:46 +0100)]
make POPEVAL safe against re-entrancy

5 years agomake POPSUB and POPFORMAT re-entrant safe
David Mitchell [Mon, 19 Oct 2015 15:12:33 +0000 (16:12 +0100)]
make POPSUB and POPFORMAT re-entrant safe

In code like

    sub DESTROY { exit }
    sub f { push @_, bless [] }
    f()

While leaving f, POPSUB will attempt to clear @_. This triggers the
destructor, which calls exit, which unwinds the context stack, calling
POPSUB again on thr same context stack entry.

Thus we need to to be careful that POPSUB can be called multiple times
on the same CX stack entry.

The general way of doing this is to NULL or update pointers *before*
calling SvREFCNT_dec() on the old value.

This commit also removes the old CxPOPSUB_DONE mechanism which was a poor
bandaid added at the last minute to 5.22 to try and achieve the basics
of what this commit does comprehensively (hopefully). Also, CxPOPSUB_DONE
was mainly a protection against re-entrantly calling LEAVE_SCOPE(). Since
that's now done before POPSUB rather than in the middle of it, it has
become reentrant-safe by default.

5 years agoreorder 'struct block' fields.
David Mitchell [Sat, 17 Oct 2015 14:22:05 +0000 (15:22 +0100)]
reorder 'struct block' fields.

On 64-bit builds, there was a 32-bit hole near the beginning. Shuffle this
towards the middle of the struct (just before the blk_u union).

Unfortunately this (moved) hole can't be filled by having an I32 at the
start of each union member, since struct block_eval is already the largest
and has no 32-bit fields.

Still, moving further on than some of the hotter fields can't hurt.

5 years agomove and rename cx_old_savestack_ix
David Mitchell [Sat, 17 Oct 2015 13:52:50 +0000 (14:52 +0100)]
move and rename cx_old_savestack_ix

Earlier on in this branch I temporarily added cx_old_savestack_ix
as the first element in the context struct. This commit moves it down
32 bits so it now follows type/gimme/u16 in the sbu and blku unions.

This means that type is now back as the first item in the struct.

I've also renamed it blku_oldsaveix to be more in keeping with similar
field names.

5 years agosimplify two conditions in pp_iter:
David Mitchell [Sat, 17 Oct 2015 12:24:30 +0000 (13:24 +0100)]
simplify two conditions in pp_iter:

    -        if (UNLIKELY(SvMAGICAL(av) || AvREIFY(av))) {
    +        if (UNLIKELY(SvRMAGICAL(av))) {

This condition is deciding whether to call av_fetch or cheat and do
sv = AvARRAY(av)[ix].

The magic test is too broad: av_fetch() only handles the specially on
RMG, not all magic.

The AvREIFY condition was originally added by 64138690 in 2001. It was
an attempt to paper over the cracks if function args are deleted, then
iterate over @_, which now contains freed elements. Since pp_iter now
includes a 'freed sv' check itself, there's no need to call into av_fetch
which has a similar check.

This makes a slight functional change - previously if @_ contained a freed
element, av_fetch would silently return NULL; now instead, pp_iter
croaks with "Use of freed value in iteration", the same as it does with
all other cases involving freed elements being iterated over.

5 years agoPUSHLOOP_PLAIN: don't set unused fields
David Mitchell [Sat, 17 Oct 2015 12:15:06 +0000 (13:15 +0100)]
PUSHLOOP_PLAIN: don't set unused fields

Currently PUSHLOOP_PLAIN sets a whole unch of fields that it isn't going
to ue to NULL or 0, e.g. blk_loop.itersave.

Skip this and leave them wild. Make cx_dump() not try to dump them for
CXt_LOOP_PLAIN

5 years agoEliminate cx->blk_loop.resetsp
David Mitchell [Sat, 17 Oct 2015 11:56:15 +0000 (12:56 +0100)]
Eliminate cx->blk_loop.resetsp

Of all the loop types, only CXt_LOOP_LIST, i.e. for (1,2,3) {},
needs to keep anything on the stack, i.e. for all the others,
cx->blk_loop.resetsp can always equal cx->blk_oldsp.
For CXt_LOOP_LIST, the same value as resetsp is stored as
blk_loop.state_u.stack.basesp.

So we can eliminate the resetsp field. This is good as the loop substruct
is the largest (ITHREADS) or joint largest with eval (non-threaded).

Also, we now don't have to store that value for non-CXt_LOOP_LIST loops.
The downside is we now have to choose between basesp and oldsp in
pp_leaveloop and pp_last, based on CX type.

5 years agopp_iter(): optimise stack handling
David Mitchell [Sat, 17 Oct 2015 09:53:10 +0000 (10:53 +0100)]
pp_iter(): optimise stack handling

Make pp_enteriter() do EXTEND(SP,1); then there's no need for pp_iter() to
check for space to push PL_sv_yes/no each time round the loop.

Since the only stack manipulation in pp_iter is now just pushing a boolean
at the end, remove dSP etc and just directly push to PL_stack_sp at the
end.

5 years agosplit CXt_LOOP_FOR into CXt_LOOP_LIST,CXt_LOOP_ARY
David Mitchell [Fri, 16 Oct 2015 23:18:44 +0000 (00:18 +0100)]
split CXt_LOOP_FOR into CXt_LOOP_LIST,CXt_LOOP_ARY

Create a new context type so that "for (1,2,3)" and "for (@ary)"
are now two separate types.

For the list type, we store the index of the base stack element in the
state union rather than having an array pointer. Currently this is just
the same as blk_resetsp, but this will shortly allow us to eliminate the
resetsp field from the struct block_loop - which is currently the largest
sub-struct within the block union.

Having two separate types also allows the two cases to be handled directly
in the main switch in the hot pp_iter code, rather than having extra
conditionals.

5 years agobenchmarks: add some 'for' array iterating
David Mitchell [Fri, 16 Oct 2015 15:20:47 +0000 (16:20 +0100)]
benchmarks: add some 'for' array iterating

5 years agostop S_undo_inc_then_croak() doing CX_POP
David Mitchell [Fri, 16 Oct 2015 13:32:26 +0000 (14:32 +0100)]
stop S_undo_inc_then_croak() doing CX_POP

I added this static function several commits commits ago but in hindsight
I don't like that a cx is passed to it and it does a CX_POP(cx). This is
too much like action at a distance. Instead make the caller save
cx->blk_eval.old_namesv and do the CX_POP itself, then pass namesv as an
arg.

5 years agopp_leaveeval: reset stack in VOID context
David Mitchell [Fri, 16 Oct 2015 12:31:57 +0000 (13:31 +0100)]
pp_leaveeval: reset stack in VOID context

    $ perl -Dst -e'eval"1"'

Gives:
    ....
    ((eval 1):1) leaveeval
        =>  (FREED)

Change it so that like all the other pp_leavefoo() functions, it does

    if (gimme == G_VOID)
        PL_stack_sp = newsp;

I can't think of any (non-debugging) perl-level badness the old behaviour
can be shown to demonstrate, but best not to have freed values left
dangling.

This also allows pp_leaveeval() to (like the other pp_leavefoo functions)
avoid doing a dSP with the associated unnecessary PUTBACKs and SPAGAINs.

Finally, the code to detect a false require is moved to earlier in the
function where it's in the same place as the rest of the stack arg
processing code.

5 years agorename S_doeval() to S_doeval_compile()
David Mitchell [Fri, 16 Oct 2015 12:07:23 +0000 (13:07 +0100)]
rename S_doeval() to S_doeval_compile()

This makes it a bit more obvious what niche in the "eval" ecosystem
that it occupies.

5 years agoS_doeval(): tidy up comments
David Mitchell [Fri, 16 Oct 2015 11:19:44 +0000 (12:19 +0100)]
S_doeval(): tidy up comments

Also, replace

    if (...) {
        ...
        return;
    }
    else
        B

with

    if (...) {
        ...
        return;
    }
    B

5 years agoremove obsolete panic from die_unwind()
David Mitchell [Fri, 16 Oct 2015 11:00:46 +0000 (12:00 +0100)]
remove obsolete panic from die_unwind()

It panics if the context it has just popped back to isn't a CXt_EVAL.
Since we have just called dopoptoeval() which can only pop to an eval, and
since we assert that its an eval, this seems such an unlikely "this can
can never happen" that it doesn't really seem worth testing for.

It was added in perl5.000, and may have made slightly more sense then, as
there used to be a POPBLOCK just before it.

5 years agopp_return: avoid potential CX stack realloc prob
David Mitchell [Fri, 16 Oct 2015 10:33:43 +0000 (11:33 +0100)]
pp_return: avoid potential CX stack realloc prob

5 years agoPerl_die_unwind(): remove unneeded assert
David Mitchell [Fri, 16 Oct 2015 10:25:42 +0000 (11:25 +0100)]
Perl_die_unwind(): remove unneeded assert

Now that the order of things has changed, PL_curcop will always have just
been restored at this point. So there's no longer any needs to assert that
its the correct value.

5 years agoadd S_undo_inc_then_croak()
David Mitchell [Thu, 15 Oct 2015 23:03:00 +0000 (00:03 +0100)]
add S_undo_inc_then_croak()

Consolidate into a single static function, code in 3 functions that all
'undo' an added %INC entry on failure, then croak.