[PATCH] Use of freed comppad array during clear_yystack()

I tried to make tests pass on a perl built with -DPERL_POISON,
as some tests were dying with segfaults. They all originated
from the same source: clear_yystack() after a compile error,
for example:

  (gdb) r -e'sub foo { my $a; { local $a = 0 } }'
  Starting program: /home/mhx/src/perl/dist/rsync/perl-current/perl -e'sub foo { my $a; { local $a = 0 } }'
  Can't localize lexical variable $a at -e line 1.

  Program received signal SIGSEGV, Segmentation fault.
  0x08088cd1 in Perl_pad_free (po=1) at pad.c:1305
  1305            SvPADTMP_off(PL_curpad[po]);
  (gdb) bt
  #0  0x08088cd1 in Perl_pad_free (po=1) at pad.c:1305
  #1  0x08063406 in Perl_op_clear (o=0x83b2bf8) at op.c:669
  #2  0x08063044 in Perl_op_free (o=0x83b2bf8) at op.c:515
  #3  0x08062fe2 in Perl_op_free (o=0x83b2c70) at op.c:495
  #4  0x082ec8e1 in S_clear_yystack (parser=0x83b1920) at perly.c:305
  #5  0x081bed64 in Perl_leave_scope (base=0) at scope.c:911
  #6  0x081157aa in S_my_exit_jump () at perl.c:5306
  #7  0x081153f9 in Perl_my_failure_exit () at perl.c:5291
  #8  0x080cfa5e in Perl_vcroak (pat=0x82f128c "Can't localize lexical variable %s", args=0xbff0a0a4) at util.c:1426
  #9  0x080cfa7c in Perl_croak (pat=0x82f128c "Can't localize lexical variable %s") at util.c:1467
  #10 0x08065e23 in Perl_mod (o=0x83b2c90, type=0) at op.c:1586
  #11 0x08068a30 in Perl_localize (o=0x83b2c90, lex=0) at op.c:2416
  #12 0x082eee29 in Perl_yyparse () at perly.y:1085
  #13 0x08109103 in S_parse_body (env=0x0, xsinit=0x80627a2 <xs_init>) at perl.c:2238
  #14 0x081077d4 in perl_parse (my_perl=0x839a008, xsinit=0x80627a2 <xs_init>, argc=2, argv=0xbff0a4e4, env=0x0) at perl.c:1662
  #15 0x0806275f in main (argc=2, argv=0xbff0a4e4, env=0xbff0a4f0) at perlmain.c:111

What happened? As far as I can see, after croaking the newly
created CV is destroyed and its pad is undef'd:

  #0  Perl_pad_undef (cv=0x83ae278) at pad.c:254
  #1  0x08073217 in Perl_cv_undef (cv=0x83ae278) at op.c:5319
  #2  0x0815d2b1 in Perl_sv_clear (sv=0x83ae278) at sv.c:5510
  #3  0x0815e20a in Perl_sv_free2 (sv=0x83ae278) at sv.c:5714
  #4  0x081be56e in Perl_leave_scope (base=0) at scope.c:830
  #5  0x081157aa in S_my_exit_jump () at perl.c:5306
  #6  0x081153f9 in Perl_my_failure_exit () at perl.c:5291
  #7  0x080cfa5e in Perl_vcroak (pat=0x82f128c "Can't localize lexical variable %s", args=0xbfad3474) at util.c:1426
  #8  0x080cfa7c in Perl_croak (pat=0x82f128c "Can't localize lexical variable %s") at util.c:1467
  #9  0x08065e23 in Perl_mod (o=0x83b2c90, type=0) at op.c:1586
  #10 0x08068a30 in Perl_localize (o=0x83b2c90, lex=0) at op.c:2416
  #11 0x082eee29 in Perl_yyparse () at perly.y:1085
  #12 0x08109103 in S_parse_body (env=0x0, xsinit=0x80627a2 <xs_init>) at perl.c:2238
  #13 0x081077d4 in perl_parse (my_perl=0x839a008, xsinit=0x80627a2 <xs_init>, argc=2, argv=0xbfad38b4, env=0x0) at perl.c:1662
  #14 0x0806275f in main (argc=2, argv=0xbfad38b4, env=0xbfad38c0) at perlmain.c:111

This will SvREFCNT_dec PL_comppad and set PL_comppad to NULL.
However, later, in clear_yystack(), when the ops are freed, the
old PL_comppad is restored by PAD_RESTORE_LOCAL, as a reference
is still in ps->comppad. But now the pad AV is already dead.

Normally (i.e. without PERL_POISON), the dead AV will have
AvARRAY(av) set to NULL by av_undef(). So PAD_RESTORE_LOCAL will
actually set PL_curpad to NULL, and thus pad_free() will not
attempt to do anything.

But with PERL_POISON, the storage for AvARRAY(av) (i.e. sv_u)
will be reused for chaining the free SV heads in the arena
(as opposed to SvANY(sv) in case of !PERL_POISON). This means
that PAD_RESTORE_LOCAL will find AvARRAY(av) non-NULL and will
set PL_curpad to that value, finally causing the segfault in
pad_free().

While I think I understand what's going on, I don't have the
slightest clue how to properly fix this. Given that it's not
a problem only under PERL_POISON, but always (as dead SV heads
are being used), I think it should ultimately be fixed.

The only thing I can offer right now is a patch to make it
work with PERL_POISON as good (or as bad) as without by
making PAD_RESTORE_LOCAL explicitly check if the pad passed
in is already dead and refusing to use it if it is:

--- pad.h.orig	2008-10-25 13:33:30.000000000 +0200
+++ pad.h	2008-10-26 02:32:30.000000000 +0100
@@ -239,7 +239,7 @@
 	      PTR2UV(PL_comppad), PTR2UV(PL_curpad)));
 
 #define PAD_RESTORE_LOCAL(opad) \
-	PL_comppad = opad;					\
+	PL_comppad = opad && SvIS_FREED(opad) ? NULL : opad;	\
 	PL_curpad =  PL_comppad ? AvARRAY(PL_comppad) : NULL;	\
 	DEBUG_Xv(PerlIO_printf(Perl_debug_log,			\
 	      "Pad 0x%"UVxf"[0x%"UVxf"] restore_local\n",	\

With this patch, all tests pass with -DPERL_POISON, and I
haven't seen any segfaults.

Marcus

-- 
 Leela: Bender, maybe you can interface with the Femputer and 
   reprogram it to let them go. 
 Bender: Maybe you can interface with my ass... by biting it.

• [PATCH] Use of freed comppad array during clear_yystack() by Marcus Holland-Moritz
• Re: [PATCH] Use of freed comppad array during clear_yystack() by Rafael Garcia-Suarez