develooper Front page | perl.perl5.porters | Postings from December 2018

Re: regexec.c: LGTM analysis warns about two comparisons, but weactually need them

Thread Previous
From:
demerphq
Date:
December 6, 2018 08:33
Subject:
Re: regexec.c: LGTM analysis warns about two comparisons, but weactually need them
Message ID:
CANgJU+WyX52X72Hbc3jLc40RmuVTS8tLQtrh8bKX4iv0CxzYwQ@mail.gmail.com
Just a general observation,  sometimes in the regex engine we use a
value like 256 in vars which look like they only hold an octet as a
flag/sentinel for things like end of string.

It is not the case /here/ but it does happen.

Yves
On Thu, 6 Dec 2018 at 09:17, Tony Cook <tony@develop-help.com> wrote:
>
> On Wed, Dec 05, 2018 at 10:28:54PM -0500, James E Keenan wrote:
> > Here is a case where the LGTM.com analysis of the Perl 5 source code
> > produces results that are at first plausible but ultimately incorrect.
> >
> > The following section of regexec.c is cited (https://lgtm.com/projects/g/Perl/perl5/alerts/?mode=tree&ruleFocus=2154840804)
> > with the warning-level alert "Comparison result is always the same":
>
> This was discussed in #133686, the comparison needs to stay to allow
> for local configuration of larger values of NUM_ANYOF_CODE_POINTS.
>
> > I then created a branch (jkeenan/lgtm-regexec-c; https://perl5.git.perl.org/perl.git/commitdiff/1a6c6e69bfa70bc55eb0d1e85538d7420659a652)
> > in which I simply removed those two comparisons.  Notwithstanding the
> > alleged superfluousness of the two comparisons, I got these failures:
> >
> > #####
> > Test Summary Report
> > -------------------
> > re/regexp_noamp.t                                                (Wstat: 0
> > Tests: 2006 Failed: 4)
> >   Failed tests:  1830-1831, 1838-1839
> > re/regexp.t                                                      (Wstat: 0
> > Tests: 2006 Failed: 4)
> >   Failed tests:  1830-1831, 1838-1839
>
> The tests are failing because of the added
> _CHECK_AND_OUTPUT_WIDE_LOCALE_CP_MSG(c);, from your commit:
>
> diff --git a/regexec.c b/regexec.c
> index eb24567e04..a265cc84cb 100644
> --- a/regexec.c
> +++ b/regexec.c
> @@ -9772,15 +9772,13 @@ S_reginclass(pTHX_ regexp * const prog, const regnode * const n, const U8* const
>             match = TRUE;
>         }
>         else if (flags & ANYOF_LOCALE_FLAGS) {
> +            _CHECK_AND_OUTPUT_WIDE_LOCALE_CP_MSG(c);
>             if ((flags & ANYOFL_FOLD)
> -                && c < 256
>                 && ANYOF_BITMAP_TEST(n, PL_fold_locale[c]))
>              {
>                  match = TRUE;
>
>
> Remove that and they pass.
>
> > So we cannot remove the two comparisons and must somehow inform LGTM that
> > these warnings are spurious.
> >
> > Can anyone explain why these unit tests should preclude removal of the two
> > comparisons?
>
> As was discussed with you in #p5p and mentioned in #133686 these
> comparisons are needed to alloww for local configuration of
> NUM_ANYOF_CODE_POINTS.
>
> Since NUM_ANYOF_CODE_POINTS is 256 (1<<8) by default LGTM considers
> the comparisons superfluous, but if NUM_ANYOF_CODE_POINTS is defined
> to 512 (or perhaps larger) to cover the Latin characters) the
> comparisons against 256 later in the code would no longer be
> superfluous.
>
> The documented LGTM mechanism to suppress a warning is a "// lgtm"
> comment[1], but since we support C89 we cannot use such comments.
>
> Since LGTM doesn't provide an admin UI like coverity does for marking
> warnings as "working as intended", we just have to live with LGTM
> warning about that code.
>
> Tony
>
> [1] https://lgtm.com/help/lgtm/alert-suppression#c-c++



-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Thread Previous


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