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)
