]> git.ipfire.org Git - thirdparty/gcc.git/commit
tree, gengtype: Fix up GC issue with DECL_VALUE_EXPR [PR118790]
authorJakub Jelinek <jakub@redhat.com>
Thu, 13 Feb 2025 13:14:50 +0000 (14:14 +0100)
committerJakub Jelinek <jakub@gcc.gnu.org>
Thu, 13 Feb 2025 13:14:50 +0000 (14:14 +0100)
commit7738c6286fba7ec2112823f53cc2cefac2c8d007
tree73050cda8d61b0eb85403ce8b5c93f53259b2f9c
parent926c989131e914e69cd73529d183ebd9d637798a
tree, gengtype: Fix up GC issue with DECL_VALUE_EXPR [PR118790]

The following testcase ICEs, because we have multiple levels of
DECL_VALUE_EXPR VAR_DECLs:
  character(kind=1) id_string[1:.id_string] [value-expr: *id_string.55];
  character(kind=1)[1:.id_string] * id_string.55 [value-expr: FRAME.107.id_string.55];
  integer(kind=8) .id_string [value-expr: FRAME.107..id_string];
id_string is the user variable mentioned in BLOCK_VARS, it has
DECL_VALUE_EXPR because it is a VLA, id_string.55 is a temporary created by
gimplify_vla_decl as the address that points to the start of the VLA, what
is normally used in the IL to access it.  But as this artificial var is then
used inside of a nested function, tree-nested.cc adds DECL_VALUE_EXPR to it
too and moves the actual value into the FRAME.107 object's member.
Now, remove_unused_locals removes id_string.55 (and various other VAR_DECLs)
from cfun->local_decls, simply because it is not mentioned in the IL at all
(neither is id_string itself, but that is kept in BLOCK_VARS as it has
DECL_VALUE_EXPR).  So, after this point, id_string.55 tree isn't referenced from
anywhere but id_string's DECL_VALUE_EXPR.  Next GC collection is triggered,
and we are unlucky enough that in the value_expr_for_decl hash table
(underlying hash map for DECL_VALUE_EXPR) the id_string.55 entry comes
before the id_string entry.  id_string is ggc_marked_p because it is
referenced from BLOCK_VARS, but id_string.55 is not, as we don't mark
DECL_VALUE_EXPR anywhere but by gt_cleare_cache on value_expr_for_decl.
But gt_cleare_cache does two things, it calls clear_slots on entries
where the key is not ggc_marked_p (so the id_string.55 mapping to
FRAME.107.id_string.55 is lost and DECL_VALUE_EXPR (id_string.55) becomes
NULL) but then later we see id_string entry, which is ggc_marked_p, so mark
the whole hash table entry, which sets ggc_set_mark on id_string.55.  But
at this point its DECL_VALUE_EXPR is lost.
Later during dwarf2out.cc we want to emit DW_AT_location for id_string, see
it has DECL_VALUE_EXPR, so emit it as indirection of id_string.55 for which
we again lookup DECL_VALUE_EXPR as it has DECL_HAS_VALUE_EXPR_P, but as it
is NULL, we ICE, instead of finding it is a subobject of FRAME.107 for which
we can find its stack location.

Now, as can be seen in the PR, I've tried to tweak tree-ssa-live.cc so that
it would keep id_string.55 in cfun->local_decls; that prohibits it from
the DECL_VALUE_EXPR of it being GC until expansion, but then we shrink and
free cfun->local_decls completely and so GC at that point still can throw
it away.

The following patch adds an extension to the GTY ((cache)) option, before
calling the gt_cleare_cache on some hash table by specifying
GTY ((cache ("somefn"))) it calls somefn on that hash table as well.
And this extra hook can do any additional ggc_set_mark needed so that
gt_cleare_cache preserves everything that is actually needed and throws
away the rest.

In order to make it just 2 pass rather than up to n passes - (if we had
say
id1 -> something, id2 -> x(id1), id3 -> x(id2), id4 -> x(id3), id5 -> x(id4)
in the value_expr_for_decl hash table in that order (where idN are VAR_DECLs
with DECL_HAS_VALUE_EXPR_P, id5 is the only one mentioned from outside and
idN -> X stands for idN having DECL_VALUE_EXPR X, something for some
arbitrary tree and x(idN) for some arbitrary tree which mentions idN
variable) and in each pass just marked the to part of entries with
ggc_marked_p base.from we'd need to repeat until we don't mark anything)
the patch calls walk_tree on DECL_VALUE_EXPR of the marked trees and if it
finds yet unmarked tree, it marks it and walks its DECL_VALUE_EXPR as well
the same way.

2025-02-13  Jakub Jelinek  <jakub@redhat.com>

PR debug/118790
* gengtype.cc (write_roots): Remove cache variable, instead break from
the loop on match and test o for NULL.  If the cache option has
non-empty string argument, call the specified function with v->name
as argument before calling gt_cleare_cache on it.
* tree.cc (gt_value_expr_mark_2, gt_value_expr_mark_1,
gt_value_expr_mark): New functions.
(value_expr_for_decl): Use GTY ((cache ("gt_value_expr_mark"))) rather
than just GTY ((cache)).
* doc/gty.texi (cache): Document optional argument of cache option.

* gfortran.dg/gomp/pr118790.f90: New test.
gcc/doc/gty.texi
gcc/gengtype.cc
gcc/testsuite/gfortran.dg/gomp/pr118790.f90 [new file with mode: 0644]
gcc/tree.cc