Branch: refs/heads/blead
  Home:   https://github.com/Perl/perl5
  Commit: 6d1334c5cd9c5a8150747d54de714d03a6141f38
      
https://github.com/Perl/perl5/commit/6d1334c5cd9c5a8150747d54de714d03a6141f38
  Author: David Mitchell <da...@iabyn.com>
  Date:   2024-01-03 (Wed, 03 Jan 2024)

  Changed paths:
    M pp_sort.c

  Log Message:
  -----------
  pp_sort: fix leak in PERL_RC_STACK inline sorting

For the optimised case where the src and dst are both the same array,
e.g.
    @a = sort { ... } @a;

pp_sort() optimises this. When the code was modified to run under
PERL_RC_STACK, I introduced a leak: all the SVs on the stack after
sorting were then stored in the array and their ref counts incremented,
then the stack pointer was reset *without* decrementing the ref count of
each SV. So every SV in the array by the time pp_sort() returned had a
reference count one too high.

The fix is trivial - don't bump the ref counts when storing them in
the array.


  Commit: 9c4b36214a66fdba52027f881d6619e1f9a0d15a
      
https://github.com/Perl/perl5/commit/9c4b36214a66fdba52027f881d6619e1f9a0d15a
  Author: David Mitchell <da...@iabyn.com>
  Date:   2024-01-03 (Wed, 03 Jan 2024)

  Changed paths:
    M op.c
    M t/op/svleak.t

  Log Message:
  -----------
  fix leak in list const folding under PERL_RC_STACK

S_gen_constant_list() wasn't taking account of the stack possibly being
reference-counted, and so when a list was being constant-folded into an
AV, that AV would leak, such as

    while (1) { my $x = eval '\(1..3)'; }


  Commit: 7184c7cb9d22e1849493d74272451de21703d71c
      
https://github.com/Perl/perl5/commit/7184c7cb9d22e1849493d74272451de21703d71c
  Author: David Mitchell <da...@iabyn.com>
  Date:   2024-01-03 (Wed, 03 Jan 2024)

  Changed paths:
    M pp_ctl.c
    M t/op/svleak.t

  Log Message:
  -----------
  fix obscure leak in sort { block } ...

This only leaked on PERL_RC_STACK builds, and only in the relatively
rare code path of a sort block which included a nested scope (such as
a for loop), and which then used 'return' to return the value.


  Commit: 48cac15890ee29400bff9bb1e0c940997f1505b1
      
https://github.com/Perl/perl5/commit/48cac15890ee29400bff9bb1e0c940997f1505b1
  Author: David Mitchell <da...@iabyn.com>
  Date:   2024-01-03 (Wed, 03 Jan 2024)

  Changed paths:
    M pp_ctl.c

  Log Message:
  -----------
  fix minor leak under use feature 'module_true'

Under PERL_RC_STACK builds, any return value from the module, i.e.
the
    1;
or other final statement value, would leak.


  Commit: 6ca3ba049e69f7b8d67362ce4fac7b812a47ee1f
      
https://github.com/Perl/perl5/commit/6ca3ba049e69f7b8d67362ce4fac7b812a47ee1f
  Author: David Mitchell <da...@iabyn.com>
  Date:   2024-01-03 (Wed, 03 Jan 2024)

  Changed paths:
    M sv.c

  Log Message:
  -----------
  Resurrect immortals before checking for SvTEMP()

sv_clear() and sv_free2() both do, in this order, (simplified):

    #ifdef DEBUGGING
        if (SvTEMP(sv))
            Perl_ck_warner_d(..., Attempt to free temp prematurely",...);
    #endif

    if (SvIMMORTAL(sv))
        SvREFCNT(sv) = SvREFCNT_IMMORTAL

Now, it so happens that under DEBUGGING PERL_RC_STACK builds,

a) immortals such s PL_sv_undef have their refcount set to only 10 to
deliberately trigger the edge case of them being freed more often;

b) PERL_RC_STACK builds increasingly don't bother to increment the
reference counts of immortals when pushing them on the stack - this
saves a bit of time, and just means that once every two billion times on
normal builds the ref count drops to zero and sv_clear() sets it back to
SvREFCNT_IMMORTAL.

The combination of these has suddenly made it much more likely that
an immortal on the stack which has also been mortalised, will be passed
to sv_clear() and thus spuriously output the warning message.

So this commit swaps the order of the checks.

