public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine
[not found] <bug-92178-4@http.gcc.gnu.org/bugzilla/>
@ 2023-06-15 18:45 ` anlauf at gcc dot gnu.org
2023-06-16 4:41 ` kargl at gcc dot gnu.org
` (11 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: anlauf at gcc dot gnu.org @ 2023-06-15 18:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92178
anlauf at gcc dot gnu.org changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |anlauf at gcc dot gnu.org
--- Comment #12 from anlauf at gcc dot gnu.org ---
Created attachment 55333
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55333&action=edit
Updated version of Steve's patch
This patch adds evaluation of arguments that are neither constant, variables,
or pointers from functions that may appear in a variable definition context.
This appears to fix all testcases here and "regresses" only for
gfortran.dg/assumed_type_17.f90, where the scan pattern does no longer match.
The tree-dump shows an additional intermediate variable for the call with
explicit parens as compared to without the patch, but the resulting assembler
is unchanged.
I was expecting trouble with finalization, but all current testcases pass.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine
[not found] <bug-92178-4@http.gcc.gnu.org/bugzilla/>
2023-06-15 18:45 ` [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine anlauf at gcc dot gnu.org
@ 2023-06-16 4:41 ` kargl at gcc dot gnu.org
2023-06-16 19:07 ` anlauf at gcc dot gnu.org
` (10 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: kargl at gcc dot gnu.org @ 2023-06-16 4:41 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92178
kargl at gcc dot gnu.org changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |kargl at gcc dot gnu.org
--- Comment #13 from kargl at gcc dot gnu.org ---
(In reply to anlauf from comment #12)
> Created attachment 55333 [details]
> Updated version of Steve's patch
>
> This patch adds evaluation of arguments that are neither constant, variables,
> or pointers from functions that may appear in a variable definition context.
>
> This appears to fix all testcases here and "regresses" only for
> gfortran.dg/assumed_type_17.f90, where the scan pattern does no longer match.
> The tree-dump shows an additional intermediate variable for the call with
> explicit parens as compared to without the patch, but the resulting assembler
> is unchanged.
>
> I was expecting trouble with finalization, but all current testcases pass.
Harald,
Thanks for finding this! I have no pr92178.diff file on any of systems, and I
quite frankly don't know if I can reconstruct the analysis I gave earlier. :-(
It's a good sign that the patch doesn't interfere with finalization.
Hopefully, Mikael or Paul can take a quick peek at the patch. I doubt
I'm allowed to approve your new patch.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine
[not found] <bug-92178-4@http.gcc.gnu.org/bugzilla/>
2023-06-15 18:45 ` [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine anlauf at gcc dot gnu.org
2023-06-16 4:41 ` kargl at gcc dot gnu.org
@ 2023-06-16 19:07 ` anlauf at gcc dot gnu.org
2023-07-02 18:50 ` anlauf at gcc dot gnu.org
` (9 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: anlauf at gcc dot gnu.org @ 2023-06-16 19:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92178
--- Comment #14 from anlauf at gcc dot gnu.org ---
I looked at cases with character arguments, and it appears that there is a
general issue with (lack or improper) generation of temporaries. Consider:
program p
implicit none
character(4), allocatable :: a(:)
integer :: k = -999
a = ["aa","bb"]
! call assign_s (a, (a(2) // "")) ! OK
! call assign_s (a, a(2) // "" ) ! OK
! call assign_s (a, (a(2))) ! no proper temporary
! call assign_s (a, a(2) ) ! invalid Fortran
call assign_s (a, (a(2)(1:3))) ! no proper temporary
print *, allocated (a), k
contains
subroutine assign_s (a, b)
character(*), allocatable, intent(out) :: a(:)
! character(*), value :: b ! rejected (-> pr110290)
character(*) :: b
k = len (b)
print *, b
end subroutine
end
The first two variants work as expected, the fourth is IMHO invalid because
of aliasing, but the third and fifth should work.
However, compiling with -fsanitize=address,undefined shows that they don't.
Inspecting the dump-tree suggests that there is no proper temporary for the
second argument, even though it is requested. Also, running under gdb,
I see that the gfc_evaluate_now from the patch is called.
Do we need to do something special to get temporaries here?
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine
[not found] <bug-92178-4@http.gcc.gnu.org/bugzilla/>
` (2 preceding siblings ...)
2023-06-16 19:07 ` anlauf at gcc dot gnu.org
@ 2023-07-02 18:50 ` anlauf at gcc dot gnu.org
2023-07-02 18:51 ` anlauf at gcc dot gnu.org
` (8 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: anlauf at gcc dot gnu.org @ 2023-07-02 18:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92178
--- Comment #15 from anlauf at gcc dot gnu.org ---
Created attachment 55453
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55453&action=edit
Revised patch
Revised patch that takes Tobias' suggestion into account that we scan
the formal argument list first to determine whether we better evaluate
argument expressions first. This may avoid creating of temporaries for
many cases when none is needed.
This does not yet handle the case of character arguments, where for yet
unknown reasons no temporary is created. Might be deferred for a second
patch.
Working on testcases right now.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine
[not found] <bug-92178-4@http.gcc.gnu.org/bugzilla/>
` (3 preceding siblings ...)
2023-07-02 18:50 ` anlauf at gcc dot gnu.org
@ 2023-07-02 18:51 ` anlauf at gcc dot gnu.org
2023-07-02 20:39 ` anlauf at gcc dot gnu.org
` (7 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: anlauf at gcc dot gnu.org @ 2023-07-02 18:51 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92178
anlauf at gcc dot gnu.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Assignee|unassigned at gcc dot gnu.org |anlauf at gcc dot gnu.org
Status|NEW |ASSIGNED
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine
[not found] <bug-92178-4@http.gcc.gnu.org/bugzilla/>
` (4 preceding siblings ...)
2023-07-02 18:51 ` anlauf at gcc dot gnu.org
@ 2023-07-02 20:39 ` anlauf at gcc dot gnu.org
2023-07-08 14:15 ` cvs-commit at gcc dot gnu.org
` (6 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: anlauf at gcc dot gnu.org @ 2023-07-02 20:39 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92178
--- Comment #16 from anlauf at gcc dot gnu.org ---
Submitted: https://gcc.gnu.org/pipermail/fortran/2023-July/059545.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine
[not found] <bug-92178-4@http.gcc.gnu.org/bugzilla/>
` (5 preceding siblings ...)
2023-07-02 20:39 ` anlauf at gcc dot gnu.org
@ 2023-07-08 14:15 ` cvs-commit at gcc dot gnu.org
2023-07-13 19:31 ` mikael at gcc dot gnu.org
` (5 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-07-08 14:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92178
--- Comment #17 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Harald Anlauf <anlauf@gcc.gnu.org>:
https://gcc.gnu.org/g:b1079fc88f082d3c5b583c8822c08c5647810259
commit r14-2395-gb1079fc88f082d3c5b583c8822c08c5647810259
Author: Harald Anlauf <anlauf@gmx.de>
Date: Wed Jul 5 22:21:09 2023 +0200
Fortran: fixes for procedures with ALLOCATABLE,INTENT(OUT) arguments
[PR92178]
gcc/fortran/ChangeLog:
PR fortran/92178
* trans-expr.cc (gfc_conv_procedure_call): Check procedures for
allocatable dummy arguments with INTENT(OUT) and move deallocation
of actual arguments after evaluation of argument expressions before
the procedure is executed.
gcc/testsuite/ChangeLog:
PR fortran/92178
* gfortran.dg/intent_out_16.f90: New test.
* gfortran.dg/intent_out_17.f90: New test.
* gfortran.dg/intent_out_18.f90: New test.
Co-authored-by: Steven G. Kargl <kargl@gcc.gnu.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine
[not found] <bug-92178-4@http.gcc.gnu.org/bugzilla/>
` (6 preceding siblings ...)
2023-07-08 14:15 ` cvs-commit at gcc dot gnu.org
@ 2023-07-13 19:31 ` mikael at gcc dot gnu.org
2023-07-14 12:16 ` cvs-commit at gcc dot gnu.org
` (4 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: mikael at gcc dot gnu.org @ 2023-07-13 19:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92178
Mikael Morin <mikael at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |mikael at gcc dot gnu.org
--- Comment #18 from Mikael Morin <mikael at gcc dot gnu.org> ---
Followup patches submitted:
https://gcc.gnu.org/pipermail/fortran/2023-July/059580.html
https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624081.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine
[not found] <bug-92178-4@http.gcc.gnu.org/bugzilla/>
` (7 preceding siblings ...)
2023-07-13 19:31 ` mikael at gcc dot gnu.org
@ 2023-07-14 12:16 ` cvs-commit at gcc dot gnu.org
2023-07-14 12:16 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-07-14 12:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92178
--- Comment #19 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Mikael Morin <mikael@gcc.gnu.org>:
https://gcc.gnu.org/g:e93452a5712e87ba624562ba7164b1e1394d18fb
commit r14-2519-ge93452a5712e87ba624562ba7164b1e1394d18fb
Author: Mikael Morin <mikael@gcc.gnu.org>
Date: Fri Jul 14 14:15:07 2023 +0200
fortran: defer class wrapper initialization after deallocation [PR92178]
If an actual argument is associated with an INTENT(OUT) dummy, and code
to deallocate it is generated, generate the class wrapper initialization
after the actual argument deallocation.
This is achieved by passing a cleaned up expression to
gfc_conv_class_to_class, so that the class wrapper initialization code
can be isolated and moved independently after the deallocation.
PR fortran/92178
gcc/fortran/ChangeLog:
* trans-expr.cc (gfc_conv_procedure_call): Use a separate gfc_se
struct, initalized from parmse, to generate the class wrapper.
After the class wrapper code has been generated, copy it back
depending on whether parameter deallocation code has been
generated.
gcc/testsuite/ChangeLog:
* gfortran.dg/intent_out_19.f90: New test.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine
[not found] <bug-92178-4@http.gcc.gnu.org/bugzilla/>
` (8 preceding siblings ...)
2023-07-14 12:16 ` cvs-commit at gcc dot gnu.org
@ 2023-07-14 12:16 ` cvs-commit at gcc dot gnu.org
2023-07-14 12:16 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-07-14 12:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92178
--- Comment #20 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Mikael Morin <mikael@gcc.gnu.org>:
https://gcc.gnu.org/g:71e4d568b1264bca46d30c5fc4933f137d05ca24
commit r14-2520-g71e4d568b1264bca46d30c5fc4933f137d05ca24
Author: Mikael Morin <mikael@gcc.gnu.org>
Date: Fri Jul 14 14:15:21 2023 +0200
fortran: Factor data references for scalar class argument wrapping
[PR92178]
In the case of a scalar actual arg passed to a polymorphic assumed-rank
dummy with INTENT(OUT) attribute, avoid repeatedly evaluating the actual
argument reference by saving a pointer to it. This is non-optimal, but
may also be invalid, because the data reference may depend on its own
content. In that case the expression can't be evaluated after the data
has been deallocated.
There are two ways redundant expressions are generated:
- parmse.expr, which contains the actual argument expression, is
reused to get or set subfields in gfc_conv_class_to_class.
- gfc_conv_class_to_class, to get the virtual table pointer associated
with the argument, generates a new expression from scratch starting
with the frontend expression.
The first part is fixed by saving parmse.expr to a pointer and using
the pointer instead of the original expression.
The second part is fixed by adding a separate field to gfc_se that
is set to the class container expression when the expression to
evaluate is polymorphic. This needs the same field in gfc_ss_info
so that its value can be propagated to gfc_conv_class_to_class which
is modified to use that value. Finally gfc_conv_procedure saves the
expression in that field to a pointer in between to avoid the same
problem as for the first part.
PR fortran/92178
gcc/fortran/ChangeLog:
* trans.h (struct gfc_se): New field class_container.
(struct gfc_ss_info): Ditto.
(gfc_evaluate_data_ref_now): New prototype.
* trans.cc (gfc_evaluate_data_ref_now): Implement it.
* trans-array.cc (gfc_conv_ss_descriptor): Copy class_container
field from gfc_se struct to gfc_ss_info struct.
(gfc_conv_expr_descriptor): Copy class_container field from
gfc_ss_info struct to gfc_se struct.
* trans-expr.cc (gfc_conv_class_to_class): Use class container
set in class_container field if available.
(gfc_conv_variable): Set class_container field on encountering
class variables or components, clear it on encountering
non-class components.
(gfc_conv_procedure_call): Evaluate data ref to a pointer now,
and replace later references by usage of the pointer.
gcc/testsuite/ChangeLog:
* gfortran.dg/intent_out_20.f90: New test.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine
[not found] <bug-92178-4@http.gcc.gnu.org/bugzilla/>
` (9 preceding siblings ...)
2023-07-14 12:16 ` cvs-commit at gcc dot gnu.org
@ 2023-07-14 12:16 ` cvs-commit at gcc dot gnu.org
2024-04-02 17:42 ` anlauf at gcc dot gnu.org
2024-04-02 19:39 ` mikael at gcc dot gnu.org
12 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-07-14 12:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92178
--- Comment #21 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Mikael Morin <mikael@gcc.gnu.org>:
https://gcc.gnu.org/g:9206641d0899e4bae3ad6765129661ff3bcc423a
commit r14-2521-g9206641d0899e4bae3ad6765129661ff3bcc423a
Author: Mikael Morin <mikael@gcc.gnu.org>
Date: Fri Jul 14 14:15:51 2023 +0200
fortran: Reorder array argument evaluation parts [PR92178]
In the case of an array actual arg passed to a polymorphic array dummy
with INTENT(OUT) attribute, reorder the argument evaluation code to
the following:
- first evaluate arguments' values, and data references,
- deallocate data references associated with an allocatable,
intent(out) dummy,
- create a class container using the freed data references.
The ordering used to be incorrect between the first two items,
when one argument was deallocated before a later argument evaluated
its expression depending on the former argument.
r14-2395-gb1079fc88f082d3c5b583c8822c08c5647810259 fixed it by treating
arguments associated with an allocatable, intent(out) dummy in a
separate, later block. This, however, wasn't working either if the data
reference of such an argument was depending on its own content, as
the class container initialization was trying to use deallocated
content.
This change generates class container initialization code in a separate
block, so that it is moved after the deallocation block without moving
the rest of the argument evaluation code.
This alone is not sufficient to fix the problem, because the class
container generation code repeatedly uses the full expression of
the argument at a place where deallocation might have happened
already. This is non-optimal, but may also be invalid, because the data
reference may depend on its own content. In that case the expression
can't be evaluated after the data has been deallocated.
As in the scalar case previously treated, this is fixed by saving
the data reference to a pointer before any deallocation happens,
and then only refering to the pointer. gfc_reset_vptr is updated
to take into account the already evaluated class container if it's
available.
Contrary to the scalar case, one hunk is needed to wrap the parameter
evaluation in a conditional, to avoid regressing in
optional_class_2.f90. This used to be handled by the class wrapper
construction which wrapped the whole code in a conditional. With
this change the class wrapper construction can't see the parameter
evaluation code, so the latter is updated with an additional handling
for optional arguments.
PR fortran/92178
gcc/fortran/ChangeLog:
* trans.h (gfc_reset_vptr): Add class_container argument.
* trans-expr.cc (gfc_reset_vptr): Ditto. If a valid vptr can
be obtained through class_container argument, bypass evaluation
of e.
(gfc_conv_procedure_call): Wrap the argument evaluation code
in a conditional if the associated dummy is optional. Evaluate
the data reference to a pointer now, and replace later
references with usage of the pointer.
gcc/testsuite/ChangeLog:
* gfortran.dg/intent_out_21.f90: New test.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine
[not found] <bug-92178-4@http.gcc.gnu.org/bugzilla/>
` (10 preceding siblings ...)
2023-07-14 12:16 ` cvs-commit at gcc dot gnu.org
@ 2024-04-02 17:42 ` anlauf at gcc dot gnu.org
2024-04-02 19:39 ` mikael at gcc dot gnu.org
12 siblings, 0 replies; 13+ messages in thread
From: anlauf at gcc dot gnu.org @ 2024-04-02 17:42 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92178
anlauf at gcc dot gnu.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Assignee|anlauf at gcc dot gnu.org |mikael at gcc dot gnu.org
--- Comment #22 from anlauf at gcc dot gnu.org ---
Mikael,
since you did the essential work and commit, I am reassigning to you.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine
[not found] <bug-92178-4@http.gcc.gnu.org/bugzilla/>
` (11 preceding siblings ...)
2024-04-02 17:42 ` anlauf at gcc dot gnu.org
@ 2024-04-02 19:39 ` mikael at gcc dot gnu.org
12 siblings, 0 replies; 13+ messages in thread
From: mikael at gcc dot gnu.org @ 2024-04-02 19:39 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92178
Mikael Morin <mikael at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Assignee|mikael at gcc dot gnu.org |unassigned at gcc dot gnu.org
Status|ASSIGNED |NEW
--- Comment #23 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to anlauf from comment #22)
> Mikael,
>
> since you did the essential work and commit, I am reassigning to you.
Well, I'm not working on this any more.
As far as I know, comment #0 is fixed, but comment #14 remains unfixed.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-04-02 19:39 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <bug-92178-4@http.gcc.gnu.org/bugzilla/>
2023-06-15 18:45 ` [Bug fortran/92178] Segmentation fault after passing allocatable array as intent(out) and its element as value into the same subroutine anlauf at gcc dot gnu.org
2023-06-16 4:41 ` kargl at gcc dot gnu.org
2023-06-16 19:07 ` anlauf at gcc dot gnu.org
2023-07-02 18:50 ` anlauf at gcc dot gnu.org
2023-07-02 18:51 ` anlauf at gcc dot gnu.org
2023-07-02 20:39 ` anlauf at gcc dot gnu.org
2023-07-08 14:15 ` cvs-commit at gcc dot gnu.org
2023-07-13 19:31 ` mikael at gcc dot gnu.org
2023-07-14 12:16 ` cvs-commit at gcc dot gnu.org
2023-07-14 12:16 ` cvs-commit at gcc dot gnu.org
2023-07-14 12:16 ` cvs-commit at gcc dot gnu.org
2024-04-02 17:42 ` anlauf at gcc dot gnu.org
2024-04-02 19:39 ` mikael at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).