Discussion:
[Patch, fortran] PR86328 - [8/9 Regression] Runtime segfault reading an allocatable class(*) object in allocate statements
Paul Richard Thomas
2018-08-29 15:55:27 UTC
Permalink
The attached patch fixes PR86328 and PR86760. The regression was
caused by my commit r252949.

The parts of the patch that fix the PRs are in trans.c and
trans-array.c. The problem was caused by fixing the expressions that
would provide the 'span' in gfc_build_array_ref, since the latter
expected a variable expression. A number of evaluations of component
array elements were producing pre blocks that were not added and so
the temporaries were not being evaluated.

The fix is to pass the COMPONENT_REF and extract the 'span' directly from it.

The rest of the patch arises from PR86328 comment #12. In fact, this
took most of the time that I have spent on these PRs :-( Having done
this, I felt that I had to include this part of the patch in the
submission. However, I have found a host of related bugs, which I will
put together in one PR.

My inclination is to commit the patch without the parts in resolve.c,
trans-expr.c and pr86328_12.f90, especially for 8-branch. I am open to
suggestions for 9-branch.

Bootstraps and regtests on FC28/x68_64 - OK for 8- and 9-branches?

Paul

2018-08-29 Paul Thomas <***@gcc.gnu.org>

PR fortran/86328
PR fortran/86760
* resolve.c (resolve_ordinary_assign): Ensure that the vtable
is generated for intrinsic assignment to unlimited polymorphic
entities.
* trans-array.c (gfc_conv_scalarized_array_ref): Do not fix
info->descriptor but pass it directly to gfc_build_array_ref.
(gfc_conv_array_ref): Likewise for se->expr.
* trans-expr.c (trans_class_assignment): For unlimited
polymorphic assignments, 'size' must be multiplied by the rhs
'_len' values if non-zero.
(gfc_trans_assignment_1): For scalar polymorphic assignments to
allocatable lhs, finalize and deallocate before the assignment
is made.
* trans.c (gfc_build_array_ref): If 'decl' is a COMPONENT_REF
obtain the span field directly from it.

2018-08-29 Paul Thomas <***@gcc.gnu.org>

PR fortran/86328
PR fortran/86760
* gfortran.dg/pr86328.f90 : New test.
* gfortran.dg/pr86328_12.f90 : New test of the problem reported
in comment 12 of the PR.
* gfortran.dg/pr86760.f90 : New test.
Janus Weil
2018-08-29 19:18:52 UTC
Permalink
Hi Paul,
Post by Paul Richard Thomas
The attached patch fixes PR86328 and PR86760. The regression was
caused by my commit r252949.
The parts of the patch that fix the PRs are in trans.c and
trans-array.c. The problem was caused by fixing the expressions that
would provide the 'span' in gfc_build_array_ref, since the latter
expected a variable expression. A number of evaluations of component
array elements were producing pre blocks that were not added and so
the temporaries were not being evaluated.
The fix is to pass the COMPONENT_REF and extract the 'span' directly from it.
The rest of the patch arises from PR86328 comment #12. In fact, this
took most of the time that I have spent on these PRs :-( Having done
this, I felt that I had to include this part of the patch in the
submission. However, I have found a host of related bugs, which I will
put together in one PR.
My inclination is to commit the patch without the parts in resolve.c,
trans-expr.c and pr86328_12.f90, especially for 8-branch. I am open to
suggestions for 9-branch.
Bootstraps and regtests on FC28/x68_64 - OK for 8- and 9-branches?
the patch is ok for trunk from my side. I also agree that it makes
sense to backport those parts that address the regression to 8-branch.
Thanks for the fix!

Cheers,
Janus
Paul Richard Thomas
2018-09-01 11:58:39 UTC
Permalink
Hi Janus,

Thanks for the review. I decided to commit just the parts that address
the regression to both branches. Assignment to polymorphic variables
is in such a mess that I did not consider it sensible to apply part of
a fragment of Band Aid. I will raise a PR for the bugs that I know of.

Committed to 8-branch as r264027 and trunk as r264008

Cheers

Paul
Post by Janus Weil
Hi Paul,
Post by Paul Richard Thomas
The attached patch fixes PR86328 and PR86760. The regression was
caused by my commit r252949.
The parts of the patch that fix the PRs are in trans.c and
trans-array.c. The problem was caused by fixing the expressions that
would provide the 'span' in gfc_build_array_ref, since the latter
expected a variable expression. A number of evaluations of component
array elements were producing pre blocks that were not added and so
the temporaries were not being evaluated.
The fix is to pass the COMPONENT_REF and extract the 'span' directly from it.
The rest of the patch arises from PR86328 comment #12. In fact, this
took most of the time that I have spent on these PRs :-( Having done
this, I felt that I had to include this part of the patch in the
submission. However, I have found a host of related bugs, which I will
put together in one PR.
My inclination is to commit the patch without the parts in resolve.c,
trans-expr.c and pr86328_12.f90, especially for 8-branch. I am open to
suggestions for 9-branch.
Bootstraps and regtests on FC28/x68_64 - OK for 8- and 9-branches?
the patch is ok for trunk from my side. I also agree that it makes
sense to backport those parts that address the regression to 8-branch.
Thanks for the fix!
Cheers,
Janus
--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
Dominique d'Humières
2018-09-01 17:29:58 UTC
Permalink
Hi Paul,

For the record, the part of the original patch that has not been applied fixes also pr84543.

Cheers,

Dominique

Loading...