This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
pp_leavesub: reset SP in void context
authorDavid Mitchell <davem@iabyn.com>
Sun, 8 Nov 2015 15:05:01 +0000 (15:05 +0000)
committerDavid Mitchell <davem@iabyn.com>
Wed, 3 Feb 2016 09:18:32 +0000 (09:18 +0000)
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.

ext/XS-APItest/t/call.t
pp_hot.c

index c474639..15b0965 100644 (file)
@@ -56,8 +56,8 @@ sub Foo::d {
 
 for my $test (
     # flags      args           expected         description
-    [ G_VOID,    [ ],           [ qw(z 1) ],     '0 args, G_VOID' ],
-    [ G_VOID,    [ qw(a p q) ], [ qw(z 1) ],     '3 args, G_VOID' ],
+    [ G_VOID,    [ ],           [ 0 ],           '0 args, G_VOID' ],
+    [ G_VOID,    [ qw(a p q) ], [ 0 ],           '3 args, G_VOID' ],
     [ G_SCALAR,  [ ],           [ qw(y 1) ],     '0 args, G_SCALAR' ],
     [ G_SCALAR,  [ qw(a p q) ], [ qw(y 1) ],     '3 args, G_SCALAR' ],
     [ G_ARRAY,   [ ],           [ qw(x 1) ],     '0 args, G_ARRAY' ],
@@ -81,9 +81,7 @@ for my $test (
        "$description call_pv('f')");
 
     ok(eq_array( [ eval_sv('f(' . join(',',map"'$_'",@$args) . ')', $flags) ],
-                 $flags == G_VOID ? [ 0 ] : $expected
-               ),
-        "$description eval_sv('f(args)')");
+        $expected), "$description eval_sv('f(args)')");
 
     ok(eq_array( [ call_method('meth', $flags, $obj, @$args) ], $expected),
        "$description call_method('meth')");
@@ -137,9 +135,7 @@ for my $test (
        $expected), "$description G_NOARGS call_pv('f')");
 
     ok(eq_array( [ sub { eval_sv('f(@_)', $flags|G_NOARGS) }->(@$args) ],
-                  $flags == G_VOID ? [ 0 ] :  $expected
-               ),
-        "$description G_NOARGS eval_sv('f(@_)')");
+        $expected), "$description G_NOARGS eval_sv('f(@_)')");
 
     # XXX call_method(G_NOARGS) isn't tested: I'm assuming
     # it's not a sensible combination. DAPM.
index 7a5ad78..e212997 100644 (file)
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -3351,6 +3351,11 @@ PP(pp_leavesub)
            }
        }
     }
+    else {
+        /* G_VOID */
+        SP = newsp;
+    }
+
     PUTBACK;
 
     CX_LEAVE_SCOPE(cx);