On 16/09/2020 08:02, Andre Vehreschild wrote:
Hi Mark,

a few remarks:

[...]

[PATCH] Fortran  : Two further previously missed ICEs PR53298

There were 3 ICEs with different call stacks in the comments of this
PR.  A previous commit fixed only one of those ICEs.

The ICEs fixed here are in trans-array.c and trans-expr.c.

The first ICE occurred when the array reference is not AR_ELEMENT
gfc_conv_scalarized_array_ref is called with se and ar, if se->ss is
NULL the ICE occurs.  If se->ss is NULL there is nothing to do before
the return.

The second ICE occurs in code that did not match its comments.  Fixing
the code to match the comments fixes the ICE.  A side affect is that
the in the tree dumps for finalize_35.f90 and finalize_36.f90 contain
   ^^^
   Spurious "the" found.
wording has been updated.

[...]

diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 6566c47d4ae..06268739515 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -3638,8 +3638,11 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref *
ar, gfc_expr *expr, /* Handle scalarized references separately.  */
    if (ar->type != AR_ELEMENT)
      {
-      gfc_conv_scalarized_array_ref (se, ar);
-      gfc_advance_se_ss_chain (se);
+      if (se->ss)
+       {
+         gfc_conv_scalarized_array_ref (se, ar);
+         gfc_advance_se_ss_chain (se);
+       }

Why is this only in element ref needed and not every else? When I tried
to fix ICEs this way, I was usually asked if was fixing symptom and not
the error.

This is for references that aren't elements. As I understand it se corresponds to the upper bound, e.g. for a(1:) the upper bound may not be known at this point so se is NULL resulting in a crash due to it being de-referenced in gfc_conv_scalarized_array_ref.


        return;
      }
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 36ff9b5cbc6..193553ace0b 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -2474,8 +2474,8 @@ gfc_conv_component_ref (gfc_se * se, gfc_ref *
ref) RECORD_TYPE within a UNION_TYPE) always use the given FIELD_DECL.
*/
    if (context != TREE_TYPE (decl)
-      && !(   TREE_CODE (TREE_TYPE (field)) == UNION_TYPE /* Field is
union */
-           || TREE_CODE (context) == UNION_TYPE))         /* Field is
map */
+      && (   TREE_CODE (context) == UNION_TYPE /* Field is union */
+          || TREE_CODE (context) == MAP_TYPE)) /* Field is map */
      {
        tree f2 = c->norestrict_decl;
        if (!f2 || DECL_FIELD_CONTEXT (f2) != TREE_TYPE (decl))

(Sorry for the line breaks).

I can't help it, but the old code looked so dubious that I wonder why
it worked in the first place. Have you tried with a mapped type?
structure /st/
  union
    map
      character(5) a, b
    end map
    map
      character(10) c
    end map
  end union
end structure

record /st/ r

r.c = "abcde12345"
write(*,*) r.a(2:), r.b(3:), r.c(6:)

end

outputs (as expected) with and without the patch

 bcde34512345

I can add this as an extra test case if required.


Regards,
        Andre


--
https://www.codethink.co.uk/privacy.html

Reply via email to