Re: ICE on wrong colde [PR94192]

[Fritz Reese via Gcc-patches]

On Fri, Apr 10, 2020 at 8:27 AM Linus König <l...@sig-st.de> wrote:
>
> Hi,
>
> I fixed the style issues. However, omitting the checks for NULL produced
> several regressions in my previous tests.
>

The style looks good. Please share testcases which exhibit the
regressions. They will also need to be included in
gcc/testsuite/gfortran.dg/ as part of the final commit. I am not aware
of a good source of documentation for writing the testcases
appropriate for DejaGnu, but you can examine other testcases to see
some examples. I can help you format the testcases as well. For the
final commit, a ChangeLog entry will also be necessary: take a look at
gcc/fortran/ChangeLog and match the format of previous entries. In
this case an entry for this patch might look something like this:

2020-04-10  Linus König  <l...@sig-st.de>

        PR fortran/94192
        * resolve.c (resolve_fl_var_and_proc): Mark invalid array pointer
        symbol with error.
        * simplify.c (simplify_bound): Don't simplify if an error was
        already reported.


With this patch, I am not convinced that NULL components would be
handled properly. It appears that if array is NULL, the new check will
not pass and the next line "array->ts.type == BT_CLASS" will segfault.
If array->symtree is NULL and the next two checks for BT_CLASS and
EXPR_VARIABLE do not pass, then the line "as = array->symtree->n.sym"
will segfault. Perhaps seeing the relevant testcases will clarify the
conditions which this patch covers.

If you believe the NULL guards are required, please consider the
following replacement for the code in simplify_bound, which would
protect the surrounding code as well:

diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index f63f63c9ef6..20d02210971 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -4155,10 +4155,14 @@ 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;

+  if (!array)
+    return NULL;
+
   if (array->ts.type == BT_CLASS)
     return NULL;

@@ -4169,8 +4173,16 @@ simplify_bound (gfc_expr *array, gfc_expr *dim,
gfc_expr *kind, int upper)
       goto done;
     }

+  if (!array->symtree)
+    return NULL;
+
+  /* Do not attempt to resolve if error has already been issued.  */
+  array_sym = array->symtree->n.sym;
+  if (!array_sym || 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)


---
Fritz Reese