Hi Paul,
this looks all good now, and is OK for mainline as well as backporting!
***
While playing with the testcase, I found 3 remaining smaller issues that
are pre-existing, so they should not delay your present work. To make
it clear: these are not regressions.
When "maliciously" perturbing the testcase by adding parentheses in the
right places, I see the following:
Replacing
associate (var => foo ()) ! OK after r14-9489-g3fd46d859cda10
by
associate (var => (foo ()))
gives an ICE here with 14-branch and 15-mainline.
Similarly replacing
allocate (y, source = x(1)) ! Gave zero length here
by
allocate (y, source = (x(1)))
Furthermore, replacing
allocate(x, source = foo ())
by
allocate(x, source = (foo ()))
gives a runtime segfault with both 14-branch and 15-mainline.
So this is something for another day...
Thanks for the patch!
Harald
Am 12.05.24 um 13:27 schrieb Paul Richard Thomas:
Hi Harald,
Please find attached my resubmission for pr113363. The changes are as
follows:
(i) The chunk in gfc_conv_procedure_call is new. This was the source of one
of the memory leaks;
(ii) The incorporation of the _len field in trans_class_assignment was done
for the pr84006 patch;
(iii) The source of all the invalid memory accesses and so on was down to
the use of realloc. I tried all sorts of workarounds such as testing the
vptrs and the sizes but only free followed by malloc worked. I have no idea
at all why this is the case; and
(iv) I took account of your remarks about the chunk in trans-array.cc by
removing it and that the chunk in trans-stmt.cc would leak frontend memory.
OK for mainline (and -14 branch after a few-weeks)?
Regards
Paul
Fortran: Fix wrong code in unlimited polymorphic assignment [PR113363]
2024-05-12 Paul Thomas <pa...@gcc.gnu.org>
gcc/fortran
PR fortran/113363
* trans-array.cc (gfc_array_init_size): Use the expr3 dtype so
that the correct element size is used.
* trans-expr.cc (gfc_conv_procedure_call): Remove restriction
that ss and ss->loop be present for the finalization of class
array function results.
(trans_class_assignment): Use free and malloc, rather than
realloc, for character expressions assigned to unlimited poly
entities.
* trans-stmt.cc (gfc_trans_allocate): Build a correct rhs for
the assignment of an unlimited polymorphic 'source'.
gcc/testsuite/
PR fortran/113363
* gfortran.dg/pr113363.f90: New test.
The first chunk in trans-array.cc ensures that the array dtype is set to
the source dtype. The second chunk ensures that the lhs _len field does
not
default to zero and so is specific to dynamic types of character.
Why the two gfc_copy_ref? valgrind pointed my to the tail
of gfc_copy_ref which already has:
dest->next = gfc_copy_ref (src->next);
so this looks redundant and leaks frontend memory?
***
Playing with the testcase, I find several invalid writes with
valgrind, or a heap buffer overflow with -fsanitize=address .
