ICE on wrong code [PR94192]

Message ID e76f75ea-76ac-dd4f-5414-652eb5c3d836@sig-st.de
State Superseded
Headers show
Series
  • ICE on wrong code [PR94192]
Related show

Commit Message

Linus König April 6, 2020, 12:51 p.m.
Hi all,

I'm new to gcc and this is my first patch. The idea is not have another 
resolution of a pointer if an error has occurred previously. I hope this 
meets all the criteria for a patch. In case anything is missing or 
wrong, I'm open to add to or change the patch.

Best regards,

Linus König

2020-04-06  Linus Koenig <link@sig-st.de>

     PR fortran/94192
     * resolve.c (resolve_fl_var_and_proc): Set flag "error" to 1 if
     pointer is found to not have an assumed rank or a deferred shape.
     * simplify.c (simplify_bound): If an error has been issued for a
     given pointer, one should not attempt to find its bounds.

2020-04-06  Linus Koenig <link@sig-st.de>

     PR fortran/94192
     * gfortran.dg/bound_resolve_after_error_1.f90: New test.

! Testcase for bound check after issued error
! See PR 94192
! { dg-do compile }
program bound_for_illegal
  
contains

  subroutine bnds(a)  ! { dg-error "must have a deferred shape or assumed rank" }
    integer, pointer, intent(in) :: a(1:2)
    print *,lbound(a)
  end subroutine bnds

end program bound_for_illegal

Comments

Jason Merrill via Gcc-patches April 6, 2020, 3:19 p.m. | #1
On Mon, Apr 6, 2020 at 8:51 AM Linus König <link@sig-st.de> wrote:
>

> Hi all,

>

> I'm new to gcc and this is my first patch. The idea is not have another

> resolution of a pointer if an error has occurred previously. I hope this

> meets all the criteria for a patch. In case anything is missing or

> wrong, I'm open to add to or change the patch.


First, thanks for your work.

In principle the patch looks fine. I was surprised that I could find
no other code that uses gfc_symbol::error; however, your usage of it
seems appropriate.

A note about style, there should be spaces between logical operators:

   /* Do not attempt to resolve if error has already been issued.  */
-  if (array&&array->symtree&&array->symtree->n.sym)
+  if (array && array->symtree && array->symtree->n.sym)
     {


Additionally, in simplify.c (simplify_bound) you can see the
surrounding code does not check array or array->symtree->n.sym for
NULL. One can assume this precondition holds and factor the code
slightly simpler. I believe also it is sufficient to check only "if
(array_sym->error)". By the time sym->error is set in
resolve_fl_var_and_proc, sym->resolved must already be set to 1
because this is the first step in resolve_symbol.

diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index 66ed925c10d..84aeafc1e8b 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -4090,6 +4090,7 @@ static gfc_expr *
 simplify_bound (gfc_expr *array, gfc_expr *dim, gfc_expr *kind, int upper)
 {
   gfc_ref *ref;
+  gfc_symbol *array_sym;
   gfc_array_spec *as;
   int d;

@@ -4103,8 +4104,13 @@ simplify_bound (gfc_expr *array, gfc_expr *dim,
gfc_expr *kind, int upper)
       goto done;
     }

+  /* Do not attempt to resolve if error has already been issued.  */
+  array_sym = array->symtree->n.sym;
+  if (array_sym->error)
+    return NULL;
+
   /* Follow any component references.  */
-  as = array->symtree->n.sym->as;
+  as = array_sym->as;
   for (ref = array->ref; ref; ref = ref->next)
     {
       switch (ref->type)


Thanks again for your patch.

---
Fritz Reese

Patch

diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 23b5a2b4439..ca149c0dd00 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -12607,6 +12607,7 @@  resolve_fl_var_and_proc (gfc_symbol *sym, int mp_flag)
 	{
 	  gfc_error ("Array pointer %qs at %L must have a deferred shape or "
 		     "assumed rank", sym->name, &sym->declared_at);
+	  sym->error = 1;
 	  return false;
 	}
     }
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index 66ed925c10d..54af85bcab8 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -4089,10 +4089,19 @@  returnNull:
 static gfc_expr *
 simplify_bound (gfc_expr *array, gfc_expr *dim, gfc_expr *kind, int upper)
 {
+  gfc_symbol *array_sym;
   gfc_ref *ref;
   gfc_array_spec *as;
   int d;
 
+  /* Do not attempt to resolve if error has already been issued.  */
+  if (array&&array->symtree&&array->symtree->n.sym)
+    {
+      array_sym = array->symtree->n.sym;
+      if (array_sym->error && array_sym->resolved)
+	return NULL;
+    }
+
   if (array->ts.type == BT_CLASS)
     return NULL;