Skip to content

Commit 1eb696f

Browse files
krystophnyharald-anlauf
authored andcommitted
fortran: Fix ICE and self-assignment bugs with recursive allocatable finalizers [PR90519]
Derived types with recursive allocatable components and FINAL procedures trigger an ICE in gimplify_call_expr because the finalizer wrapper's result symbol references itself (final->result = final), creating a cycle. This patch creates a separate __result_<typename> symbol to break the cycle. Self-assignment (a = a) with such types causes use-after-free because the left-hand side is finalized before copying, destroying the source. This patch adds detection using gfc_dep_compare_expr at compile time and pointer comparison at runtime to skip finalization when lhs == rhs. Parenthesized self-assignment (a = (a)) creates a temporary, defeating the simple self-assignment detection. This patch adds strip_parentheses() to look through INTRINSIC_PARENTHESES operators and ensure deep_copy is enabled for such cases. Test pr112459.f90 now expects 6 _final calls instead of 12 because separate result symbols eliminate double-counting in tree dumps. PR fortran/90519 gcc/fortran/ChangeLog: * trans-expr.cc (strip_parentheses): New helper function to strip INTRINSIC_PARENTHESES operators from expressions. (is_runtime_conformable): Use strip_parentheses to handle cases like a = (a) when checking for self-assignment. (gfc_trans_assignment_1): Strip parentheses before checking if expr2 is a variable, ensuring deep_copy is enabled for cases like a = (a). Also strip parentheses when checking for self-assignment to avoid use-after-free in finalization. (gfc_trans_scalar_assign): Add comment about parentheses handling. * class.cc (generate_finalization_wrapper): Create separate result symbol for finalizer wrapper functions instead of self-referencing the procedure symbol, avoiding ICE in gimplify_call_expr. gcc/testsuite/ChangeLog: * gfortran.dg/finalizer_recursive_alloc_1.f90: New test for ICE fix. * gfortran.dg/finalizer_recursive_alloc_2.f90: New execution test. * gfortran.dg/finalizer_self_assign.f90: New test for self-assignment including a = a, a = (a), and a = (((a))) cases using if/stop pattern. * gfortran.dg/pr112459.f90: Update to expect 6 _final calls instead of 12, reflecting corrected self-assignment behavior. Signed-off-by: Christopher Albert <[email protected]>
1 parent 5e62a23 commit 1eb696f

File tree

6 files changed

+223
-14
lines changed

6 files changed

+223
-14
lines changed