In the SvIMMORTAL() branch, it now also turns off SvTEMP.


  Commit: 7f04cfc05d1d08fcba13a426ea4da76216c707cf
      
https://github.com/Perl/perl5/commit/7f04cfc05d1d08fcba13a426ea4da76216c707cf
  Author: David Mitchell <da...@iabyn.com>
  Date:   2024-01-03 (Wed, 03 Jan 2024)

  Changed paths:
    M embed.fnc
    M embed.h
    M inline.h
    M pod/perlguts.pod
    M proto.h

  Log Message:
  -----------
  add _IMM variants to the rpp_foo() fns

These new function variants assume that the item being put on the stack
is one of the immortals (PL_sv_undef/yes/no/zero), and so skips
incrementing their reference count. This is for a minor efficiency
saving, rather than being necessary for correct functioning of the code.

This commit also tidies up a few of the related rpp_ functions: in
particular moving asserts out of the PERL_RC_STACK-only code into the
general code: an rpp_foo_NN() function should assert fail on a null SV
regardless of whether perl has been compiled under PERL_RC_STACK or not.


  Commit: b6f2485657db9591ba249dc917047dbd11b373cf
      
https://github.com/Perl/perl5/commit/b6f2485657db9591ba249dc917047dbd11b373cf
  Author: David Mitchell <da...@iabyn.com>
  Date:   2024-01-03 (Wed, 03 Jan 2024)

  Changed paths:
    M pp.c
    M pp_ctl.c
    M pp_hot.c
    M pp_sort.c
    M pp_sys.c

  Log Message:
  -----------
  use rpp_foo_NN() and rpp_foo_IMM() widely

Make more use of the recently-added _NN and _IMM_NN variants of common
functions throughout the pp*.c files. The _NN ones assume anything being
popped of the stack is non-NULL, so that check can be skipped for each
SV being popped. The _IMM variants mean that the one item being put on
the stack is an immortal like &PL_sv_undef, so doesn't need its
reference count adjusting.

So these are all just small optimisations.


  Commit: 8d0d5d78ace202493b29b6240d3b1dd63ed3957b
      
https://github.com/Perl/perl5/commit/8d0d5d78ace202493b29b6240d3b1dd63ed3957b
  Author: David Mitchell <da...@iabyn.com>
  Date:   2024-01-03 (Wed, 03 Jan 2024)

  Changed paths:
    M pp.c
    M pp_sys.c

  Log Message:
  -----------
  make RC-stack-aware: unwrap pp_prtf, pp_sprintf

Remove the temporary wrappers from pp_prtf() (which implements the perl
'printf' function but saves two whole letters!) and pp_sprintf.


  Commit: 6fedd5b15734714f92d60d58570113359b986470
      
https://github.com/Perl/perl5/commit/6fedd5b15734714f92d60d58570113359b986470
  Author: David Mitchell <da...@iabyn.com>
  Date:   2024-01-03 (Wed, 03 Jan 2024)

  Changed paths:
    M pp_ctl.c
    M pp_hot.c

  Log Message:
  -----------
  make RC-stack-aware: unwrap pp_subst, pp_substcont

Remove the temporary wrappers from pp_subst() and pp_substcont().

Note that under s///e, any arguments that were on the stack on entry
to pp_subst() are now left on there until the final call to pp_substcont,
who's responsibility it is now to pop them. This is so that they don't
get prematurely freed on PERL_RC_STACK builds.


  Commit: be80f5674fa2e558706681232d33b0082159e8fe
      
https://github.com/Perl/perl5/commit/be80f5674fa2e558706681232d33b0082159e8fe
  Author: David Mitchell <da...@iabyn.com>
  Date:   2024-01-03 (Wed, 03 Jan 2024)

  Changed paths:
    M pp_hot.c

  Log Message:
  -----------
  pp_subst(): consolidate some duplicated code

There are a couple of places in pp_subst() which create a new mortal to
return the iteration count. Consolidate them into a single code block at
the end of the function.


  Commit: 38f33f60a7c6374744c21cdd74c844ca09291749
      
https://github.com/Perl/perl5/commit/38f33f60a7c6374744c21cdd74c844ca09291749
  Author: David Mitchell <da...@iabyn.com>
  Date:   2024-01-03 (Wed, 03 Jan 2024)

  Changed paths:
    M pp.c

  Log Message:
  -----------
  make RC-stack-aware: unwrap pp_bless()

