Re: [Patch, Fortran] Fix some issues found by Coverity's static-code analysis scan

[Tobias Burnus <burnus at net-b dot de>]

Hi Mikael,

thanks for your comments.

As a pre-script: Will you look at Paul's revised assignment patch - or 
should I do it?


Mikael Morin wrote:
> > [Out of bounds]
> > diff --git a/gcc/fortran/array.c b/gcc/fortran/array.c
> > index 44ec72e..1611c3b 100644
> > --- a/gcc/fortran/array.c
> > +++ b/gcc/fortran/array.c
> > @@ -253,7 +253,7 @@ coarray:
> > -      else if (ar->dimen_type[ar->codimen + ar->dimen] == DIMEN_STAR)
> > +      else if (ar->dimen_type[ar->codimen + ar->dimen - 1] == DIMEN_STAR)
> That one is not completely obvious.

I think that the Coverity warning (and hence the patch) is bogus. 
Actually, all three out-of-bounds warnings in gcc/fortran seem to be 
wrong. I re-checked the code and do now believe that the old version is 
perfectly okay.

If you want to know the Coverity's diagnostic, you can also have access 
to GCC's Coverity scan results. Are you interested?


> --- a/gcc/fortran/trans-io.c
> +++ b/gcc/fortran/trans-io.c
> @@ -2310,7 +2313,7 @@ gfc_trans_transfer (gfc_code * code)
> -  if (se.ss == NULL)
> +  if (se.ss == NULL || expr->rank == 0)
>
> I'm not sure about that one.
> I tried adding a `gcc_assert (expr->rank != 0);' in the `else' branch
> and it passed for me on pr36286.f90 (but I don't have graphite enabled).
>
> All the other changes look good to me (OK for them).

I have committed the two patches without the bogus array.c change and 
without the change above.

I will now rebuild again with
   gcc_assert (expr->rank != 0);
and post all failures.

(A while later.) Hmm, for some reasons, I do not see any failure. I 
wonder why I saw about a dozen failures before.

I am really happy because those two parts of the patch I didn't like.

> > TODO:
> >                gfc_free_namelist (old->namelist_tail);
> >                old->namelist_tail->next = NUL
> According to the code in match.c, `namelist_tail' seems to be the last
> element in the chain so that new elements can be added to
> `namelist_tail->next'.
> Then `gfc_undo_symbol' should probably have:
>                 gfc_free_namelist (old->namelist_tail->next);
>                 old->namelist_tail->next = NULL;

Thanks for the analysis. Will you create a patch?

Tobias