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

Reply via email to