On Thu, Apr 16, 2020 at 7:53 AM Thomas Koenig via Fortran <fort...@gcc.gnu.org> wrote: > > Hello world, > > this patch fixes PR PR93500. One part of it is due to > what Steve wrote in the patch (returning from resolutions when both > operands are NULL), but that still left a nonsensical error. > Returning &gfc_bad_expr when simplifying bounds resulted in the > division by zero error actually reaching the user. > > As to why there is an extra error when this is done in the main > program, as compared to a subroutine, I don't know, but I do not > particularly care. What is important is that the first error message > is clear and reaches the user. > > Regression-tested. OK for trunk?
How odd. It seems something in the procedure matching routine fails to free the symbol node for "a", while this _is_ done for the program case. A bug for another day... IMO a more clear test case uses "implicit none", which reveals the more sensible "Symbol .a. at ... has no IMPLICIT type" (at least for the program case, where a second error is displayed). One can see another variation of this with a declaration like "integer, dimension(lbound(a)) :: c" which gives a similar error "Symbol .a. is used before it is typed at ...". Regarding the new code in simplify.c: bounds[d] = simplify_bound_dim([...]) if (bounds[d] == NULL || bounds[d] == &gfc_bad_expr) { [...] if (gfc_seen_div0) { gfc_free_expr (bounds[d]); return &gfc_bad_expr; } [...] First, it appears if simplify_bound_dim returns &gfc_bad_expr (and a div/0 occurs) then this code will free &gfc_bad_expr. I'm not sure whether or not that can actually occur, but it is certainly incorrect, since &gfc_bad_expr points to static storage. The only other possible case is bounds[d] == NULL, in which case the free is a no-op. I suggest removing the free call. That being said, it looks like the same error condition can occur with the lcobound intrinsic. I see code inside simplify_cobound nearly identical to that in simplify_bound which is not guarded by the new gfc_seen_div0 check. Someone more familiar with coarrays may be able to generate a testcase which exhibits the same regression using lcobound, but I am confident it can occur. This suggests to me that the check is better placed in simplify_bound_dim, which both simplify_bound and simplify_cobound call. If simplify_bound_dim returns &gfc_bad_expr, it appears both routines should continue to do the right thing already (which would not include freeing gfc_bad_expr). It is the call to gfc_resolve_array_spec within simplify_bound_dim which signals the div0, so I believe the div0 check should be inserted right here (around line 4075). How about the following patch to simplify.c instead (which appears to have the fortunate side-effect of fixing a formerly leaked result expression): diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c index d5703e38251..5395694dc67 100644 --- a/gcc/fortran/simplify.c +++ b/gcc/fortran/simplify.c @@ -4073,7 +4073,13 @@ simplify_bound_dim (gfc_expr *array, gfc_expr *kind, int d, int upper, gcc_assert (as); if (!gfc_resolve_array_spec (as, 0)) - return NULL; + { + gfc_free_expr (result); + if (gfc_seen_div0) + return &gfc_bad_expr; + else + return NULL; + } /* The last dimension of an assumed-size array is special. */ if ((!coarray && d == as->rank && as->type == AS_ASSUMED_SIZE && !upper) --- Fritz Reese
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c index d5703e38251..5395694dc67 100644 --- a/gcc/fortran/simplify.c +++ b/gcc/fortran/simplify.c @@ -4073,7 +4073,13 @@ simplify_bound_dim (gfc_expr *array, gfc_expr *kind, int d, int upper, gcc_assert (as); if (!gfc_resolve_array_spec (as, 0)) - return NULL; + { + gfc_free_expr (result); + if (gfc_seen_div0) + return &gfc_bad_expr; + else + return NULL; + } /* The last dimension of an assumed-size array is special. */ if ((!coarray && d == as->rank && as->type == AS_ASSUMED_SIZE && !upper)