Discussion:
[PR Fortran/83183] Fix infinite recursion (ICE) with -finit-derived when initializing allocatable BT_DERIVED components
Fritz Reese
2018-06-25 22:06:13 UTC
Permalink
All,

The attached patch fixes PR 83183. Previously, attempting to generate
initializers for allocatable components of the same derived type
triggered infinite recursion. Passes regression tests. OK for trunk
and gcc-8-branch?

From 01961cc78b8ecd5272521098ae3166516a49dcd1 Mon Sep 17 00:00:00 2001
From: Fritz Reese <***@gmail.com>
Date: Mon, 25 Jun 2018 17:51:00 -0400
Subject: [PATCH] PR fortran/83183

Fix infinite recursion occurring with -finit-derived generating initializers
for allocatable derived-type components.

gcc/fortran/

* expr.c (component_initializer): Do not generate initializers with
allocatable BT_DERIVED components.
---
gcc/fortran/expr.c | 1 +
gcc/testsuite/gfortran.dg/init_flag_18.f90 | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/init_flag_18.f90
Steve Kargl
2018-06-26 01:04:15 UTC
Permalink
Post by Fritz Reese
The attached patch fixes PR 83183. Previously, attempting to generate
initializers for allocatable components of the same derived type
triggered infinite recursion. Passes regression tests. OK for trunk
and gcc-8-branch?
if (c->initializer || !generate
|| (ts->type == BT_CLASS && !c->attr.allocatable)
+ || (ts->type == BT_DERIVED && c->attr.allocatable)
Fritz,

The patch looks simple enough. It does seem odd to me
that BT_CLASS has !c->attr.allocatable and BT_DERIVED
is c->attr.allocatable, i.e., bang vs no bang. Is this
because class is not affected by -finit-derived?
--
Steve
Fritz Reese
2018-06-27 21:07:10 UTC
Permalink
On Mon, Jun 25, 2018 at 9:04 PM Steve Kargl
... It does seem odd to me
that BT_CLASS has !c->attr.allocatable and BT_DERIVED
is c->attr.allocatable, i.e., bang vs no bang. Is this
because class is not affected by -finit-derived?
I'm glad you raised the question. As a result I looked a little harder
at the condition -- it had always been somewhat of a mystery to me
actually, as I copied it from some old initializer code. I'll talk
about what I discovered here. For a tl;dr see the bottom for a new
patch.

There are a few subtleties involved. First, 'ts->type' refers to the
type of the structure containing the component, rather than the
component itself. For this reason my patch is actually incorrect. The
new condition should read:

- || (ts->type == BT_DERIVED && c->attr.allocatable)
+ || (c->ts.type == BT_DERIVED && c->attr.allocatable)

The BT_CLASS clause is to prevent generating initializers for
components within a BT_CLASS definition, because these components are
special (_hash, _size, _extends, _def_init, _copy, _final,
_deallocate, _data, _vptr). I believe it is true that the
c->attr.allocatable check is bogus along with c->ts.type == BT_CLASS.
The intent is likely to pass-through component_initializer() for the
special "_data" component, so that EXPR_NULL will be used in
if (comp->attr.allocatable
|| (comp->ts.type == BT_CLASS && CLASS_DATA (comp)->attr.allocatable))
{
I've found I could exploit these weak conditions by using a BT_CLASS
pointer component with -finit-derived. I've reported the issue in PR
86325. After taking a good hard look at the conditions involved, I've
learned the following rules, which were previously unenforced:

* with -finit-derived, allocatable and pointer components (including
BT_CLASS components with an allocatable or pointer _data component)
should initialize with EXPR_NULL
* even without -finit-derived, allocatable components (including
BT_CLASS components with an allocatable _data component) should be
initialized using EXPR_NULL
* special components of a BT_CLASS structure (named _*) should never
have an initializer generated by gfc_generate_initializer()
* gfc_component::initializer is for user-defined assignment
initializers and should never be set by gfc_generate_initializer()

I have thus simplified, implemented, and documented the conditions and
rules above. Vacuously this fixes PR 83183, since a component which
would invoke a recursive derived-type initializer generation must be
allocatable or a pointer; with the above rules, such components are
now assigned EXPR_NULL with -finit-derived which avoids the recursion.
Without -finit-derived, allocatable components are still generated an
EXPR_NULL expression, matching the compiler's original behavior. This
also fixes PR 86325 (mentioned above).

The patch is attached. OK for trunk and 7/8-branch?

From e190d59153eaa7fbfcfabc93db31ddda0de3b869 Mon Sep 17 00:00:00 2001
From: Fritz Reese <***@gmail.com>
Date: Mon, 25 Jun 2018 17:51:00 -0400
Subject: [PATCH 1/3] PR fortran/83183 PR fortran/86325

Fix allocatable/pointer conditions for -finit-derived.

gcc/fortran/

* expr.c (class_allocatable, class_pointer, comp_allocatable,
comp_pointer): New helpers.
(component_initializer): Generate EXPR_NULL for allocatable or pointer
components. Do not generate initializers for components within BT_CLASS.
Do not assign to comp->initializer.
(gfc_generate_initializer): Use new helpers; move code to generate
EXPR_NULL for class allocatable components into component_initializer().

gcc/testsuite/

* gfortran.dg/init_flag_19.f03: New testcase.
---
gcc/fortran/expr.c | 73 ++++++++++++++++++++----------
gcc/testsuite/gfortran.dg/init_flag_18.f90 | 19 ++++++++
gcc/testsuite/gfortran.dg/init_flag_19.f03 | 36 +++++++++++++++
3 files changed, 103 insertions(+), 25 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/init_flag_18.f90
create mode 100644 gcc/testsuite/gfortran.dg/init_flag_19.f03

Loading...