Remove the temporary wrapper from pp_bless().


  Commit: a4d3204d9e74bccac5de11f1373a45091ef5cb0f
      
https://github.com/Perl/perl5/commit/a4d3204d9e74bccac5de11f1373a45091ef5cb0f
  Author: David Mitchell <da...@iabyn.com>
  Date:   2024-01-03 (Wed, 03 Jan 2024)

  Changed paths:
    M embed.fnc
    M embed.h
    M inline.h
    M pp_hot.c
    M proto.h

  Log Message:
  -----------
  Optimise rpp_replace_2_{1,IMM}_NN()

These two static functions are used in a lot of pp functions.
This commit does two main things. First, it makes the size of the inline
function smaller, and second, it uses a single branch (rather than two)
to decide whether either of the two SVs being popped need to be freed.

In detail: apart from the actual stack manipulation itself, the other
main action of these two functions:

    rpp_replace_2_1_NN()
    rpp_replace_2_IMM_NN()

is to do the equivalent of

    SvREFCNT_dec_NN(PL_stack[-1]);
    SvREFCNT_dec_NN(PL_stack[-0]);

Now, SvREFCNT_dec_NN() is an inline function which expands to
something like:

    U32 rc = SvREFCNT(sv);
    if (LIKELY(rc > 1))
        SvREFCNT(sv) = rc - 1;
    else
        Perl_sv_free2(aTHX_ sv, rc);

With this expanded *twice* within the body of rpp_replace_2_1_NN(),
there are two branch tests and two function calls - all of which are
expanded inline into the bodies of all 50+ pp functions which use it.
This commit makes this be changed to something equivalent to

    U32 rc1 = SvREFCNT(sv1);
    U32 rc2 = SvREFCNT(sv2);
    if (LIKELY(rc1 > 1 && rc2 > 1)) {
        SvREFCNT(sv1) = rc1 - 1;
        SvREFCNT(sv2) = rc2 - 1;
    }
    else
        Perl_rpp_free_2_(aTHX_ sv1, sv2, rc1, rc2);

Where Perl_rpp_free_2_() does the hard work of deciding whether either
or both SVs actually need freeing.

This approach assumes that, most of the time, rpp_replace_2_1_NN() won't
actually be freeing either of the two old args on the stack, because
often they are likely to be PADTMPs or lexicals or array elements or
or immortals or whatever, which have a longer lifetime. I.e.  this
commit is betting that

    $a + ($b * $c);   # RHS of '+' is a PADTMP

is more common than

    $a + f();         # RHS of '+' is a temporary SV with RC==1


  Commit: 63fe8426df927c85284654e825ab062355f699e6
      
https://github.com/Perl/perl5/commit/63fe8426df927c85284654e825ab062355f699e6
  Author: David Mitchell <da...@iabyn.com>
  Date:   2024-01-03 (Wed, 03 Jan 2024)

  Changed paths:
    M inline.h
    M pp_hot.c

  Log Message:
  -----------
  make rpp_popfree_2_NN() use rpp_free_2_()

Like for Perl_rpp_replace_2_1*, this means that freeing one or both SVs
being popped is done by a single function call.


  Commit: 4b9ded7af299053465268884a78c5dad32aedc8b
      
https://github.com/Perl/perl5/commit/4b9ded7af299053465268884a78c5dad32aedc8b
  Author: David Mitchell <da...@iabyn.com>
  Date:   2024-01-03 (Wed, 03 Jan 2024)

  Changed paths:
    M embed.fnc
    M embed.h
    M inline.h
    M op.c
    M pod/perlguts.pod
    M pp.c
    M pp_ctl.c
    M pp_hot.c
    M pp_sort.c
    M pp_sys.c
    M proto.h
    M sv.c
    M t/op/svleak.t

  Log Message:
  -----------
  [MERGE] PERL_RC_STACK: add _IMM, unwrap, fix leaks

- unwrap the ops pp_prtf, pp_sprintf, pp_subst, pp_substcont, pp_bless
- fix a few PERL_RC_STACK-related leaks I noticed along the way
- add some _IMM variants of the rpp_ functions when the SV being pushed
  is an immortal
- make wide use of the _IMM and _NN rpp_ function variants
- optimise two-item stack pops and replaces


Compare: https://github.com/Perl/perl5/compare/7f3402777ae7...4b9ded7af299

Reply via email to