[Bug fortran/95053] New: [11.0 regression] ICE in f951: gfc_divide()

[Bug fortran/95053] New: [11.0 regression] ICE in f951: gfc_divide()

[juergen.reuter at desy dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

            Bug ID: 95053
           Summary: [11.0 regression] ICE in f951: gfc_divide()
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: juergen.reuter at desy dot de
  Target Milestone: ---

Created attachment 48507
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48507&action=edit
Reproducer

In some of our legacy code which we are still using, the new gcc/gfortran 11.0
release seems to have introduced a regression which leads to an ICE with the
following message, cf. below. I presume that this regression has been
introduced after the 11.0.0 has been opened, so must be really fresh. 
The file is attached, ist just 250 lines. 
$ gfortran -c mnstin.f 
f951: internal compiler error: gfc_divide(): Bad basic type
0x7fce8b gfc_report_diagnostic
        ../../gcc/fortran/error.c:782
0x7fce8b gfc_internal_error(char const*, ...)
        ../../gcc/fortran/error.c:1402
0x7c4411 gfc_divide(gfc_expr*, gfc_expr*)
        ../../gcc/fortran/arith.c:1831
0x838665 match_add_operand
        ../../gcc/fortran/matchexp.c:394
0x8388c6 match_level_2
        ../../gcc/fortran/matchexp.c:480
0x838aa6 match_level_3
        ../../gcc/fortran/matchexp.c:551
0x838bd8 match_level_4
        ../../gcc/fortran/matchexp.c:599
0x838bd8 match_and_operand
        ../../gcc/fortran/matchexp.c:693
0x838de6 match_or_operand
        ../../gcc/fortran/matchexp.c:722
0x838ee6 match_equiv_operand
        ../../gcc/fortran/matchexp.c:765
0x838fe8 match_level_5
        ../../gcc/fortran/matchexp.c:811
0x8380c2 gfc_match_expr(gfc_expr**)
        ../../gcc/fortran/matchexp.c:870
0x863fda match_actual_arg
        ../../gcc/fortran/primary.c:1662
0x8656a5 gfc_match_actual_arglist(int, gfc_actual_arglist**, bool)
        ../../gcc/fortran/primary.c:1927
0x867ecd gfc_match_rvalue(gfc_expr**)
        ../../gcc/fortran/primary.c:3611
0x8382bf match_primary
        ../../gcc/fortran/matchexp.c:157
0x8382bf match_level_1
        ../../gcc/fortran/matchexp.c:211
0x8382bf match_mult_operand
        ../../gcc/fortran/matchexp.c:267
0x8385fc match_add_operand
        ../../gcc/fortran/matchexp.c:356
0x8388c6 match_level_2
        ../../gcc/fortran/matchexp.c:480

[Bug fortran/95053] [11.0 regression] ICE in f951: gfc_divide()

[juergen.reuter at desy dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #1 from Jürgen Reuter <juergen.reuter at desy dot de> ---
I shrank the example even further:
      SUBROUTINE MNSTIN
 132  FORMAT (' UNIT',I3,' ALREADY OPENED WITH NAME:',A/
     +        '                 NEW NAME IGNORED:',A)
      RETURN
      END

It looks like it is the combination of integer format with this special form of
character line continuation.

[Bug fortran/95053] [11.0 regression] ICE in f951: gfc_divide()

[anlauf at gmx dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

Harald Anlauf <anlauf at gmx dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |anlauf at gmx dot de

--- Comment #2 from Harald Anlauf <anlauf at gmx dot de> ---
(In reply to Jürgen Reuter from comment #1)
> I shrank the example even further:
>       SUBROUTINE MNSTIN
>  132  FORMAT (' UNIT',I3,' ALREADY OPENED WITH NAME:',A/
>      +        '                 NEW NAME IGNORED:',A)
>       RETURN
>       END
> 
> It looks like it is the combination of integer format with this special form
> of character line continuation.

Indeed, I found an even shorter reproducer:

      SUBROUTINE MNSTIN ()
* Adding a comma before or after the "/" avoids the ICE
 132  FORMAT (A/
     +     ' B')
      END

I am responsible for the change in gfc_divide, but fail to see
where there is a division...

[Bug fortran/95053] [11.0 regression] ICE in f951: gfc_divide()

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0

[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()

[juergen.reuter at desy dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #3 from Jürgen Reuter <juergen.reuter at desy dot de> ---
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.

[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()

[anlauf at gcc dot gnu.org]

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()

[kargl at gcc dot gnu.org]

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()

[anlauf at gcc dot gnu.org]

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()

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Harald Anlauf <anlauf@gcc.gnu.org>:

https://gcc.gnu.org/g:1422c2e4462c9b7c44aa035ac56af77565556181

commit r11-288-g1422c2e4462c9b7c44aa035ac56af77565556181
Author: Harald Anlauf <anlauf@gmx.de>
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  <anlauf@gmx.de>

            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  <anlauf@gmx.de>

            PR fortran/95053
            * gfortran.dg/pr95053.f: New test.

[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()

[anlauf at gcc dot gnu.org]

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()

[kargl at gcc dot gnu.org]

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()

[seurer at linux dot vnet.ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #10 from Bill Seurer <seurer at linux dot vnet.ibm.com> ---
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()

[seurer at linux dot vnet.ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #11 from Bill Seurer <seurer at linux dot vnet.ibm.com> ---
/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()

[anlauf at gcc dot gnu.org]

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()

[seurer at linux dot vnet.ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #13 from Bill Seurer <seurer at linux dot vnet.ibm.com> ---
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()

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #14 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Tue, May 12, 2020 at 06:43:54PM +0000, seurer at linux dot vnet.ibm.com
wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053
> 
> --- Comment #13 from Bill Seurer <seurer at linux dot vnet.ibm.com> ---
> 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()

[anlauf at gcc dot gnu.org]

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()

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |WAITING

--- Comment #16 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
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()

[seurer at linux dot vnet.ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #17 from Bill Seurer <seurer at linux dot vnet.ibm.com> ---
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()

[seurer at linux dot vnet.ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #18 from Bill Seurer <seurer at linux dot vnet.ibm.com> ---
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()

[seurer at linux dot vnet.ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #19 from Bill Seurer <seurer at linux dot vnet.ibm.com> ---
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()

[anlauf at gcc dot gnu.org]

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()

[seurer at linux dot vnet.ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #21 from Bill Seurer <seurer at linux dot vnet.ibm.com> ---
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()

[wschmidt at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

Bill Schmidt <wschmidt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wschmidt at gcc dot gnu.org

--- Comment #22 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
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()

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #23 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Thu, May 14, 2020 at 02:57:37PM +0000, wschmidt at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053
> 
> Bill Schmidt <wschmidt at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |wschmidt at gcc dot gnu.org
> 
> --- Comment #22 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
> 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()

[wschmidt at linux dot ibm.com]

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 <sgk at troutmask dot apl.washington.edu> ---
> On Thu, May 14, 2020 at 02:57:37PM +0000, wschmidt at gcc dot gnu.org wrote:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053
>>
>> Bill Schmidt <wschmidt at gcc dot gnu.org> changed:
>>
>>             What    |Removed                     |Added
>> ----------------------------------------------------------------------------
>>                   CC|                            |wschmidt at gcc dot gnu.org
>>
>> --- Comment #22 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
>> 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()

[wschmidt at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #25 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
But I'm not going to worry about it further.

[Bug fortran/95053] [11 regression] ICE in f951: gfc_divide()

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #26 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
(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()

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #27 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Thu, May 14, 2020 at 06:39:24PM +0000, tkoenig at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053
> 
> --- Comment #26 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
> (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
10    x = (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 | 10    x = (y / c) * c
      |  1
Warning: Legacy Extension: Label at (1) is not in the same block as
the GOTO statement at (2)
              NaN   2.00000000  

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()

[anlauf at gcc dot gnu.org]

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()

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95053

--- Comment #29 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Harald Anlauf <anlauf@gcc.gnu.org>:

https://gcc.gnu.org/g:e5abd1cb9160619721336ed800779a01548231f1

commit r11-461-ge5abd1cb9160619721336ed800779a01548231f1
Author: Harald Anlauf <anlauf@gmx.de>
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  <anlauf@gmx.de>

            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  <anlauf@gmx.de>

            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()

[anlauf at gcc dot gnu.org]

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.