gcc/fortran/class.cc

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1733,10 +1733,12 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns,
17331733
{
17341734
gfc_symbol *final, *array, *fini_coarray, *byte_stride, *sizes, *strides;
17351735
gfc_symbol *ptr = NULL, *idx, *idx2, *is_contiguous, *offset, *nelem;
1736+
gfc_symbol *result = NULL;
17361737
gfc_component *comp;
17371738
gfc_namespace *sub_ns;
17381739
gfc_code *last_code, *block;
17391740
char *name;
1741+
char *result_name;
17401742
bool finalizable_comp = false;
17411743
gfc_expr *ancestor_wrapper = NULL, *rank;
17421744
gfc_iterator *iter;
@@ -1824,14 +1826,33 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns,
18241826
final->attr.function = 1;
18251827
final->attr.pure = 0;
18261828
final->attr.recursive = 1;
1827-
final->result = final;
18281829
final->ts.type = BT_INTEGER;
18291830
final->ts.kind = 4;
18301831
final->attr.artificial = 1;
18311832
final->attr.always_explicit = 1;
18321833
final->attr.if_source = IFSRC_DECL;
18331834
if (ns->proc_name->attr.flavor == FL_MODULE)
18341835
final->module = ns->proc_name->name;
1836+
1837+
/* Create a separate result symbol instead of using final->result = final.
1838+
Self-referencing result symbols (final->result = final) create a cycle
1839+
in the symbol structure that causes an ICE in gimplify_call_expr when
1840+
the finalizer wrapper is used as a procedure pointer initializer. */
1841+
result_name = xasprintf ("__result_%s", tname);
1842+
if (gfc_get_symbol (result_name, sub_ns, &result) != 0)
1843+
gfc_internal_error ("Failed to create finalizer result symbol");
1844+
free (result_name);
1845+
1846+
if (!gfc_add_flavor (&result->attr, FL_VARIABLE, result->name,
1847+
&gfc_current_locus)
1848+
|| !gfc_add_result (&result->attr, result->name, &gfc_current_locus))
1849+
gfc_internal_error ("Failed to set finalizer result attributes");
1850+
1851+
result->ts = final->ts;
1852+
result->attr.artificial = 1;
1853+
gfc_set_sym_referenced (result);
1854+
gfc_commit_symbol (result);
1855+
final->result = result;
18351856
gfc_set_sym_referenced (final);
18361857
gfc_commit_symbol (final);
18371858

@@ -1959,7 +1980,7 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns,
19591980

19601981
/* Set return value to 0. */
19611982
last_code = gfc_get_code (EXEC_ASSIGN);
1962-
last_code->expr1 = gfc_lval_expr_from_sym (final);
1983+
last_code->expr1 = gfc_lval_expr_from_sym (result);
19631984
last_code->expr2 = gfc_get_int_expr (4, NULL, 0);
19641985
sub_ns->code = last_code;
19651986

gcc/fortran/trans-expr.cc

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11697,7 +11697,17 @@ gfc_trans_scalar_assign (gfc_se *lse, gfc_se *rse, gfc_typespec ts,
1169711697
}
1169811698

1169911699
gfc_add_block_to_block (&block, &rse->pre);
11700-
gfc_add_block_to_block (&block, &lse->finalblock);
11700+
11701+
/* Skip finalization for self-assignment. */
11702+
if (deep_copy && lse->finalblock.head)
11703+
{
11704+
tmp = build3_v (COND_EXPR, cond, build_empty_stmt (input_location),
11705+
gfc_finish_block (&lse->finalblock));
11706+
gfc_add_expr_to_block (&block, tmp);
11707+
}
11708+
else
11709+
gfc_add_block_to_block (&block, &lse->finalblock);
11710+
1170111711
gfc_add_block_to_block (&block, &lse->pre);
1170211712

1170311713
gfc_add_modify (&block, lse->expr,
@@ -12683,12 +12693,30 @@ alloc_scalar_allocatable_for_assignment (stmtblock_t *block,
1268312693
to make sure we do not check for reallocation unneccessarily. */
1268412694

1268512695

12696+
/* Strip parentheses from an expression to get the underlying variable.
12697+
This is needed for self-assignment detection since (a) creates a
12698+
parentheses operator node. */
12699+
12700+
static gfc_expr *
12701+
strip_parentheses (gfc_expr *expr)
12702+
{
12703+
while (expr->expr_type == EXPR_OP
12704+
&& expr->value.op.op == INTRINSIC_PARENTHESES)
12705+
expr = expr->value.op.op1;
12706+
return expr;
12707+
}
12708+
12709+
1268612710
static bool
1268712711
is_runtime_conformable (gfc_expr *expr1, gfc_expr *expr2)
1268812712
{
1268912713
gfc_actual_arglist *a;
1269012714
gfc_expr *e1, *e2;
1269112715

12716+
/* Strip parentheses to handle cases like a = (a). */
12717+
expr1 = strip_parentheses (expr1);
12718+
expr2 = strip_parentheses (expr2);
12719+
1269212720
switch (expr2->expr_type)
1269312721
{
1269412722
case EXPR_VARIABLE:
@@ -13390,10 +13418,15 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * expr2, bool init_flag,
1339013418
}
1339113419

1339213420
/* Comply with F2018 (7.5.6.3). Make sure that any finalization code is added
13393-
after evaluation of the rhs and before reallocation. */
13421+
after evaluation of the rhs and before reallocation.
13422+
Skip finalization for self-assignment to avoid use-after-free.
13423+
Strip parentheses from both sides to handle cases like a = (a). */
1339413424
final_expr = gfc_assignment_finalizer_call (&lse, expr1, init_flag);
13395-
if (final_expr && !(expr2->expr_type == EXPR_VARIABLE
13396-
&& expr2->symtree->n.sym->attr.artificial))
13425+
if (final_expr
13426+
&& gfc_dep_compare_expr (strip_parentheses (expr1),
13427+
strip_parentheses (expr2)) != 0
13428+
&& !(strip_parentheses (expr2)->expr_type == EXPR_VARIABLE
13429+
&& strip_parentheses (expr2)->symtree->n.sym->attr.artificial))
1339713430
{
1339813431
if (lss == gfc_ss_terminator)
1339913432
{
@@ -13416,13 +13449,18 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * expr2, bool init_flag,
1341613449

1341713450
/* If nothing else works, do it the old fashioned way! */
1341813451
if (tmp == NULL_TREE)
13419-
tmp
13420-
= gfc_trans_scalar_assign (&lse, &rse, expr1->ts,
13421-
gfc_expr_is_variable (expr2) || scalar_to_array
13422-
|| expr2->expr_type == EXPR_ARRAY,
13423-
!(l_is_temp || init_flag) && dealloc,
13424-
expr1->symtree->n.sym->attr.codimension,
13425-
assoc_assign);
13452+
{
13453+
/* Strip parentheses to detect cases like a = (a) which need deep_copy. */
13454+
gfc_expr *expr2_stripped = strip_parentheses (expr2);
13455+
tmp
13456+
= gfc_trans_scalar_assign (&lse, &rse, expr1->ts,
13457+
gfc_expr_is_variable (expr2_stripped)
13458+
|| scalar_to_array
13459+
|| expr2->expr_type == EXPR_ARRAY,
13460+
!(l_is_temp || init_flag) && dealloc,
13461+
expr1->symtree->n.sym->attr.codimension,
13462+
assoc_assign);
13463+
}
1342613464

1342713465
/* Add the lse pre block to the body */
1342813466
gfc_add_block_to_block (&body, &lse.pre);
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
! { dg-do compile }
2+
! PR fortran/90519
3+
4+
module pr90519_finalizer_mod
5+
implicit none
6+
type :: t
7+
type(t), allocatable :: child
8+
contains
9+
final :: finalize_t
10+
end type t
11+
contains
12+
subroutine finalize_t(self)
13+
type(t), intent(inout) :: self
14+
end subroutine finalize_t
15+
end module pr90519_finalizer_mod
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
! { dg-do run }
2+
! { dg-output " finalizing id\\s+0\\n finalizing id\\s+1\\n finalizer count =\\s+2\\n" }
3+
! PR fortran/90519
4+
5+
module pr90519_finalizer_run_mod
6+
implicit none
7+
integer :: finalizer_count = 0
8+
type :: tree_t
9+
integer :: id = -1
10+
type(tree_t), allocatable :: child
11+
contains
12+
final :: finalize_tree
13+
end type tree_t
14+
contains
15+
subroutine finalize_tree(self)
16+
type(tree_t), intent(inout) :: self
17+
finalizer_count = finalizer_count + 1
18+
print *, 'finalizing id', self%id
19+
end subroutine finalize_tree
20+
end module pr90519_finalizer_run_mod
21+
22+
program test_finalizer
23+
use pr90519_finalizer_run_mod
24+
implicit none
25+
block
26+
type(tree_t) :: root
27+
root%id = 0
28+
allocate(root%child)
29+
root%child%id = 1
30+
end block
31+
print *, 'finalizer count =', finalizer_count
32+
end program test_finalizer
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
! { dg-do run }
2+
! Test self-assignment with recursive allocatable and finalizer
3+
! This should preserve allocatable components after a = a and a = (a)
4+
5+
module self_assign_mod
6+
implicit none
7+
type :: node_t
8+
integer :: value = 0
9+
type(node_t), allocatable :: next
10+
contains
11+
final :: finalize_node
12+
end type node_t
13+
contains
14+
subroutine finalize_node(self)
15+
type(node_t), intent(inout) :: self
16+
end subroutine finalize_node
17+
end module self_assign_mod
18+
19+
program test_self_assign
20+
use self_assign_mod
21+
implicit none
22+
23+
call test_simple_self_assign()
24+
call test_parenthesized_self_assign()
25+
call test_triple_parenthesized_self_assign()
26+
call test_array_bounds()
27+
28+
contains
29+
30+
subroutine test_simple_self_assign()
31+
type(node_t) :: a
32+
33+
a%value = 100
34+
allocate(a%next)
35+
a%next%value = 200
36+
37+
! Simple self-assignment should preserve all components
38+
a = a
39+
40+
if (a%value /= 100) stop 1
41+
if (.not. allocated(a%next)) stop 2
42+
if (a%next%value /= 200) stop 3
43+
end subroutine test_simple_self_assign
44+
45+
subroutine test_parenthesized_self_assign()
46+
type(node_t) :: a
47+
48+
a%value = 100
49+
allocate(a%next)
50+
a%next%value = 200
51+
52+
! Parenthesized self-assignment should also preserve all components
53+
a = (a)
54+
55+
if (a%value /= 100) stop 4
56+
if (.not. allocated(a%next)) stop 5
57+
if (a%next%value /= 200) stop 6
58+
end subroutine test_parenthesized_self_assign
59+
60+
subroutine test_triple_parenthesized_self_assign()
61+
type(node_t) :: a
62+
63+
a%value = 100
64+
allocate(a%next)
65+
a%next%value = 200
66+
67+
! Triple-nested parentheses should also work correctly
68+
a = (((a)))
69+
70+
if (a%value /= 100) stop 7
71+
if (.not. allocated(a%next)) stop 8
72+
if (a%next%value /= 200) stop 9
73+
end subroutine test_triple_parenthesized_self_assign
74+
75+
subroutine test_array_bounds()
76+
type(node_t), allocatable :: b(:), c(:)
77+
78+
! Test array bounds behavior with parentheses.
79+
! Per F2023:10.2.1.3, lbound((b),1) = 1 even if lbound(b,1) = 5.
80+
! However, for b = (b) where b is already allocated with the right shape,
81+
! NO reallocation occurs, so bounds are preserved.
82+
! For c = (b) where c is unallocated, c gets allocated with default bounds.
83+
allocate(b(5:5))
84+
b(5)%value = 500
85+
86+
! Self-assignment with parentheses: no reallocation (same shape), bounds preserved
87+
b = (b)
88+
if (.not. allocated(b)) stop 10
89+
if (lbound(b, 1) /= 5) stop 11 ! Bounds preserved (no realloc)
90+
if (ubound(b, 1) /= 5) stop 12
91+
if (b(5)%value /= 500) stop 13
92+
93+
! Assignment to unallocated array: gets default (1-based) bounds
94+
c = (b)
95+
if (.not. allocated(c)) stop 14
96+
if (lbound(c, 1) /= 1) stop 15 ! Default bounds (new allocation)
97+
if (ubound(c, 1) /= 1) stop 16
98+
if (c(1)%value /= 500) stop 17
99+
end subroutine test_array_bounds
100+
101+
end program test_self_assign

gcc/testsuite/gfortran.dg/pr112459.f90

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,6 @@ program myprog
3434
print *,"After allocation"
3535
end program myprog
3636
! Final subroutines were called with std=gnu and -w = > 14 "_final"s.
37-
! { dg-final { scan-tree-dump-times "_final" 12 "original" } }
37+
! Count reduced from 12 after PR90519 fix - separate result symbols
38+
! disambiguate procedure references from result variables.
39+
! { dg-final { scan-tree-dump-times "_final" 6 "original" } }

0 commit comments

Comments
 (0)