[PATCH] PR fortran/68283 -- remove a rogue gfc_internal_error()

[PATCH] PR fortran/68283 -- remove a rogue gfc_internal_error()

[Steve Kargl]

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

This probably falls under the "obviously correct" moniker.
It has been built and tested on i386-*-freebsd.  OK to commit?

The patch removes a gfc_internal_error().  I suspect that it
was originally put into gfortran to cover "correctly written
valid Fortran code cannot possibly ever hit this line of 
code; so, it must be an internal error to reach this line".
The code in PR 68283 is not valid Fortran.  A number of error
messages are spewed by gfortran prior to hitting this line of code.
The patch simply removes the gfc_internal_error(), which allows
gfortran to exit gracefully.

2015-11-11  Steven G. Kargl  <kargl@gcc.gnu.org>

	PR fortran/68283
	* primary.c (gfc_variable_attr): Remove a gfc_internal_error().

-- 
Steve

[-- Attachment #2: pr68283.diff --]
[-- Type: text/x-diff, Size: 414 bytes --]

Index: gcc/fortran/primary.c
===================================================================
--- gcc/fortran/primary.c	(revision 229970)
+++ gcc/fortran/primary.c	(working copy)
@@ -2268,8 +2268,6 @@ gfc_variable_attr (gfc_expr *expr, gfc_t
 		    && errors > 0)
 		  break;
 	      }
-	    if (n == ref->u.ar.as->rank)
-	      gfc_internal_error ("gfc_variable_attr(): Bad array reference");
 	  }
 
 	break;

Re: [PATCH] PR fortran/68283 -- remove a rogue gfc_internal_error()

[Jerry DeLisle]

On 11/11/2015 10:34 AM, Steve Kargl wrote:
> This probably falls under the "obviously correct" moniker.
> It has been built and tested on i386-*-freebsd.  OK to commit?
> 
> The patch removes a gfc_internal_error().  I suspect that it
> was originally put into gfortran to cover "correctly written
> valid Fortran code cannot possibly ever hit this line of 
> code; so, it must be an internal error to reach this line".
> The code in PR 68283 is not valid Fortran.  A number of error
> messages are spewed by gfortran prior to hitting this line of code.
> The patch simply removes the gfc_internal_error(), which allows
> gfortran to exit gracefully.
> 
> 2015-11-11  Steven G. Kargl  <kargl@gcc.gnu.org>
> 
> 	PR fortran/68283
> 	* primary.c (gfc_variable_attr): Remove a gfc_internal_error().
> 

OK Steve,

Jerry

Re: [PATCH] PR fortran/68283 -- remove a rogue gfc_internal_error()

[Steve Kargl]

On Sat, Nov 14, 2015 at 02:51:08PM +0100, Dominique d'Humi??res wrote:
> Hi Steve,
> 
> Although I have not strong objection to your proposed patch,
> I???ld prefer the following one

The patch is fine.  Need a ChangeLog entry.

(patch elided)

>  
> Now both patches are just papering over the real issues:
> 
> (1) Why is this block reached when compiling with -ffrontend-optimize,
> but not with -fno-frontend-optimize (Thomas)?

I saw Thomas's reply.  Of course, -ffrontend-optimize takes a
different code path through the compiler and rewrites some of
the internal state along the way.  If the source code is fixed,
the ICE goes away.  Why waste time worrying about the cause of
an ICE that clearly should be suppressed in the presences of a
sequence of emitted errors?

> (2) Is there expected side effect(s) when removing the' for???
> block introduced at revision r221955 for pr56852 (Paul)?

I doubt that there are anything side effects.

-- 
Steve

Re: [PATCH] PR fortran/68283 -- remove a rogue gfc_internal_error()

[Thomas Koenig]

Hi Dominique,

> (1) Why is this block reached when compiling with -ffrontend-optimize, but not with -fno-frontend-optimize (Thomas)?

The problem here is that gfc_variable_attr is called (indirectly)
during optimize_assignment.  In this case, this causes the ICE
because of the existing error condition.

Here is the backtrace:

#0  gfc_internal_error (gmsgid=gmsgid@entry=0x14e4170 
"gfc_variable_attr(): Bad array reference") at 
../../trunk/gcc/fortran/error.c:1288
#1  0x000000000069f15a in gfc_variable_attr (expr=<optimized out>, 
ts=ts@entry=0x0) at ../../trunk/gcc/fortran/primary.c:2272
#2  0x000000000069f180 in gfc_expr_attr (e=e@entry=0x2162d20) at 
../../trunk/gcc/fortran/primary.c:2351
#3  0x00000000006d50de in gfc_check_dependency (expr1=0x2162990, 
expr2=0x2162d20, identical=<optimized out>) at 
../../trunk/gcc/fortran/dependency.c:1292
#4  0x00000000006d5043 in gfc_check_dependency (expr1=0x2162990, 
expr2=0x20da550, identical=true) at 
../../trunk/gcc/fortran/dependency.c:1260
#5  0x0000000000770f8c in optimize_assignment (c=0x20da730) at 
../../trunk/gcc/fortran/frontend-passes.c:1162
#6  optimize_code (c=<optimized out>, walk_subtrees=<optimized out>, 
data=<optimized out>) at ../../trunk/gcc/fortran/frontend-passes.c:206


It might be worth not running the optimization when error conditions
exist.  I'll think about this.

However, I have no objection to just removing the gfc_internal_error.

Regards

	Thomas

Re: [PATCH] PR fortran/68283 -- remove a rogue gfc_internal_error()

[Dominique d'Humières]

Hi Steve,

Although I have not strong objection to your proposed patch, I’ld prefer the following one

--- ../_clean/gcc/fortran/primary.c	2015-10-18 13:07:28.000000000 +0200
+++ gcc/fortran/primary.c	2015-11-13 23:32:08.000000000 +0100
@@ -2194,7 +2194,7 @@ check_substring:
 symbol_attribute
 gfc_variable_attr (gfc_expr *expr, gfc_typespec *ts)
 {
-  int dimension, codimension, pointer, allocatable, target, n;
+  int dimension, codimension, pointer, allocatable, target;
   symbol_attribute attr;
   gfc_ref *ref;
   gfc_symbol *sym;
@@ -2253,22 +2253,9 @@ gfc_variable_attr (gfc_expr *expr, gfc_t
 	  case AR_UNKNOWN:
 	    /* If any of start, end or stride is not integer, there will
 	       already have been an error issued.  */
-	    for (n = 0; n < ref->u.ar.as->rank; n++)
-	      {
-		int errors;
-		gfc_get_errors (NULL, &errors);
-		if (((ref->u.ar.start[n]
-		      && ref->u.ar.start[n]->ts.type == BT_UNKNOWN)
-		     ||
-		     (ref->u.ar.end[n]
-		      && ref->u.ar.end[n]->ts.type == BT_UNKNOWN)
-		     ||
-		     (ref->u.ar.stride[n]
-		      && ref->u.ar.stride[n]->ts.type == BT_UNKNOWN))
-		    && errors > 0)
-		  break;
-	      }
-	    if (n == ref->u.ar.as->rank)
+	    int errors;
+	    gfc_get_errors (NULL, &errors);
+	    if (errors == 0)
 	      gfc_internal_error ("gfc_variable_attr(): Bad array reference");
 	  }
 
Now both patches are just papering over the real issues:

(1) Why is this block reached when compiling with -ffrontend-optimize, but not with -fno-frontend-optimize (Thomas)?
(2) Is there expected side effect(s) when removing the' for‘ block introduced at revision r221955 for pr56852 (Paul)?

Dominique