FAQ
I've identified YABL (Yet Another Bloody Leak) in 5.6.1 T2, but I'm
stumped as to how to fix it. The problem is PL_reg_start_tmp. The code
in Perl_save_re_context() does this:

SAVEVPTR(PL_reg_start_tmp); /* from regexec.c */
PL_reg_start_tmp = 0;
SAVEFREEPV(PL_reg_start_tmp);

So it saves the address of and current value of PL_reg_start_tmp on the
stack, NULLs it and then saves the null address on the stack to be
freed. This is of course broken. S_regtry then changes the value of
PL_reg_start_tmp thusly:

if(PL_reg_start_tmp)
Renew(PL_reg_start_tmp, PL_reg_start_tmpl, char*);
else
New(22,PL_reg_start_tmp, PL_reg_start_tmpl, char*);

The net result is that the buffer allocated to PL_reg_start_tmp is never
freed, as the incorrect NULL value has been saved on the stack. What
I'm looking for is a SAVExxx macro that saves the address and value of a
pointer on the stack, and on scope exit frees whatever the pointer is
*currently* poining to before restoring the previous value. All the
SAVExxx variants I have found free the saved value of the pointer
instead of the current value. Does a SAVExxx macro with the behaviour
required exist?

Alan Burlison

Search Discussions

  • Gurusamy Sarathy at Feb 6, 2001 at 10:43 pm

    On Tue, 06 Feb 2001 20:52:54 GMT, Alan Burlison wrote:
    I've identified YABL (Yet Another Bloody Leak) in 5.6.1 T2, but I'm
    stumped as to how to fix it. The problem is PL_reg_start_tmp. The code
    in Perl_save_re_context() does this:

    SAVEVPTR(PL_reg_start_tmp); /* from regexec.c */
    PL_reg_start_tmp = 0;
    SAVEFREEPV(PL_reg_start_tmp);
    I haven't looked closely, but try replacing those three lines with:

    SAVEGENERICPV(PL_reg_start_tmp);


    Sarathy
    gsar@ActiveState.com
  • Gurusamy Sarathy at Feb 6, 2001 at 11:28 pm

    On Tue, 06 Feb 2001 14:43:32 PST, Gurusamy Sarathy wrote:
    On Tue, 06 Feb 2001 20:52:54 GMT, Alan Burlison wrote:
    I've identified YABL (Yet Another Bloody Leak) in 5.6.1 T2, but I'm
    stumped as to how to fix it. The problem is PL_reg_start_tmp. The code
    in Perl_save_re_context() does this:

    SAVEVPTR(PL_reg_start_tmp); /* from regexec.c */
    PL_reg_start_tmp = 0;
    SAVEFREEPV(PL_reg_start_tmp);
    I haven't looked closely, but try replacing those three lines with:

    SAVEGENERICPV(PL_reg_start_tmp);
    You may need to add this line too after that to preserve the semantics:

    PL_reg_start_tmp = 0;


    Sarathy
    gsar@ActiveState.com
  • Alan Burlison at Feb 6, 2001 at 11:37 pm
    Give that man a medal - great minds obviously think alike (although mine
    is just a bit slower :-)

    $ p4 diff regcomp.c
    ==== //depot/maint-5.6/pureperl/regcomp.c#1 -
    /home1/homedir/alanbur/perlforce/pureperl/regcomp.c ====
    *** /tmp/tmp.295817.0 Tue Feb 6 23:33:54 2001
    --- /home1/homedir/alanbur/perlforce/pureperl/regcomp.c Tue Feb 6
    22:05:04 2001
    ***************
    *** 4555,4563 ****
    SAVEVPTR(PL_reglastparen); /* Similarly for
    lastparen. */
    SAVEPPTR(PL_regtill); /* How far we are required to
    go. */
    SAVEI8(PL_regprev); /* char before regbol,
    \n if none */
    ! SAVEVPTR(PL_reg_start_tmp); /* from regexec.c */
    PL_reg_start_tmp = 0;
    - SAVEFREEPV(PL_reg_start_tmp);
    SAVEI32(PL_reg_start_tmpl); /* from regexec.c */
    PL_reg_start_tmpl = 0;
    SAVEVPTR(PL_regdata);
    --- 4555,4562 ----
    SAVEVPTR(PL_reglastparen); /* Similarly for
    lastparen. */
    SAVEPPTR(PL_regtill); /* How far we are required to
    go. */
    SAVEI8(PL_regprev); /* char before regbol,
    \n if none */
    ! SAVEGENERICPV(PL_reg_start_tmp); /* from regexec.c */
    PL_reg_start_tmp = 0;
    SAVEI32(PL_reg_start_tmpl); /* from regexec.c */
    PL_reg_start_tmpl = 0;
    SAVEVPTR(PL_regdata);

    Thanks for the confirmation!

    Alan Burlison
  • Nick at Feb 7, 2001 at 9:08 pm

    Alan Burlison writes:
    Give that man a medal - great minds obviously think alike (although mine
    is just a bit slower :-)

    $ p4 diff regcomp.c
    ! SAVEVPTR(PL_reg_start_tmp); /* from regexec.c */
    PL_reg_start_tmp = 0;
    - SAVEFREEPV(PL_reg_start_tmp);
    ! SAVEGENERICPV(PL_reg_start_tmp); /* from regexec.c */

    Alan Burlison
    Same change made to blead perl as change 8711 - the wrapped p4 diff did not
    apply as a patch so I did it by hand.

    --
    Nick Ing-Simmons
  • Alan Burlison at Feb 7, 2001 at 10:01 pm

    nick@ing-simmons.net wrote:

    Same change made to blead perl as change 8711 - the wrapped p4 diff did not
    apply as a patch so I did it by hand.
    That's not the only leak/heap corruption I have fixed - there are
    several others in the pureperl branch you might want to propagate into
    bleadperl. Before I fixed it Perl_pmtrans was leaking 96% of all the
    memory it allocated.

    Alan Burlison
  • Edward Peschko at Feb 6, 2001 at 11:59 pm
    hey,

    the following is a 'sample' patch that prettifies normal, everyday confess
    output. It turns:

    File /home/edwardp/finalwork2/Webspace/http-apache/tarballs/wwwcount[0-9]*.tar.gz does not exist!
    Dying
    Distrib::Format::new('Distrib::Format', 'tarballs/wwwcount[0-9]*.tar.gz' 'HASH(0x9e03b8)') called at ../../tools/lib/Distrib/Edit.pm line 210
    Distrib::Edit::_makefileobjects('Distrib::Edit=HASH(0x9e03e8)', 'tarballs/wwwcount[0-9]*.tar.gz') called at ../../tools/lib/Distrib/Edit.pm line 16
    Distrib::Edit::new('Distrib::Edit', 'tarballs/wwwcount[0-9]*.tar.gz') called at compiledist.p line 92
    main::_fixwwwcount() called at compiledist.p line 20
    Died at ../../tools/lib/Distrib/Format.pm line 40.

    into -

    File /home/edwardp/finalwork2/Webspace/http-apache/tarballs/wwwcount[0-9]*.tar.gz does not exist!
    Dying
    ----
    Distrib::Format::new --> ../../tools/lib/Distrib/Edit.pm:210
    Distrib::Edit::_makefileobjects --> ../../tools/lib/Distrib/Edit.pm:16
    Distrib::Edit::new --> compiledist.p:92
    main::_fixwwwcount --> compiledist.p:20

    ----
    Died at ../../tools/lib/Distrib/Format.pm line 40.

    Note that this is not a *real* patch - I didn't get my hands dirty and go into
    the Carp::Heavy module, instead its meant as a 'proof of concept' module for
    discussion. Its not as detailed, but is cleaner - sometimes IMO you want the
    messy trace and sometimes the clean one. Ultimately, the really cool thing to
    do would be to be able to customize the display of your stacktraces, something
    like 'sprintf'. Patch follows:
    ----

    *** Carp.pm.old Sun Aug 13 11:34:04 2000
    --- Carp.pm.new Sun Jan 28 00:18:13 2001
    ***************
    *** 11,16 ****
    --- 11,23 ----

    confess - die of errors with stack backtrace

    + neatcluck - same as cluck, without arguments and in prettified format
    + (not exported by default)
    +
    + neatconfess - same as confess, without arguments and in prettified format
    + (not exported by default)
    +
    +
    =head1 SYNOPSIS

    use Carp;
    ***************
    *** 71,77 ****
    require Exporter;
    @ISA = ('Exporter');
    @EXPORT = qw(confess croak carp);
    ! @EXPORT_OK = qw(cluck verbose neatcluck neatconfess);
    @EXPORT_FAIL = qw(verbose); # hook to enable verbose mode


    --- 78,84 ----
    require Exporter;
    @ISA = ('Exporter');
    @EXPORT = qw(confess croak carp);
    ! @EXPORT_OK = qw(cluck verbose neatcluck neatconfess);
    @EXPORT_FAIL = qw(verbose); # hook to enable verbose mode


    ***************
    *** 110,124 ****
    goto &shortmess_heavy;
    }


    ! # the following four functions call longmess() or shortmess() depending on
    # whether they should generate a full stack trace (confess() and cluck())
    # or simply report the caller's package (croak() and carp()), respectively.
    # confess() and croak() die, carp() and cluck() warn.

    ! sub croak { die shortmess @_ }
    ! sub confess { die longmess @_ }
    ! sub carp { warn shortmess @_ }
    ! sub cluck { warn longmess @_ }

    1;
    --- 117,158 ----
    goto &shortmess_heavy;
    }

    + sub _clean
    + {
    + my ($text) = @_;
    + $text = "\n----\n" . $text . "\n----\n";
    +
    + my $length;
    + while ($text =~
    + m"\n\t(\S+?)\(.*\)\scalled\sat\s(\S+)\sline\s(\d+)(?=\n)"g)
    + {
    + $length = (length($1) > $length)? length($1) : $length;
    + }
    +
    + my $xx = 0;
    + $text=~s/
    + \n\t(\S+?)\(.*?\)\scalled\sat\s(\S+)\sline\s(\d+)(?=\n)
    + /
    + ($xx++ == 0)? "\n----" .
    + "\n\t".$1." "x($length - length($1)+1) . "--> $2:$3" :
    + "\n\t".$1." "x($length - length($1)+1) . "--> $2:$3"
    + /gexs;

    ! # $text =~ s".*?Carp::neat[^\n]+""sg;
    ! return($text);
    ! }
    !
    ! # the following six functions call longmess() or shortmess() depending on
    # whether they should generate a full stack trace (confess() and cluck())
    # or simply report the caller's package (croak() and carp()), respectively.
    # confess() and croak() die, carp() and cluck() warn.

    ! sub croak { die shortmess @_ }
    ! sub confess { die longmess @_ }
    ! sub carp { warn shortmess @_ }
    ! sub cluck { warn longmess @_ }
    !
    ! sub neatconfess { die _clean(longmess @_) }
    ! sub neatcluck { warn _clean(longmess @_) }

    1;
  • Tim Bunce at Feb 7, 2001 at 11:30 am
    Death is rarely pretty, and forensic evidence often essential.

    I'd rather not bloat Carp with these, it's too widely used.
    $SIG{__WARN__/__DIE__} hooks or a module on CPAN would be fine.

    Tim.
    On Tue, Feb 06, 2001 at 03:59:15PM -0800, Edward Peschko wrote:

    hey,

    the following is a 'sample' patch that prettifies normal, everyday confess
    output. It turns:

    File /home/edwardp/finalwork2/Webspace/http-apache/tarballs/wwwcount[0-9]*.tar.gz does not exist!
    Dying
    Distrib::Format::new('Distrib::Format', 'tarballs/wwwcount[0-9]*.tar.gz' 'HASH(0x9e03b8)') called at ../../tools/lib/Distrib/Edit.pm line 210
    Distrib::Edit::_makefileobjects('Distrib::Edit=HASH(0x9e03e8)', 'tarballs/wwwcount[0-9]*.tar.gz') called at ../../tools/lib/Distrib/Edit.pm line 16
    Distrib::Edit::new('Distrib::Edit', 'tarballs/wwwcount[0-9]*.tar.gz') called at compiledist.p line 92
    main::_fixwwwcount() called at compiledist.p line 20
    Died at ../../tools/lib/Distrib/Format.pm line 40.

    into -

    File /home/edwardp/finalwork2/Webspace/http-apache/tarballs/wwwcount[0-9]*.tar.gz does not exist!
    Dying
    ----
    Distrib::Format::new --> ../../tools/lib/Distrib/Edit.pm:210
    Distrib::Edit::_makefileobjects --> ../../tools/lib/Distrib/Edit.pm:16
    Distrib::Edit::new --> compiledist.p:92
    main::_fixwwwcount --> compiledist.p:20

    ----
    Died at ../../tools/lib/Distrib/Format.pm line 40.

    Note that this is not a *real* patch - I didn't get my hands dirty and go into
    the Carp::Heavy module, instead its meant as a 'proof of concept' module for
    discussion. Its not as detailed, but is cleaner - sometimes IMO you want the
    messy trace and sometimes the clean one. Ultimately, the really cool thing to
    do would be to be able to customize the display of your stacktraces, something
    like 'sprintf'. Patch follows:
    ----

    *** Carp.pm.old Sun Aug 13 11:34:04 2000
    --- Carp.pm.new Sun Jan 28 00:18:13 2001
    ***************
    *** 11,16 ****
    --- 11,23 ----

    confess - die of errors with stack backtrace

    + neatcluck - same as cluck, without arguments and in prettified format
    + (not exported by default)
    +
    + neatconfess - same as confess, without arguments and in prettified format
    + (not exported by default)
    +
    +
    =head1 SYNOPSIS

    use Carp;
    ***************
    *** 71,77 ****
    require Exporter;
    @ISA = ('Exporter');
    @EXPORT = qw(confess croak carp);
    ! @EXPORT_OK = qw(cluck verbose neatcluck neatconfess);
    @EXPORT_FAIL = qw(verbose); # hook to enable verbose mode


    --- 78,84 ----
    require Exporter;
    @ISA = ('Exporter');
    @EXPORT = qw(confess croak carp);
    ! @EXPORT_OK = qw(cluck verbose neatcluck neatconfess);
    @EXPORT_FAIL = qw(verbose); # hook to enable verbose mode


    ***************
    *** 110,124 ****
    goto &shortmess_heavy;
    }


    ! # the following four functions call longmess() or shortmess() depending on
    # whether they should generate a full stack trace (confess() and cluck())
    # or simply report the caller's package (croak() and carp()), respectively.
    # confess() and croak() die, carp() and cluck() warn.

    ! sub croak { die shortmess @_ }
    ! sub confess { die longmess @_ }
    ! sub carp { warn shortmess @_ }
    ! sub cluck { warn longmess @_ }

    1;
    --- 117,158 ----
    goto &shortmess_heavy;
    }

    + sub _clean
    + {
    + my ($text) = @_;
    + $text = "\n----\n" . $text . "\n----\n";
    +
    + my $length;
    + while ($text =~
    + m"\n\t(\S+?)\(.*\)\scalled\sat\s(\S+)\sline\s(\d+)(?=\n)"g)
    + {
    + $length = (length($1) > $length)? length($1) : $length;
    + }
    +
    + my $xx = 0;
    + $text=~s/
    + \n\t(\S+?)\(.*?\)\scalled\sat\s(\S+)\sline\s(\d+)(?=\n)
    + /
    + ($xx++ == 0)? "\n----" .
    + "\n\t".$1." "x($length - length($1)+1) . "--> $2:$3" :
    + "\n\t".$1." "x($length - length($1)+1) . "--> $2:$3"
    + /gexs;

    ! # $text =~ s".*?Carp::neat[^\n]+""sg;
    ! return($text);
    ! }
    !
    ! # the following six functions call longmess() or shortmess() depending on
    # whether they should generate a full stack trace (confess() and cluck())
    # or simply report the caller's package (croak() and carp()), respectively.
    # confess() and croak() die, carp() and cluck() warn.

    ! sub croak { die shortmess @_ }
    ! sub confess { die longmess @_ }
    ! sub carp { warn shortmess @_ }
    ! sub cluck { warn longmess @_ }
    !
    ! sub neatconfess { die _clean(longmess @_) }
    ! sub neatcluck { warn _clean(longmess @_) }

    1;
  • Edward Peschko at Feb 8, 2001 at 7:13 pm
    this is a repeat since it didn't seem to get to perl5-porters.. I apologize if
    already seen.

    ----
    On Wed, Feb 07, 2001 at 11:29:26AM +0000, Tim Bunce wrote:
    Death is rarely pretty, and forensic evidence often essential.
    How about prettifying the forensic output? As it is, I catch 95% of my errors
    faster, just from the fact that the output is cleaner and easier to trace
    through.
    I'd rather not bloat Carp with these, it's too widely used.
    $SIG{__WARN__/__DIE__} hooks or a module on CPAN would be fine.
    Well, how about a rewrite of Carp as an .xs extension? Right now, carp, cluck,
    et all are 'one size fits all' type of modules - if rewritten in C, you could
    probably 'squeeze in' the extra functionality required to generalize them and
    maybe even save a couple of CPU cycles in the process.

    Ed

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupperl5-porters @
categoriesperl
postedFeb 6, '01 at 8:53p
activeFeb 8, '01 at 7:13p
posts9
users5
websiteperl.org

People

Translate

site design / logo © 2022 Grokbase