develooper Front page | perl.perl5.porters | Postings from July 2008

Re: Change 34138: The assert()ions in sv_chop() that the passed in pointer is within the

From:
Jerry D. Hedden
Date:
July 14, 2008 05:57
Subject:
Re: Change 34138: The assert()ions in sv_chop() that the passed in pointer is within the
Message ID:
1ff86f510807140557u4dbe4c94l225e09da8cd4ff69@mail.gmail.com
This patch produces the following warning:

sv.c: In function `Perl_sv_chop':
sv.c:4392: warning: 'max_delta' might be used uninitialized in this function

The added panic uses max_delta which hasn't been set until two lines
further down.  I don't know what the proper fix should be.

> Change 34138 by nicholas@nicholas-saigo on 2008/07/13 20:22:25
>
>        The assert()ions in sv_chop() that the passed in pointer is within the
>        SV's buffer should be full-on panics, as bogus values passed in can
>        cause later heap corruption, which is a bad thing (TM).
>
> Affected files ...
>
> ... //depot/perl/pod/perldiag.pod#497 edit
> ... //depot/perl/sv.c#1547 edit
>
> Differences ...
>
> ==== //depot/perl/pod/perldiag.pod#497 (text) ====
> Index: perl/pod/perldiag.pod
> --- perl/pod/perldiag.pod#496~34022~    2008-06-08 02:02:12.000000000 -0700
> +++ perl/pod/perldiag.pod       2008-07-13 13:22:25.000000000 -0700
> @@ -3189,6 +3189,11 @@
>
>  (P) scan_num() got called on something that wasn't a number.
>
> +=item panic: sv_chop %s
> +
> +(P) The sv_chop() routine was passed a position that is not within the
> +scalar's string buffer.
> +
>  =item panic: sv_insert
>
>  (P) The sv_insert() routine was told to remove more string than there
>
> ==== //depot/perl/sv.c#1547 (text) ====
> Index: perl/sv.c
> --- perl/sv.c#1546~34136~       2008-07-12 21:04:31.000000000 -0700
> +++ perl/sv.c   2008-07-13 13:22:25.000000000 -0700
> @@ -4389,6 +4389,7 @@
>  #ifdef DEBUGGING
>     const U8 *real_start;
>  #endif
> +    STRLEN max_delta;
>
>     PERL_ARGS_ASSERT_SV_CHOP;
>
> @@ -4399,12 +4400,17 @@
>        /* Nothing to do.  */
>        return;
>     }
> -    assert(ptr > SvPVX_const(sv));
> +    /* SvPVX(sv) may move in SV_CHECK_THINKFIRST(sv), but after this line,
> +       nothing uses the value of ptr any more.  */
> +    if (ptr <= SvPVX_const(sv))
> +       Perl_croak(aTHX_ "panic: sv_chop ptr=%p, start=%p, end=%p",
> +                  ptr, SvPVX_const(sv), SvPVX_const(sv) + max_delta);
>     SV_CHECK_THINKFIRST(sv);
> -    if (SvLEN(sv))
> -       assert(delta <= SvLEN(sv));
> -    else
> -       assert(delta <= SvCUR(sv));
> +    max_delta = SvLEN(sv) ? SvLEN(sv) : SvCUR(sv);
> +    if (delta > max_delta)
> +       Perl_croak(aTHX_ "panic: sv_chop ptr=%p (was %p), start=%p, end=%p",
> +                  SvPVX_const(sv) + delta, ptr, SvPVX_const(sv),
> +                  SvPVX_const(sv) + max_delta);
>
>     if (!SvOOK(sv)) {
>        if (!SvLEN(sv)) { /* make copy of shared string */
> End of Patch.
>



nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About