[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 anlauf at gcc dot gnu.org changed: What|Removed |Added Status|WAITING |RESOLVED Resolution|--- |FIXED --- Comment #30 from anlauf at gcc dot gnu.org --- Should be fixed now. Sorry for the breakage.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #29 from CVS Commits --- The master branch has been updated by Harald Anlauf : https://gcc.gnu.org/g:e5abd1cb9160619721336ed800779a01548231f1 commit r11-461-ge5abd1cb9160619721336ed800779a01548231f1 Author: Harald Anlauf Date: Mon May 18 20:27:29 2020 +0200 PR fortran/95053 - division by zero constants Partially revert the fix for PR93499. Replace by checks for valid expressions in the declaration of array shape and PDT KIND and LEN expressions at a later stage. gcc/fortran/ 2020-05-18 Harald Anlauf PR fortran/95053 * arith.c (gfc_divide): Revert hunk introduced by patch for PR93499. * decl.c (variable_decl): Generate error for array shape not being an INTEGER constant. (gfc_get_pdt_instance): Generate error if KIND or LEN expressions in declaration of a PDT instance do not simplify to INTEGER constants. gcc/testsuite/ 2020-05-18 Harald Anlauf PR fortran/95053 * gfortran.dg/dec_structure_23.f90: Adjust to new error messages. * gfortran.dg/pr93499.f90: Adjust to new error messages. * gfortran.dg/pr95053_2.f90: New test. * gfortran.dg/pr95053_3.f90: New test.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #28 from anlauf at gcc dot gnu.org --- A patch based on comment#15 has been posted for review: https://gcc.gnu.org/pipermail/fortran/2020-May/054321.html
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #27 from Steve Kargl --- On Thu, May 14, 2020 at 06:39:24PM +, tkoenig at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 > > --- Comment #26 from Thomas Koenig --- > (In reply to wschmidt from comment #24) > > > I'm afraid I disagree. A divide-by-zero that cannot ever be executed is > > not an error. > > Well, there is PR90302. We could insert some piece of code into the > IL. A warning or an error could then be issued if the piece of code is still > present after the final optimization. > > It would make a nice project, and remove a few more false positives > as well. > gfortran supports a legacy extension of jumping into a if-block. Prior to Harald's patch (which fixes at least 1 ICE), we have % cat a.f90 program foo real x, y real, parameter :: c = 0 x = 1 y = 2 goto 10 if (c > 0) then 10x = (y / c) * c end if print *, x, y end program foo % gfcx -o z a.f90 && ./z a.f90:8:2: 6 |goto 10 | 2 7 |if (c > 0) then 8 | 10x = (y / c) * c | 1 Warning: Legacy Extension: Label at (1) is not in the same block as the GOTO statement at (2) NaN 2. So, using 'if (c > 0)' to assume DCE occurs is invalid as the code is reachable. Perhaps, my original patch submitted at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93499#c2 should have been committed with no attempt to aid a programmer from making a potential mistake.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #26 from Thomas Koenig --- (In reply to wschmidt from comment #24) > I'm afraid I disagree. A divide-by-zero that cannot ever be executed is > not an error. Well, there is PR90302. We could insert some piece of code into the IL. A warning or an error could then be issued if the piece of code is still present after the final optimization. It would make a nice project, and remove a few more false positives as well.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #25 from Bill Schmidt --- But I'm not going to worry about it further.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #24 from wschmidt at linux dot ibm.com --- On 5/14/20 12:08 PM, sgk at troutmask dot apl.washington.edu wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 > > --- Comment #23 from Steve Kargl --- > On Thu, May 14, 2020 at 02:57:37PM +, wschmidt at gcc dot gnu.org wrote: >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 >> >> Bill Schmidt changed: >> >> What|Removed |Added >> >> CC||wschmidt at gcc dot gnu.org >> >> --- Comment #22 from Bill Schmidt --- >> Breaking legitimate code, even if "borderline," does not seem right to me. >> Zero division is generally a runtime exception because of such cases. >> >> You write code for a general case, then later you discover "oh, well, we >> could >> make this variable zero for our specific usage," and now the compiler throws >> a >> fit? Seems like this is warning-level stuff. >> > If Bill's reduction of the several thousand-line file to 10ish > lines is an accurate reduction (and I have no reasons to doubt > that it isn't), then no. It is an programming error. This is > not the first time that gfortran has found a programming error > in WRF. Sure, in this case the 'if (cdleps > 0)' leads to dead > code elimination, but DCE happens after gfortran has done some > constant folding and common subexpression elimination in the > front-end. > I'm afraid I disagree. A divide-by-zero that cannot ever be executed is not an error.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #23 from Steve Kargl --- On Thu, May 14, 2020 at 02:57:37PM +, wschmidt at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 > > Bill Schmidt changed: > >What|Removed |Added > > CC||wschmidt at gcc dot gnu.org > > --- Comment #22 from Bill Schmidt --- > Breaking legitimate code, even if "borderline," does not seem right to me. > Zero division is generally a runtime exception because of such cases. > > You write code for a general case, then later you discover "oh, well, we could > make this variable zero for our specific usage," and now the compiler throws a > fit? Seems like this is warning-level stuff. > If Bill's reduction of the several thousand-line file to 10ish lines is an accurate reduction (and I have no reasons to doubt that it isn't), then no. It is an programming error. This is not the first time that gfortran has found a programming error in WRF. Sure, in this case the 'if (cdleps > 0)' leads to dead code elimination, but DCE happens after gfortran has done some constant folding and common subexpression elimination in the front-end.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 Bill Schmidt changed: What|Removed |Added CC||wschmidt at gcc dot gnu.org --- Comment #22 from Bill Schmidt --- Breaking legitimate code, even if "borderline," does not seem right to me. Zero division is generally a runtime exception because of such cases. You write code for a general case, then later you discover "oh, well, we could make this variable zero for our specific usage," and now the compiler throws a fit? Seems like this is warning-level stuff.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #21 from Bill Seurer --- We can't modify the spec code but we can add "compatibility" options. Shouldn't the if test make the compiler ignore the statement with the divide by zero? It shouldn't ever be executed.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #20 from anlauf at gcc dot gnu.org --- (In reply to Bill Seurer from comment #19) > There's some stuff above this in the module but this is the part that shows > the error and I think it contains all the declarations. > > subroutine Z() >real(r8) :: cld(99,99) >real(r8) cldeps >parameter (cldeps = 0.0_r8) >real(r8) asort(99) >if (cldeps > 0) then > asort(1) = 1.0_r8-(floor(cld(1,7)/cldeps)*cldeps) >endif >return > end subroutine Z > >15 | asort(1) = 1.0_r8-(floor(cld(1,7)/cldeps)*cldeps) > | 1 > Error: Division by zero at (1) Thanks for the small example. The parameter statement makes cldeps to a constant that is zero when that line is processed. Arguably that is borderline code. I see two ways around: 1) either replace the parameter statement for cldeps by an initialization: data cldeps / 0.0_r8 / 2) or add -fno-range-check to the command line. I could adjust the error message so that you get an approprate hint how to switch off that error. What do you think?
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #19 from Bill Seurer --- There's some stuff above this in the module but this is the part that shows the error and I think it contains all the declarations. subroutine Z() real(r8) :: cld(99,99) real(r8) cldeps parameter (cldeps = 0.0_r8) real(r8) asort(99) if (cldeps > 0) then asort(1) = 1.0_r8-(floor(cld(1,7)/cldeps)*cldeps) endif return end subroutine Z 15 | asort(1) = 1.0_r8-(floor(cld(1,7)/cldeps)*cldeps) | 1 Error: Division by zero at (1)
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #18 from Bill Seurer --- I am still cutting down the code but this should answer the question about if it really could be zero: if (cldeps > 0) then do k = k1,k2 asort(nxs) = 1.0_r8-(floor(cld(i,k)/cldeps)*cldeps) . . . 5911 |asort(nxs) = 1.0_r8-(floor(cld(i,k)/cldeps)*cldeps) | 1 Error: Division by zero at (1)
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #17 from Bill Seurer --- he patch works and has no further fallout that I see. I will still try to extract something small from that big fortran function but as I have not written any fortran code in more than 35 years it may take a bit.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 Thomas Koenig changed: What|Removed |Added Status|REOPENED|WAITING --- Comment #16 from Thomas Koenig --- We really need a test case. SPEC is no different to any other closed-source commercial software in that respect.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053
anlauf at gcc dot gnu.org changed:
What|Removed |Added
Status|RESOLVED|REOPENED
Resolution|FIXED |---
--- Comment #15 from anlauf at gcc dot gnu.org ---
A possible workaround is to partly revert the offending commit and handle
problematic cases individually. The original commit intended to handle
two different ICEs with a common invalid code.
Bill, can you try the following patch? (Note that this needs two adjustments
in the testsuite, but otherwise regtests for me. I'm not 100%sure if the
PDT part is correct.)
diff --git a/gcc/fortran/arith.c b/gcc/fortran/arith.c
index dd72f44d377..dd7f5f43930 100644
--- a/gcc/fortran/arith.c
+++ b/gcc/fortran/arith.c
@@ -1806,7 +1806,7 @@ gfc_multiply (gfc_expr *op1, gfc_expr *op2)
gfc_expr *
gfc_divide (gfc_expr *op1, gfc_expr *op2)
{
- if (op2 && op2->expr_type == EXPR_CONSTANT)
+ if (0 && op2 && op2->expr_type == EXPR_CONSTANT)
{
arith rc = ARITH_OK;
switch (op2->ts.type)
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index d650407da41..6866f460224 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -2602,6 +2602,14 @@ variable_decl (int elem)
gfc_free_expr (e);
}
+ if (not_constant && e->ts.type != BT_INTEGER)
+ {
+ gfc_error ("Explicit array shape at %C must be constant of "
+"INTEGER type and not %s type",
+gfc_basic_typename (e->ts.type));
+ m = MATCH_ERROR;
+ goto cleanup;
+ }
if (not_constant)
{
gfc_error ("Explicit shaped array with nonconstant bounds at
%C");
@@ -3736,8 +3744,9 @@ gfc_get_pdt_instance (gfc_actual_arglist *param_list,
gfc_symbol **sym,
if (kind_expr)
{
/* Try simplification even for LEN expressions. */
+ bool ok;
gfc_resolve_expr (kind_expr);
- gfc_simplify_expr (kind_expr, 1);
+ ok = gfc_simplify_expr (kind_expr, 1);
/* Variable expressions seem to default to BT_PROCEDURE.
TODO find out why this is and fix it. */
if (kind_expr->ts.type != BT_INTEGER
@@ -3748,6 +3757,12 @@ gfc_get_pdt_instance (gfc_actual_arglist *param_list,
gfc_symbol **sym,
gfc_basic_typename (kind_expr->ts.type));
goto error_return;
}
+ if (kind_expr->ts.type == BT_INTEGER && !ok)
+ {
+ gfc_error ("The parameter expression at %C does not "
+"simplify to an INTEGER constant");
+ goto error_return;
+ }
tail->expr = gfc_copy_expr (kind_expr);
}
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #14 from Steve Kargl --- On Tue, May 12, 2020 at 06:43:54PM +, seurer at linux dot vnet.ibm.com wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 > > --- Comment #13 from Bill Seurer --- > I don't know fortran and this appears to be part of a multi-thousand line > extremely complex function. > There is only a single division in that line of code. Can you determine what the value of cldeps is? The error message suggests that it is 0. If there is a testcase that you can run, then change the code to if (cldeps == 0) then print *, 'Whoops. cldeps = ', cldeps stop 1 else asort(nxs) = 1.0_r8-(floor(cld(i,k)/cldeps)*cldeps) end if
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #13 from Bill Seurer --- I don't know fortran and this appears to be part of a multi-thousand line extremely complex function.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #12 from anlauf at gcc dot gnu.org --- (In reply to Bill Seurer from comment #11) > /home/seurer/gcc/git/install/gcc-test/bin/gfortran -c -o > module_ra_cam.fppized.o -I. -I./netcdf/include -I./inc -m64 -O0 -g3 > -mcpu=power8 -Wno-deprecated-declarations -fconvert=big-endian -std=legacy > module_ra_cam.fppized.f90 > module_ra_cam.fppized.f90:6806:69: > > 6806 |asort(nxs) = > 1.0_r8-(floor(cld(i,k)/cldeps)*cldeps) > | 1 > Error: Division by zero at (1) > > This error was also triggered by r11-205 but was not fixed by the patch. It > compiles cleanly with r11-204 and earlier. I do not think (though cannot > say for sure) that it really is a divide by zero. > > This is part of 521.wrf_r in the spec2017 test suite I don't have access to spec. Can you provide enough context for a reproducer?
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #11 from Bill Seurer --- /home/seurer/gcc/git/install/gcc-test/bin/gfortran -c -o module_ra_cam.fppized.o -I. -I./netcdf/include -I./inc -m64 -O0 -g3 -mcpu=power8 -Wno-deprecated-declarations -fconvert=big-endian -std=legacy module_ra_cam.fppized.f90 module_ra_cam.fppized.f90:6806:69: 6806 |asort(nxs) = 1.0_r8-(floor(cld(i,k)/cldeps)*cldeps) | 1 Error: Division by zero at (1) This error was also triggered by r11-205 but was not fixed by the patch. It compiles cleanly with r11-204 and earlier. I do not think (though cannot say for sure) that it really is a divide by zero. This is part of 521.wrf_r in the spec2017 test suite
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #10 from Bill Seurer --- I tried the update on the spec 2000/2006 tests that were ICEing before and they compile now.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 anlauf at gcc dot gnu.org changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #8 from anlauf at gcc dot gnu.org --- Should be fixed. Please reopen if you find further issues. Sorry for the breakage.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053
kargl at gcc dot gnu.org changed:
What|Removed |Added
CC||kargl at gcc dot gnu.org
--- Comment #9 from kargl at gcc dot gnu.org ---
(In reply to anlauf from comment #6)
> It gets actually really weird during parsing.
Nah. gfortran runs a sequence of matchers, and queues
errors as it runs. If a matcher is not found, then
(normally only) the last queued error message is emitted.
If a matcher is found, the error message queue is cleaned
up.
> The following (invalid) code shows that the parser is still in an
> early phase where it has not yet decided that it is a FORMAT statement,
> but rather could be something else:
>
> format('x') = x
> end
>
> This gives:
>
> 1 | format('x') = x
> | 1
> Error: The function result on the lhs of the assignment at (1) must have the
> pointer attribute.
>
> while e.g. Intel detects:
>
> foo.f90(1): error #6072: A dummy argument of a statement function must be a
> scalar identifier. ['x']
> format('x') = x
> -^
gfortran and ifort are simply trying different sequences of
matchers.
> The simplest solution is to defer error detection and recovery by restoring
> the previous behavior when the basic type of operand 2 to gfc_divide is
> non-numeric:
>
> diff --git a/gcc/fortran/arith.c b/gcc/fortran/arith.c
> index 1cd0867a941..dd72f44d377 100644
> --- a/gcc/fortran/arith.c
> +++ b/gcc/fortran/arith.c
> @@ -1828,7 +1828,8 @@ gfc_divide (gfc_expr *op1, gfc_expr *op2)
> rc = ARITH_DIV0;
> break;
> default:
> - gfc_internal_error ("gfc_divide(): Bad basic type");
> + /* basic type is non-numeric, handle this elsewhere. */
> + break;
> }
>if (rc == ARITH_DIV0)
> {
>From my long forgotten days of working in arith.c, it is almost
always wrong to emit a gfc_internal_error. Typical an error
message is generated during simplification or resolution that
catches a problems.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #7 from CVS Commits --- The master branch has been updated by Harald Anlauf : https://gcc.gnu.org/g:1422c2e4462c9b7c44aa035ac56af77565556181 commit r11-288-g1422c2e4462c9b7c44aa035ac56af77565556181 Author: Harald Anlauf Date: Mon May 11 21:27:11 2020 +0200 PR fortran/95053 - ICE in gfc_divide(): Bad basic type The fix for PR 93499 introduced a too strict check in gfc_divide that could trigger errors in the early parsing phase. Relax the check and defer to a later stage. gcc/fortran/ 2020-05-11 Harald Anlauf PR fortran/95053 * arith.c (gfc_divide): Do not error out if operand 2 is non-numeric. Defer checks to later stage. gcc/testsuite/ 2020-05-11 Harald Anlauf PR fortran/95053 * gfortran.dg/pr95053.f: New test.
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053
anlauf at gcc dot gnu.org changed:
What|Removed |Added
Last reconfirmed||2020-05-11
Ever confirmed|0 |1
Status|UNCONFIRMED |NEW
--- Comment #6 from anlauf at gcc dot gnu.org ---
It gets actually really weird during parsing.
The following (invalid) code shows that the parser is still in an
early phase where it has not yet decided that it is a FORMAT statement,
but rather could be something else:
format('x') = x
end
This gives:
1 | format('x') = x
| 1
Error: The function result on the lhs of the assignment at (1) must have the
pointer attribute.
while e.g. Intel detects:
foo.f90(1): error #6072: A dummy argument of a statement function must be a
scalar identifier. ['x']
format('x') = x
-^
The simplest solution is to defer error detection and recovery by restoring
the previous behavior when the basic type of operand 2 to gfc_divide is
non-numeric:
diff --git a/gcc/fortran/arith.c b/gcc/fortran/arith.c
index 1cd0867a941..dd72f44d377 100644
--- a/gcc/fortran/arith.c
+++ b/gcc/fortran/arith.c
@@ -1828,7 +1828,8 @@ gfc_divide (gfc_expr *op1, gfc_expr *op2)
rc = ARITH_DIV0;
break;
default:
- gfc_internal_error ("gfc_divide(): Bad basic type");
+ /* basic type is non-numeric, handle this elsewhere. */
+ break;
}
if (rc == ARITH_DIV0)
{
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 kargl at gcc dot gnu.org changed: What|Removed |Added CC||seurer at linux dot vnet.ibm.com --- Comment #5 from kargl at gcc dot gnu.org --- *** Bug 95064 has been marked as a duplicate of this bug. ***
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053
anlauf at gcc dot gnu.org changed:
What|Removed |Added
CC||anlauf at gcc dot gnu.org
--- Comment #4 from anlauf at gcc dot gnu.org ---
Even shorter and simpler (no linebreak needed):
SUBROUTINE MNSTIN ()
* Adding a comma before or after the "/" avoids the ICE
132 FORMAT ('A'/'B')
END
Running this under gdb and setting a breakpoint in gfc_divide,
it appears to really divide characters:
(gdb) p op1->ts.type
$3 = BT_CHARACTER
(gdb) p op2->ts.type
$4 = BT_CHARACTER
What's going on here?
Most likely there's something latent in parsing FORMAT().
[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053 --- Comment #3 from Jürgen Reuter --- Just as a quick cross check: the 10.1 release works without problems, so this indeed must have been introduced in one of the earliest commits after the 10.1 was branched off.
